[GStreamer][MediaStream] Handle track addition and removal
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Nov 2018 14:01:19 +0000 (14:01 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Nov 2018 14:01:19 +0000 (14:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191599

Patch by Thibault Saunier <tsaunier@igalia.com> on 2018-11-16
Reviewed by Xabier Rodriguez-Calvar.

Source/WebCore:

Test: fast/mediastream/MediaStream-video-element-remove-track.html

* platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:
(WebCore::WebKitMediaStreamObserver::~WebKitMediaStreamObserver):
(WebCore::WebKitMediaStreamObserver::WebKitMediaStreamObserver):
(WebCore::webkitMediaStreamSrcFinalize):
(WebCore::webkitMediaStreamSrcChangeState):
(WebCore::webkit_media_stream_src_init):
(WebCore::webkitMediaStreamSrcSetupSrc):
(WebCore::webkitMediaStreamSrcAddTrack):
(WebCore::webkitMediaStreamSrcRemoveTrackByType):
(WebCore::webkitMediaStreamSrcSetStream):

LayoutTests:

* fast/mediastream/MediaStream-video-element-remove-track-expected.txt: Added.
* fast/mediastream/MediaStream-video-element-remove-track.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238276 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/mediastream/MediaStream-video-element-remove-track-expected.txt [new file with mode: 0644]
LayoutTests/fast/mediastream/MediaStream-video-element-remove-track.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp
Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.h

index 8c59982..6f4c193 100644 (file)
@@ -1,3 +1,13 @@
+2018-11-16  Thibault Saunier  <tsaunier@igalia.com>
+
+        [GStreamer][MediaStream] Handle track addition and removal
+        https://bugs.webkit.org/show_bug.cgi?id=191599
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        * fast/mediastream/MediaStream-video-element-remove-track-expected.txt: Added.
+        * fast/mediastream/MediaStream-video-element-remove-track.html: Added.
+
 2018-11-16  Antoine Quint  <graouts@apple.com>
 
         PointerEvents should not require touch event listeners to be registered
diff --git a/LayoutTests/fast/mediastream/MediaStream-video-element-remove-track-expected.txt b/LayoutTests/fast/mediastream/MediaStream-video-element-remove-track-expected.txt
new file mode 100644 (file)
index 0000000..606fa54
--- /dev/null
@@ -0,0 +1,47 @@
+Tests checking removing MediaStream track applies to the video element.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+
+
+**** calling mediaDevices.getUserMedia() ****
+PASS mediaDevices.getUserMedia succeeded.
+
+**** setup video element ****
+video.srcObject = mediaStream
+Event 'canplay'
+
+*** start playback ****
+video.play()
+video.pause()
+
+**** check video element ****
+
+**** check video tracks ****
+PASS video.videoTracks.length is 1
+PASS video.videoTracks[0].id is mediaStream.getVideoTracks()[0].id
+
+**** check audio tracks ****
+PASS video.audioTracks.length is 1
+PASS video.audioTracks[0].id is mediaStream.getAudioTracks()[0].id
+
+**** removing audio track ****
+
+**** check video element ****
+PASS video.videoWidth is mediaStream.getVideoTracks()[0].getSettings().width
+PASS video.videoHeight is mediaStream.getVideoTracks()[0].getSettings().height
+
+**** check video tracks ****
+PASS video.videoTracks.length is 1
+PASS video.videoTracks[0].id is mediaStream.getVideoTracks()[0].id
+PASS video.videoTracks[0].language is ""
+PASS video.videoTracks[0].kind is "main"
+
+**** check no audio track ****
+PASS video.audioTracks.length is 0
+PASS mediaStream.getAudioTracks().length is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/mediastream/MediaStream-video-element-remove-track.html b/LayoutTests/fast/mediastream/MediaStream-video-element-remove-track.html
new file mode 100644 (file)
index 0000000..2321e72
--- /dev/null
@@ -0,0 +1,112 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+    <head>
+        <script src="../../resources/js-test-pre.js"></script>
+        <script>
+            var video;
+            var mediaStream;
+
+            function logEvent(element, eventName, func)
+            {
+                function _eventCallback(evt)
+                {
+                    if (window.wasFinishJSTestCalled)
+                        return;
+
+                    debug(`Event <em>'${evt.type}'</em>`);
+                    if (func)
+                        func(evt);
+                }
+                element.addEventListener(eventName, _eventCallback, true);
+            }
+
+            function checkVideoElement()
+            {
+                evalAndLog("video.pause()");
+
+                debug("<br>**** check video element ****");
+                debug("<br>**** check video tracks ****");
+                shouldBe('video.videoTracks.length', '1');
+                shouldBe('video.videoTracks[0].id', 'mediaStream.getVideoTracks()[0].id');
+
+                debug("<br>**** check audio tracks ****");
+                shouldBe('video.audioTracks.length', '1');
+                shouldBe('video.audioTracks[0].id', 'mediaStream.getAudioTracks()[0].id');
+
+                setTimeout(removeAudioTrack, 100);
+            }
+
+            function checkVideoElement2()
+            {
+                debug("<br>**** check video element ****");
+                shouldBe('video.videoWidth', 'mediaStream.getVideoTracks()[0].getSettings().width');
+                shouldBe('video.videoHeight', 'mediaStream.getVideoTracks()[0].getSettings().height');
+
+                debug("<br>**** check video tracks ****");
+                shouldBe('video.videoTracks.length', '1');
+                shouldBe('video.videoTracks[0].id', 'mediaStream.getVideoTracks()[0].id');
+                shouldBeEqualToString('video.videoTracks[0].language', '');
+                shouldBeEqualToString('video.videoTracks[0].kind', 'main');
+
+                debug("<br>**** check no audio track ****");
+                shouldBe('video.audioTracks.length', '0');
+                shouldBe('mediaStream.getAudioTracks().length', '0');
+
+                finishJSTest();
+            }
+
+            function canplay()
+            {
+                debug("<br>*** start playback ****");
+                evalAndLog("video.play()");
+                setTimeout(checkVideoElement, 100);
+            }
+
+            function removeAudioTrack() {
+                track = mediaStream.getAudioTracks()[0];
+                debug("<br>**** removing audio track ****");
+                try {
+                    mediaStream.removeTrack(track);
+                } catch (exception) {
+                    testFailed("removeTrack threw an exception.");
+                    finishJSTest();
+                }
+                setTimeout(checkVideoElement2, 100);
+            }
+
+            function setupStream(stream)
+            {
+                mediaStream = stream;
+                testPassed('mediaDevices.getUserMedia succeeded.');
+
+                debug("<br>**** setup video element ****");
+                evalAndLog("video.srcObject = mediaStream");
+            }
+
+            function start()
+            {
+                description("Tests checking removing MediaStream track applies to the video element.");
+                video = document.querySelector('video');
+                logEvent(video, 'canplay', canplay)
+
+                debug("<br>**** calling mediaDevices.getUserMedia() ****");
+                if (window.testRunner)
+                    testRunner.setUserMediaPermission(true);
+                navigator.mediaDevices.getUserMedia( {video: true, audio: true} )
+                    .then(setupStream)
+                    .catch(function(reason) {
+                        debug(`Stream generation failed with error: ${reason}`);
+                    });
+            }
+
+            window.jsTestIsAsync = true;
+            window.successfullyParsed = true;
+        </script>
+    </head>
+    <body onload="start()">
+        <p id="description"></p>
+        <video controls  width="680" height="360"></video>
+        <div id="console"></div>
+        <script src="../../resources/js-test-post.js"></script>
+    </body>
+</html>
index eaa23b0..893e307 100644 (file)
@@ -1,3 +1,23 @@
+2018-11-16  Thibault Saunier  <tsaunier@igalia.com>
+
+        [GStreamer][MediaStream] Handle track addition and removal
+        https://bugs.webkit.org/show_bug.cgi?id=191599
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        Test: fast/mediastream/MediaStream-video-element-remove-track.html
+
+        * platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:
+        (WebCore::WebKitMediaStreamObserver::~WebKitMediaStreamObserver):
+        (WebCore::WebKitMediaStreamObserver::WebKitMediaStreamObserver):
+        (WebCore::webkitMediaStreamSrcFinalize):
+        (WebCore::webkitMediaStreamSrcChangeState):
+        (WebCore::webkit_media_stream_src_init):
+        (WebCore::webkitMediaStreamSrcSetupSrc):
+        (WebCore::webkitMediaStreamSrcAddTrack):
+        (WebCore::webkitMediaStreamSrcRemoveTrackByType):
+        (WebCore::webkitMediaStreamSrcSetStream):
+
 2018-11-16  Zan Dobersek  <zdobersek@igalia.com>
 
         ScalableImageDecoder: don't forcefully decode image data when querying frame completeness, duration
index 09a43e6..2513ca9 100644 (file)
@@ -41,6 +41,8 @@ namespace WebCore {
 static void webkitMediaStreamSrcPushVideoSample(WebKitMediaStreamSrc* self, GstSample* gstsample);
 static void webkitMediaStreamSrcPushAudioSample(WebKitMediaStreamSrc* self, GstSample* gstsample);
 static void webkitMediaStreamSrcTrackEnded(WebKitMediaStreamSrc* self, MediaStreamTrackPrivate&);
+static void webkitMediaStreamSrcAddTrack(WebKitMediaStreamSrc* self, MediaStreamTrackPrivate*);
+static void webkitMediaStreamSrcRemoveTrackByType(WebKitMediaStreamSrc* self, RealtimeMediaSource::Type trackType);
 
 static GstStaticPadTemplate videoSrcTemplate = GST_STATIC_PAD_TEMPLATE("video_src",
     GST_PAD_SRC,
@@ -93,7 +95,7 @@ GstStream* webkitMediaStreamNew(MediaStreamTrackPrivate* track)
         caps = adoptGRef(gst_static_pad_template_get_caps(&videoSrcTemplate));
         type = GST_STREAM_TYPE_VIDEO;
     } else {
-        GST_FIXME("Handle %d type", (gint) track->type());
+        GST_FIXME("Handle %d type", static_cast<int>(track->type()));
 
         return nullptr;
     }
@@ -142,6 +144,30 @@ private:
     WebKitMediaStreamSrc* m_mediaStreamSrc;
 };
 
+class WebKitMediaStreamObserver
+    : public MediaStreamPrivate::Observer {
+public:
+    virtual ~WebKitMediaStreamObserver() { };
+    WebKitMediaStreamObserver(WebKitMediaStreamSrc* src)
+        : m_mediaStreamSrc(src) { }
+
+    void characteristicsChanged() final { GST_DEBUG_OBJECT(m_mediaStreamSrc.get(), "renegotiation should happen"); }
+    void activeStatusChanged() final { }
+
+    void didAddTrack(MediaStreamTrackPrivate& track) final
+    {
+        webkitMediaStreamSrcAddTrack(m_mediaStreamSrc.get(), &track);
+    }
+
+    void didRemoveTrack(MediaStreamTrackPrivate& track) final
+    {
+        webkitMediaStreamSrcRemoveTrackByType(m_mediaStreamSrc.get(), track.type());
+    }
+
+private:
+    GRefPtr<WebKitMediaStreamSrc> m_mediaStreamSrc;
+};
+
 typedef struct _WebKitMediaStreamSrcClass WebKitMediaStreamSrcClass;
 struct _WebKitMediaStreamSrc {
     GstBin parent_instance;
@@ -153,7 +179,8 @@ struct _WebKitMediaStreamSrc {
     GstElement* videoSrc;
     GstClockTime firstFramePts;
 
-    std::unique_ptr<WebKitMediaStreamTrackObserver> observer;
+    std::unique_ptr<WebKitMediaStreamTrackObserver> mediaStreamTrackObserver;
+    std::unique_ptr<WebKitMediaStreamObserver> mediaStreamObserver;
     volatile gint npads;
     gulong probeid;
     RefPtr<MediaStreamPrivate> stream;
@@ -266,8 +293,13 @@ static void webkitMediaStreamSrcFinalize(GObject* object)
     WebKitMediaStreamSrc* self = WEBKIT_MEDIA_STREAM_SRC(object);
 
     GST_OBJECT_LOCK(self);
-    for (auto& track : self->stream->tracks())
-        track->removeObserver(*self->observer.get());
+    if (self->stream) {
+        for (auto& track : self->stream->tracks())
+            track->removeObserver(*self->mediaStreamTrackObserver.get());
+
+        self->stream->removeObserver(*self->mediaStreamObserver);
+        self->stream = nullptr;
+    }
     GST_OBJECT_UNLOCK(self);
 
     g_clear_pointer(&self->uri, g_free);
@@ -283,7 +315,7 @@ static GstStateChangeReturn webkitMediaStreamSrcChangeState(GstElement* element,
 
         GST_OBJECT_LOCK(self);
         for (auto& track : self->stream->tracks())
-            track->removeObserver(*self->observer.get());
+            track->removeObserver(*self->mediaStreamTrackObserver.get());
         GST_OBJECT_UNLOCK(self);
     }
 
@@ -319,7 +351,8 @@ static void webkit_media_stream_src_class_init(WebKitMediaStreamSrcClass* klass)
 
 static void webkit_media_stream_src_init(WebKitMediaStreamSrc* self)
 {
-    self->observer = std::make_unique<WebKitMediaStreamTrackObserver>(self);
+    self->mediaStreamTrackObserver = std::make_unique<WebKitMediaStreamTrackObserver>(self);
+    self->mediaStreamObserver = std::make_unique<WebKitMediaStreamObserver>(self);
     self->flowCombiner = gst_flow_combiner_new();
     self->firstAudioBufferPts = GST_CLOCK_TIME_NONE;
     self->firstFramePts = GST_CLOCK_TIME_NONE;
@@ -420,7 +453,7 @@ static gboolean webkitMediaStreamSrcSetupSrc(WebKitMediaStreamSrc* self,
         });
 
     if (observe_track)
-        track->addObserver(*self->observer.get());
+        track->addObserver(*self->mediaStreamTrackObserver.get());
 
     gst_element_sync_state_with_parent(element);
     return TRUE;
@@ -451,37 +484,47 @@ static void webkitMediaStreamSrcPostStreamCollection(WebKitMediaStreamSrc* self,
         gst_message_new_stream_collection(GST_OBJECT(self), self->streamCollection.get()));
 }
 
-gboolean webkitMediaStreamSrcSetStream(WebKitMediaStreamSrc* self, MediaStreamPrivate* stream)
+static void webkitMediaStreamSrcAddTrack(WebKitMediaStreamSrc* self, MediaStreamTrackPrivate* track)
 {
-    g_return_val_if_fail(WEBKIT_IS_MEDIA_STREAM_SRC(self), FALSE);
+    if (track->type() == RealtimeMediaSource::Type::Audio)
+        webkitMediaStreamSrcSetupAppSrc(self, track, &self->audioSrc, &audioSrcTemplate);
+    else if (track->type() == RealtimeMediaSource::Type::Video)
+        webkitMediaStreamSrcSetupAppSrc(self, track, &self->videoSrc, &videoSrcTemplate);
+    else
+        GST_INFO("Unsupported track type: %d", static_cast<int>(track->type()));
+}
 
-    if (self->audioSrc) {
-        gst_element_set_state(self->audioSrc, GST_STATE_NULL);
-        gst_bin_remove(GST_BIN(self), self->audioSrc);
-        self->audioSrc = nullptr;
-    }
+static void webkitMediaStreamSrcRemoveTrackByType(WebKitMediaStreamSrc* self, RealtimeMediaSource::Type trackType)
+{
+    if (trackType == RealtimeMediaSource::Type::Audio) {
+        if (self->audioSrc) {
+            gst_element_set_state(self->audioSrc, GST_STATE_NULL);
+            gst_bin_remove(GST_BIN(self), self->audioSrc);
+            self->audioSrc = nullptr;
+        }
+    } else if (trackType == RealtimeMediaSource::Type::Video) {
+        if (self->videoSrc) {
+            gst_element_set_state(self->videoSrc, GST_STATE_NULL);
+            gst_bin_remove(GST_BIN(self), self->videoSrc);
+            self->videoSrc = nullptr;
+        }
+    } else
+        GST_INFO("Unsupported track type: %d", static_cast<int>(trackType));
+}
 
-    if (self->videoSrc) {
-        gst_element_set_state(self->videoSrc, GST_STATE_NULL);
-        gst_bin_remove(GST_BIN(self), self->videoSrc);
-        self->videoSrc = nullptr;
-    }
+bool webkitMediaStreamSrcSetStream(WebKitMediaStreamSrc* self, MediaStreamPrivate* stream)
+{
+    ASSERT(WEBKIT_IS_MEDIA_STREAM_SRC(self));
+
+    webkitMediaStreamSrcRemoveTrackByType(self, RealtimeMediaSource::Type::Audio);
+    webkitMediaStreamSrcRemoveTrackByType(self, RealtimeMediaSource::Type::Video);
 
     webkitMediaStreamSrcPostStreamCollection(self, stream);
 
     self->stream = stream;
-    for (auto& track : stream->tracks()) {
-        if (track->type() == RealtimeMediaSource::Type::Audio) {
-            webkitMediaStreamSrcSetupAppSrc(self, track.get(), &self->audioSrc,
-                &audioSrcTemplate);
-        } else if (track->type() == RealtimeMediaSource::Type::Video) {
-            webkitMediaStreamSrcSetupAppSrc(self, track.get(), &self->videoSrc,
-                &videoSrcTemplate);
-        } else {
-            GST_INFO("Unsuported track type: %d", (gint) track->type());
-            continue;
-        }
-    }
+    self->stream->addObserver(*self->mediaStreamObserver.get());
+    for (auto& track : stream->tracks())
+        webkitMediaStreamSrcAddTrack(self, track.get());
 
     return TRUE;
 }
index eb4112e..1599bae 100644 (file)
@@ -40,7 +40,7 @@ typedef struct _WebKitMediaStreamSrc WebKitMediaStreamSrc;
 #define WEBKIT_IS_MEDIA_STREAM_SRC(o) (G_TYPE_CHECK_INSTANCE_TYPE((o), WEBKIT_TYPE_MEDIA_STREAM_SRC))
 #define WEBKIT_TYPE_MEDIA_STREAM_SRC (webkit_media_stream_src_get_type())
 GType webkit_media_stream_src_get_type(void) G_GNUC_CONST;
-gboolean webkitMediaStreamSrcSetStream(WebKitMediaStreamSrc*, MediaStreamPrivate*);
+bool webkitMediaStreamSrcSetStream(WebKitMediaStreamSrc*, MediaStreamPrivate*);
 } // WebCore
 
 #endif // ENABLE(VIDEO) && ENABLE(MEDIA_STREAM) && USE(LIBWEBRTC)