Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Description :
The audio was not rendered because of re-rendering of react element based on
queueCounter and roomInfo. This causes the dom to re-render when call gets accepted
because after accepting call, queueCounter changes.
The audio element gets recreated. But VoIP user probably holds the old one.
The behaviour is not predictable when such case happens. If everything gets cleanly setup,
even if the audio element goes headless, it still continues to play the remote audio.
But in other cases, it is unreferenced the one on dom has its srcObject as null.
This causes no audio.

This fix provides a way to re-initialise the rendering elements in VoIP user
and calls this function on useEffect() if the re-render has happen.
  • Loading branch information
amolghode1981 committed Feb 24, 2022
1 parent 8242369 commit 378bacb
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 1 deletion.
5 changes: 5 additions & 0 deletions client/lib/voip/Stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ export default class Stream {
*/

init(rmElement: HTMLMediaElement): void {
if (this.renderingMediaElement) {
// Someone already has setup the stream and initializing it once again
// Clear the existing stream object
this.renderingMediaElement.srcObject == null;
}
this.renderingMediaElement = rmElement;
}
/**
Expand Down
14 changes: 13 additions & 1 deletion client/lib/voip/VoIPUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,6 @@ export class VoIPUser extends Emitter<VoipEvents> implements OutgoingRequestDele

this.remoteStream = new Stream(remoteStream);
const mediaElement = this.mediaStreamRendered?.remoteMediaElement;

if (mediaElement) {
this.remoteStream.init(mediaElement);
this.remoteStream.onTrackAdded(this.onTrackAdded.bind(this));
Expand Down Expand Up @@ -615,4 +614,17 @@ export class VoIPUser extends Emitter<VoipEvents> implements OutgoingRequestDele
isReady(): boolean {
return this.state.isReady;
}

/**
* This function allows to change the media renderer media elements.
*/
switchMediaRenderer(mediaRenderer: IMediaStreamRenderer): void {
if (this.remoteStream) {
this.mediaStreamRendered = mediaRenderer;
this.remoteStream.init(mediaRenderer.remoteMediaElement);
this.remoteStream.onTrackAdded(this.onTrackAdded.bind(this));
this.remoteStream.onTrackRemoved(this.onTrackRemoved.bind(this));
this.remoteStream.play();
}
}
}
49 changes: 49 additions & 0 deletions client/providers/CallProvider/CallProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,55 @@ export const CallProvider: FC = ({ children }) => {
handleCallHangup,
]);

useEffect(() => {
if (isUseVoipClientResultError(result)) {
return;
}

if (isUseVoipClientResultLoading(result)) {
return;
}
/**
* This code may need a revisit when we handle callinqueue differently.
* Check clickup taks for more details
* https://app.clickup.com/t/22hy1k4
* When customer called a queue (Either using skype or using internal number), call would get established
* customer would hear agent's voice but agent would not hear anything from customer.
* This issue was observed on unstable. It was found to be incosistent to reproduce.
* On some developer env, it would happen randomly. On Safari it did not happen if
* user refreshes before taking every call.
*
* The reason behind this was as soon as agent accepts a call, queueCounter would change.
* This change will trigger re-rendering of media and creation of audio element.
* This audio element gets used by voipClient to render the remote audio.
* Because the re-render happend, it would hold a stale reference.
*
* If the dom is inspected, audio element just before body is usually created by this class.
* this audio element.srcObject contains null value. In working case, it should display
* valid stream object.
*
* Reason for inconsistecies :
* This element is utilised in VoIPUser::setupRemoteMedia
* This function is called when webRTC receives a remote track event. i.e when the webrtc's peer connection
* starts receiving media. This event call back depends on several factors. How does asterisk setup streams.
* How does it creates a bridge which patches up the agent and customer (Media is flowing thru asterisk).
* When it works in de-environment, it was observed that the audio element in dom and the audio element hold
* by VoIPUser is different. Nonetheless, this stale audio element holds valid media stream, which is being played.
* Hence sometimes the audio is heard.
*
* Ideally call component once gets stable, should not get rerendered. Queue, Room creation are the parameters
* which should be independent and should not control the call component.
*
* Solution :
* Either make the audio elemenent rendered independent of rest of the DOM.
* or implement useEffect. This useEffect will reset the rendering elements with the latest audio tag.
*
* Note : If this code gets refactor, revisit the line below to check if this call is needed.
*
*/
remoteAudioMediaRef.current && result.voipClient.switchMediaRenderer({ remoteMediaElement: remoteAudioMediaRef.current });
});

const visitorEndpoint = useEndpoint('POST', 'livechat/visitor');
const voipEndpoint = useEndpoint('GET', 'voip/room');
const voipCloseRoomEndpoint = useEndpoint('POST', 'voip/room.close');
Expand Down

0 comments on commit 378bacb

Please sign in to comment.