Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

openvidu-browser: stopping tracks even if no initialized by OpenVidu #107

Closed
mserve opened this issue Aug 13, 2018 · 15 comments
Closed

openvidu-browser: stopping tracks even if no initialized by OpenVidu #107

mserve opened this issue Aug 13, 2018 · 15 comments

Comments

@mserve
Copy link
Contributor

mserve commented Aug 13, 2018

If a session disconnects, MediaTracks are stopped regarding of their origin. This also stops tracks coming from custom sources, i.e. a track is given to OpenVidu.initPublisher's videoSource

This breaks playing in video tags which should be kept also after OpenVidu stops.

@mserve
Copy link
Contributor Author

mserve commented Sep 1, 2018

Hi, should this one be fixed with e456b5e? I got a conflict when merging the current master branch into my dev branch with #108

@pabloFuente
Copy link
Member

This is already included in a patch in the development master branch of OpenVidu Browser library. Will be available in the next release.

@pabloFuente
Copy link
Member

And yes, that is the commit that included the change.

@mserve
Copy link
Contributor Author

mserve commented Sep 1, 2018

Great, thank you! Then I'll test the next days and close both #107 and #108 👍

@mserve
Copy link
Contributor Author

mserve commented Sep 1, 2018

solved in current master ✔️

@mserve mserve closed this as completed Sep 1, 2018
mserve pushed a commit to mserve/openvidu that referenced this issue Oct 6, 2018
@mserve
Copy link
Contributor Author

mserve commented Oct 6, 2018

it seems that the solution in e456b5e did not solve this issue in all cases.

