From 378bacbf13514740e7c8c3bc26d384a70b280167 Mon Sep 17 00:00:00 2001 From: amolghode Date: Thu, 24 Feb 2022 16:11:54 +0530 Subject: [PATCH] Clickup task : https://app.clickup.com/t/22hy1k4 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. --- client/lib/voip/Stream.ts | 5 ++ client/lib/voip/VoIPUser.ts | 14 +++++- .../providers/CallProvider/CallProvider.tsx | 49 +++++++++++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) diff --git a/client/lib/voip/Stream.ts b/client/lib/voip/Stream.ts index e04e9696e73c..d8d33b95927f 100644 --- a/client/lib/voip/Stream.ts +++ b/client/lib/voip/Stream.ts @@ -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; } /** diff --git a/client/lib/voip/VoIPUser.ts b/client/lib/voip/VoIPUser.ts index 289df0bfb25f..bc71eb3122af 100644 --- a/client/lib/voip/VoIPUser.ts +++ b/client/lib/voip/VoIPUser.ts @@ -241,7 +241,6 @@ export class VoIPUser extends Emitter 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)); @@ -615,4 +614,17 @@ export class VoIPUser extends Emitter 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(); + } + } } diff --git a/client/providers/CallProvider/CallProvider.tsx b/client/providers/CallProvider/CallProvider.tsx index 8af27ee16354..b91c74bb6539 100644 --- a/client/providers/CallProvider/CallProvider.tsx +++ b/client/providers/CallProvider/CallProvider.tsx @@ -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');