Skip to content

Commit

Permalink
Fix: Local video not showing to local user but still broadcasts to ot…
Browse files Browse the repository at this point in the history
…hers in the call (#2558)
  • Loading branch information
JamesBurnside authored Nov 30, 2022
1 parent b3eb013 commit fb0f5f9
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fix issue where local video would be not showing to the user whilst it was broadcasting to everyone else on the call. This happened in a race condition if the local video tile unmounted and remounted in quick succession multiple times.",
"packageName": "@azure/communication-react",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fix issue where local video would be not showing to the user whilst it was broadcasting to everyone else on the call. This happened in a race condition if the local video tile unmounted and remounted in quick succession multiple times.",
"packageName": "@azure/communication-react",
"email": "[email protected]",
"dependentChangeType": "patch"
}
1 change: 1 addition & 0 deletions packages/calling-stateful-client/src/Logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export enum EventNames {
CREATE_STREAM_INVALID_PARAMS = 'CREATE_STREAM_INVALID_PARAMS',
DISPOSE_STREAM_INVALID_PARAMS = 'DISPOSE_LOCAL_STREAM_INVALID_PARAMS',
LOCAL_STREAM_ALREADY_RENDERED = 'LOCAL_STREAM_ALREADY_RENDERED',
LOCAL_STREAM_ALREADY_DISPOSED = 'LOCAL_STREAM_ALREADY_DISPOSED',
LOCAL_STREAM_STOPPING = 'LOCAL_STREAM_STOPPING',
LOCAL_CREATED_STREAM_STOPPING = 'LOCAL_CREATED_STREAM_STOPPING',
LOCAL_STREAM_RENDERING = 'LOCAL_STREAM_RENDERING',
Expand Down
47 changes: 37 additions & 10 deletions packages/calling-stateful-client/src/StreamUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,13 +217,15 @@ async function createViewLocalVideo(
return;
}

// "Stopping" only happens if the stream was in "rendering" but `disposeView` was called.
// Now that `createView` has been re-called, we can flip the state back to "rendering".
if (renderInfo.status === 'Stopping') {
_logEvent(callingStatefulLogger, {
name: EventNames.LOCAL_STREAM_STOPPING,
level: 'warning',
message: 'LocalVideoStream is in the middle of stopping.'
message: 'LocalVideoStream was marked as stopping by dispose view. Resetting state to "Rendering".'
});
console.warn('LocalVideoStream is in the middle of stopping');
internalContext.setLocalRenderInfo(callId, renderInfo.stream, 'Rendering', renderInfo.renderer);
return;
}

Expand Down Expand Up @@ -468,21 +470,46 @@ function disposeViewLocalVideo(context: CallContext, internalContext: InternalCa
return;
}

// Sets the status and also renderer. I think we need to always set renderer to undefined since in all status when
// cleaned up should have renderer as undefined. If the status is 'Rendered' and renderer is not defined it should
// be cleaned up below so we can set it to undefined here.
if (renderInfo.status === 'Rendering') {
// Nothing to dispose of or clean up -- we can safely exit early here.
if (renderInfo.status === 'NotRendered') {
_logEvent(callingStatefulLogger, {
name: EventNames.LOCAL_STREAM_ALREADY_DISPOSED,
level: 'info',
message: 'LocalVideoStream is already disposed.'
});
return;
}

// Status is already marked as "stopping" so we can exit early here. This is because stopping only occurs
// when the stream is being created in createView but hasn't been completed being created yet. The createView
// method will see the "stopping" status and perform the cleanup
if (renderInfo.status === 'Stopping') {
_logEvent(callingStatefulLogger, {
name: EventNames.LOCAL_STREAM_STOPPING,
level: 'info',
message: 'Local stream is still rendering. Changing status to stopping.',
message: 'Local stream is already stopping.',
data: streamLogInfo
});
internalContext.setLocalRenderInfo(callId, renderInfo.stream, 'Stopping', undefined);
} else {
internalContext.setLocalRenderInfo(callId, renderInfo.stream, 'NotRendered', undefined);
return;
}

// If the stream is in the middle of being rendered (i.e. has state "Rendering"), we need the status as
// "stopping" without performing any cleanup. This will tell the `createView` method that it should stop
// rendering and clean up the state once the view has finished being created.
if (renderInfo.status === 'Rendering') {
_logEvent(callingStatefulLogger, {
name: EventNames.REMOTE_STREAM_STOPPING,
level: 'info',
message: 'Remote stream is still rendering. Changing status to stopping.',
data: streamLogInfo
});
internalContext.setLocalRenderInfo(callId, renderInfo.stream, 'Stopping', renderInfo.renderer);
return;
}

// Else the state must be in the "Rendered" state, so we can dispose the renderer and clean up the state.
internalContext.setLocalRenderInfo(callId, renderInfo.stream, 'NotRendered', undefined);

if (renderInfo.renderer) {
_logEvent(callingStatefulLogger, {
name: EventNames.DISPOSING_LOCAL_RENDERER,
Expand Down

0 comments on commit fb0f5f9

Please sign in to comment.