disposeMediaStream(): void {
is still closing my tracks initialized as custom stream.

Maybe #127 would provide a proper solution, using same logic as for disposeWebRtcPeer, at least for my application the

@mserve mserve reopened this Oct 6, 2018
@pabloFuente
Copy link
Member

Hi,

Could you provide a sample code replicating the issue? This way we can assure the problem still exists and if so properly fix it.

Thanks.

@mserve
Copy link
Contributor Author

mserve commented Oct 8, 2018

It's not that easy to provide a mininmal working example, this one is showing the basic approach without the "token" magic which is done via WebSockets communication with a NodeJS backend in my case.

Expected / wanted behaviour:

  1. use getUserMedia to get the source
  2. attach it to a
  3. start sharing the video at some point isung initSession, initPublisher and so on
  4. stop sharing by disconnecting using OpenViduSession.disconnect()
  5. video keeps running
  6. start over with 3.

That way, I establish sessions only when required, but people are "on standby" with granted consent.

Barebone example to show the issue (token magic missing!!!)

<!doctype <!DOCTYPE html>
<html>

<head>
    <meta charset="utf-8" />
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <title>Issue 107</title>
    <meta name="viewport" content="width=device-width, initial-scale=1">
    <script src="openvidu/openvidu-browser/static/js/openvidu-browser-2.5.0.js"></script>
    <script>
        // OpenVidu
        var OV = new OpenVidu();
        var OpenViduSession;
        var publisher;
        var videoTrack;

        // Get user consent
        OV.getUserMedia({
            audioSource: false, // No audio source - not supported in screen share
            videoSource: 'screen', // The source of video. 
            publishAudio: false, // Whether you want to start publishing with your audio unmuted or not
            publishVideo: true, // Start unmuted
        }).then(mediaStream => {
            // Ready
            console.log("[OV] Media Stream ready.");
            videoTrack = mediaStream.getVideoTracks()[0];
            var video = document.createElement('video');
            video.srcObject = new MediaStream([this.videoTrack]);
            document.getElementById("main-video").appendChild(video);
            video.play();
        });


        function startSharing() {
            // Start session
            OpenViduSession = OV.initSession();

            // Set the token
            var token = getToken();

            // Init the publisher
            publisher = OV.initPublisher(undefined, {
                    audioSource: false, // No audio source - not supported in screen share
                    videoSource: videoTrack, // Use existing track to publish
                    mirror: false
                },
                (error) => { // Function to be executed when the method finishes
                    if (error) {
                        console.error('[OV] Error while initializing publisher: ', error);
                    } else {
                        console.log('[OV] Publisher successfully initialized');
                        OpenViduSession.connect().then(
                            function () {
                                OpenViduSession.publish(publisher);
                            });
                    }
                });
        }

        function stopSharing() {
            if (OpenViduSession)
                OpenViduSession.disconnect();

            // Clear session
            OpenViduSession = null;

        }

        function getToken() {
            //Magic to make a token
            return "TOKENTOKENTOKEN";
        }
    </script>
</head>

<body>
    <div id="main-video">

    </div>
    <button onclick="startSharing()">Start sharing</button>
    <button onclick="stopSharing()">Stop sharing</button>
</body>

</html>

On disconnect, a lots of events are triggered (https://openvidu.io/docs/how-do-i/leave-session/) and one of them (streamDestroyed) kills the tracks regardless of the origin by calling disposeMediaStream:

- hence the track is stopped.

Hope that helps to clarify the issue...

@pabloFuente
Copy link
Member

pabloFuente commented Oct 9, 2018

Hi again,

Can you please confirm the following behaviour?:

If you subscribe to event streamDestroyed in your Publisher object, does it appears this INFO message in the browser's console?:

Calling default behavior upon 'streamDestroyed' event dispatched by 'Publisher'

You can do that with a snippet like this:

 publisher.on('streamDestroyed', event => console.log('Event streamDestroyed dispatched by Publisher');

If it does, then we will add a fix to avoid the disposal of custom MediaStream objects

@pabloFuente
Copy link
Member

pabloFuente commented Oct 9, 2018

Either way, the behaviour you stated in your sample code is not really the expected use of OpenVidu session objects. All OpenVidu connections to the session (one for each user) should be kept alive as long as the video conference is running. To unpublish a stream you should simply call Session.unpublish(publisher). To sum up, your point number 3 shouldn't be launched more than one time, at least initSession and session.connect operations. And if you don't want a user to actually leave the videoconference, you shouldn't call session.disconnect.

Regards

@mserve
Copy link
Contributor Author

mserve commented Dec 15, 2018

sorry, didn't find the time to dig into this. After Christmas, I'll update to 2.7.0 and recheck this one

Agreed on your comment on the session, but those can be indeed different conferences - I do some kind of classroom m:n screensharing where students join a virtual room to show a screen on the digital whiteboard hosted by the teachers via browsers only, some kind of m:n approach. I want to keep the consent after the application got it once after login, so the screen share must keep "running". By not ending the sessions, i would have m*n sessions opened in the background :)

@pabloFuente
Copy link
Member

Closing for inactivity

@mserve
Copy link
Contributor Author

mserve commented Oct 4, 2019

I refactored the working example to keep the session. Expected behaviour: after unpublishing, the video should still be running. I can confirm that

Calling default behavior upon 'streamDestroyed' event dispatched by 'Publisher'

shows up in the console and verified using the method you proposed. Below the new example using the openvidu demo servers to test "in real".

BTW: it would be good to state somewhere in the docs that getDisplayMedia has a quite strict same-origin-policy. It seems not to be possible to call getDisplayMedia from a script which is from a different origin. As you see below in the example, I also tried to load OpenVidu from jsdeliver and this leads to

Object { name: "SCREEN_CAPTURE_DENIED", message: "AbortError: Starting video failed" }
<!doctype html>
<html>

<head>
    <meta charset="utf-8" />
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <title>Issue 107</title>
    <meta name="viewport" content="width=device-width, initial-scale=1">
    <script src="openvidu/openvidu-browser/static/js/openvidu-browser-2.11.0.js"></script>
    <!-- <script src="https://cdn.jsdelivr.net/npm/[email protected]/static/js/openvidu-browser-2.11.0.js"
        integrity="sha256-T1wrhhwl/9nCjM02YYU12yEvmo+F97t3xZHG5KsgRsY=" crossorigin="anonymous"></script> -->
    <script src="https://code.jquery.com/jquery-3.3.1.min.js"
        integrity="sha256-FgpCb/KJQlLNfOu91ta32o/NMZxltwRo8QtmkMRdAu8=" crossorigin="anonymous"></script>
    <script>
        // OpenVidu
        var OV = new OpenVidu();
        var OpenViduSession = OV.initSession();

        var publisher;
        var videoTrack;

        // Get user consent
        function getUserConsent() {
            console.log("[OV] Asking for consent....");
            OV.getUserMedia({
                audioSource: false, // No audio source - not supported in screen share
                videoSource: 'screen', // The source of video. 
            }).then(mediaStream => {
                // Ready
                console.log("[OV] Media Stream ready.");
                videoTrack = mediaStream.getVideoTracks()[0];
                var video = document.createElement('video');
                video.srcObject = new MediaStream([this.videoTrack]);
                document.getElementById("main-video").appendChild(video);
                video.play();
                $('#prepareSharing').prop('disabled', false);

            }).catch(err => {
                console.error(err);
            });
        }


        function prepareSharing() {
            console.log('[OV] prepareSharing() initialized');

            publisher = OV.initPublisher(undefined, {
                audioSource: false, // No audio source - not supported in screen share
                videoSource: videoTrack,
                publishAudio: false, // Whether you want to start publishing with your audio unmuted or not
                publishVideo: true, // Start unmuted				
                mirror: false
            },
                (error) => { // Function to be executed when the method finishes
                    if (error) {
                        console.error('[OV] Error while initializing publisher: ', error);
                    } else {
                        console.log('[OV] Publisher successfully initialized');
                        publisher.on('streamDestroyed', event => console.log('Event streamDestroyed dispatched by Publisher'));
                        $('#startSharing').prop('disabled', false);
                    }
                }
            );
        }

        function startSharing() {
            console.log('[OV] startSharing() initialized');
            // Set the token
            getToken('SessionScreen').then(token => {
                console.log("Token: " + token);
                // Connect the publisher

                OpenViduSession.connect(token).then(
                    () => {
                        OpenViduSession.publish(publisher);
                        $('#stopSharing').prop('disabled', false);
                        $('#startSharing').prop('disabled', true);
                    });
            });
        }

        function stopSharing() {
            OpenViduSession.unpublish(publisher);
            $('#stopSharing').prop('disabled', true);
            $('#startSharing').prop('disabled', false);
        }

        /** from demo **/
        var OPENVIDU_SERVER_URL = "https://demos.openvidu.io:4443";
        var OPENVIDU_SERVER_SECRET = "MY_SECRET";

        function getToken(mySessionId) {
            return createSession(mySessionId).then(sessionId => createToken(sessionId));
        }

        function createSession(sessionId) { // See https://openvidu.io/docs/reference-docs/REST-API/#post-apisessions
            return new Promise((resolve, reject) => {
                $.ajax({
                    type: "POST",
                    url: OPENVIDU_SERVER_URL + "/api/sessions",
                    data: JSON.stringify({ customSessionId: sessionId }),
                    headers: {
                        "Authorization": "Basic " + btoa("OPENVIDUAPP:" + OPENVIDU_SERVER_SECRET),
                        "Content-Type": "application/json"
                    },
                    success: response => resolve(response.id),
                    error: (error) => {
                        if (error.status === 409) {
                            resolve(sessionId);
                        } else {
                            console.warn('No connection to OpenVidu Server. This may be a certificate error at ' + OPENVIDU_SERVER_URL);
                            if (window.confirm('No connection to OpenVidu Server. This may be a certificate error at \"' + OPENVIDU_SERVER_URL + '\"\n\nClick OK to navigate and accept it. ' +
                                'If no certificate warning is shown, then check that your OpenVidu Server is up and running at "' + OPENVIDU_SERVER_URL + '"')) {
                                location.assign(OPENVIDU_SERVER_URL + '/accept-certificate');
                            }
                        }
                    }
                });
            });
        }

        function createToken(sessionId) { // See https://openvidu.io/docs/reference-docs/REST-API/#post-apitokens
            return new Promise((resolve, reject) => {
                $.ajax({
                    type: "POST",
                    url: OPENVIDU_SERVER_URL + "/api/tokens",
                    data: JSON.stringify({ session: sessionId }),
                    headers: {
                        "Authorization": "Basic " + btoa("OPENVIDUAPP:" + OPENVIDU_SERVER_SECRET),
                        "Content-Type": "application/json"
                    },
                    success: response => resolve(response.token),
                    error: error => reject(error)
                });
            });
        }
        /*
        function getToken() {
            //Magic to make a token
            return "TOKENTOKENTOKEN";
        } */
    </script>
</head>

<body>
    <div id="main-video">

    </div>
    <button id="getUserConsent" onclick="getUserConsent()">Get user consent</button>
    <button id="prepareSharing" onclick="prepareSharing()" disabled="disabled">Prepare sharing</button>
    <button id="startSharing" onclick="startSharing()" disabled="disabled">Start sharing</button>
    <button id="stopSharing" onclick="stopSharing()" disabled="disabled">Stop sharing</button>
</body>

</html>

@semmel
Copy link
Contributor

semmel commented Aug 17, 2020

Expected / wanted behaviour:

  1. use getUserMedia to get the source
  2. attach it to a element
  3. start sharing the video at some point isung initSession, initPublisher and so on
  4. stop sharing by disconnecting using OpenViduSession.disconnect()
  5. video keeps running
  6. start over with 3.

That way, I establish sessions only when required, but people are "on standby" with granted consent

I have the same problem, but I fear that

makes the pattern above impossible.

It is perfectly reasonable to setup local audio and video before entering the conference. Leaving the conference should not destroy MediaTracks which were created before entering. If it does it would be nice to have a MediaStreamTrack event for that. Unfortunately MediaStreamTrack.stop() does not fire events.

A workaround requires much restructuring of the application:
Instead of OpenViduSession.connect(theAccessToken),OpenViduSession.disconnect() pairs one could use

const publisher = OpenVidu.initPublisher(undefined, {
  audioSource: localAudioTrack,
  videoSource: localVideoTrack,
  publishAudio: false,
  publishVideo: false
});
openViduSession.connect(theAccessToken);
openViduSession.publish(publisher);
 // …
// "enter" the conference
openViduSession.signal({data: "enter"});
publisher.publishAudio(true);
publisher.publishVideo(true);
 // …
// "leave" the conference
openViduSession.signal({data: "leave"});
publisher.publishAudio(false);
publisher.publishVideo(false);

to simulate entering and leaving the conference while the local MediaStreamTracks keep running.

@semmel
Copy link
Contributor

semmel commented Aug 20, 2020

I patched my uncompressed distribution openvidu-browser.js file so that streamDestroyed in StreamEvent.callDefaultBehaviour no longer stops my MediaStreamTracks.

(Note that this is openvidu v. 2.11.0)

Index: web_modules/openvidu-browser.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- web_modules/openvidu-browser.js	(date 1597766287751)
+++ web_modules/openvidu-browser.js	(date 1597766287751)
@@ -4904,15 +4904,17 @@
     /**
      * @hidden
      */
-    Stream.prototype.disposeMediaStream = function () {
+    Stream.prototype.disposeMediaStream = function (shouldNotStopTracks) {
         if (this.mediaStream) {
+          if (!shouldNotStopTracks) {
             this.mediaStream.getAudioTracks().forEach(function (track) {
-                track.stop();
+              track.stop();
             });
             this.mediaStream.getVideoTracks().forEach(function (track) {
-                track.stop();
+              track.stop();
             });
-            delete this.mediaStream;
+          }
+          delete this.mediaStream;
         }
         console.info((!!this.outboundStreamOpts ? 'Local ' : 'Remote ') + "MediaStream from 'Stream' with id [" + this.streamId + '] is now disposed');
     };
@@ -5946,7 +5948,9 @@
                 }
             }
             // Dispose the MediaStream local object
-            this.stream.disposeMediaStream();
+            // see https://github.com/OpenVidu/openvidu/issues/107
+            this.stream.disposeMediaStream(true);
+            
             // Remove from DOM all video elements associated to this Stream, if there's a StreamManager defined
             // (method Session.subscribe must have been called)
             if (this.stream.streamManager)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants