Recognize when a video receives a new srcObject and re-render it

[SejH]

With the deprecation of the src attribute and not being able to use MutationObserver with srcObject, I noticed that adding a new MediaStream to a video element with a already existing srcObject (and not setting it to null in between) will result in the video displaying only a black square.
So to combat that I added a check for if the video "updated" the srcObject and if so re-renders the MediaStream.

[hthetiot]

Can you check Travis build @SejH the npm run lint most likely failed.
Thank you will review once Travis happy.

[hthetiot]

js/videoElementsHandler.js
  line 206  col 13  'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).  (W104)
  line 208  col 17  'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).  (W104)
  line 209  col 17  'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).  (W104)

Please don't use const, (technically let would better in your case) we do es5 here cause once compiled it goes es5 anyway.

Please also run npm run build to generate dist file and please comit the buiild.

Thank you.

[hthetiot]

Thank you @SejH for making travis happy..

Now the review.

1. I'm worried that it does not release the previous Renderer as we should, can you confirm emptied still get trigger when you set srcObject, I dont think so?
2. I think we still need to setup the events from handleVideo(video); can you try this instead it will clear previous renderer and create the new one.

var stream = video.srcObject;
if (stream && typeof stream.getBlobId === 'function') {
    // Release previous renderer	
    releaseMediaStreamRenderer(video);
    // Install new renderer
    provideMediaStreamRenderer(video, stream.getBlobId());
}

[hthetiot]

I'm going to make some quick test to confirm my review assumption.

[hthetiot]

I confirm my feedback on the review after testing the suggested change so the previous renderer get close and a new one get created with the events. So when the original Stream close it do not stop the new renderer but instead create a new one.

Tested using test scripts: https://github.com/cordova-rtc/cordova-plugin-iosrtc/blob/master/extra/renderer-and-libwebrtc-tests.js

And following:

localVideoEl.srcObject = peerStream;
peerStream.stop();

diff --git i/js/videoElementsHandler.js w/js/videoElementsHandler.js
index 5057712..58e220a 100644
--- i/js/videoElementsHandler.js
+++ w/js/videoElementsHandler.js
@@ -201,6 +201,15 @@ function observeVideo(video) {
                if (video.srcObject && !video._iosrtcMediaStreamRendererId) {
                        // If this video element was NOT previously handling a MediaStreamRenderer, release it.
                        handleVideo(video);
+               } else if (video.srcObject && video._iosrtcMediaStreamRendererId) {
+                       // The video element has received a new srcObject.
+                       var stream = video.srcObject;
+                       if (stream && typeof stream.getBlobId === 'function') {
+                           // Release previous renderer        
+                           releaseMediaStreamRenderer(video);
+                           // Install new renderer
+                           provideMediaStreamRenderer(video, stream.getBlobId());
+                       }
                }
        });

[hthetiot]

Thank you @SejH I hope you understand my goal is to make sure this is not creating issues and not diminishing your contribution. Please make the change as suggested and will merge if you want to test more using the test scripts to double check feel free.

Thank you again for finding this issue.

[hthetiot]

Thank you @SejH
Will be released on 5.0.4 and will mention you on Changelog.

[hthetiot]

I also merged on #394 aka task/libwebrtc-update cause i think you also using it. @SejH