Changeset 243199 in webkit

Timestamp:

Mar 20, 2019 6:11:08 AM (5 years ago)

Author:

aboya@igalia.com

Message:

[MSE][GStreamer] Fix handling of resolution changes in AppendPipeline
​https://bugs.webkit.org/show_bug.cgi?id=195855

Reviewed by Xabier Rodriguez-Calvar.

Source/WebCore:

MediaSample instances produced by the AppendPipeline were not
accounting for resolution changes. The causes of this are twofold:

1) m_presentationSize is set by connectDemuxerSrcPadToAppsink() (by
calling parseDemuxerSrcPadCaps()), but not by appsinkCapsChanged().

2) appsinkCapsChanged() was being called in the main thread as an
asynchronous task. In consequence, even if m_presentationSize is set
there, many samples with the new resolution would still be wrapped in
a MediaSampleGStreamer using the old resolution by the main thread
running consumeAppsinkAvailableSamples() before appsinkCapsChanged()
is dispatched.

This patch fixes these problems by updating m_presentationSize in
appsinkCapsChanged() and making the streaming thread block until the
main thread has dispatched appsinkCapsChanged(). This way the handling
of caps changes is serialized with the handling of frames.

Test: media/media-source/media-source-samples-resolution-change.html

• platform/graphics/gstreamer/mse/AppendPipeline.cpp:

(WebCore::AppendPipeline::AppendPipeline):
(WebCore::AppendPipeline::appsinkCapsChanged):

LayoutTests:

• media/media-source/content/test-green-6s-320x240.mp4: Added.
• media/media-source/content/test-red-3s-480x360.mp4: Added.
• media/media-source/media-source-samples-resolution-change-expected.txt: Added.
• media/media-source/media-source-samples-resolution-change.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/media/media-source/content/test-green-6s-320x240.mp4 (added)
• LayoutTests/media/media-source/content/test-red-3s-480x360.mp4 (added)
• LayoutTests/media/media-source/media-source-samples-resolution-change-expected.txt (added)
• LayoutTests/media/media-source/media-source-samples-resolution-change.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-03-20  Alicia Boya García  <aboya@igalia.com>
+
+        [MSE][GStreamer] Fix handling of resolution changes in AppendPipeline
+        https://bugs.webkit.org/show_bug.cgi?id=195855
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        * media/media-source/content/test-green-6s-320x240.mp4: Added.
+        * media/media-source/content/test-red-3s-480x360.mp4: Added.
+        * media/media-source/media-source-samples-resolution-change-expected.txt: Added.
+        * media/media-source/media-source-samples-resolution-change.html: Added.
+
 2019-03-20  Joanmarie Diggs  <jdiggs@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-03-20  Alicia Boya García  <aboya@igalia.com>
+
+        [MSE][GStreamer] Fix handling of resolution changes in AppendPipeline
+        https://bugs.webkit.org/show_bug.cgi?id=195855
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        MediaSample instances produced by the AppendPipeline were not
+        accounting for resolution changes. The causes of this are twofold:
+
+        1) m_presentationSize is set by connectDemuxerSrcPadToAppsink() (by
+        calling parseDemuxerSrcPadCaps()), but not by appsinkCapsChanged().
+
+        2) appsinkCapsChanged() was being called in the main thread as an
+        asynchronous task. In consequence, even if m_presentationSize is set
+        there, many samples with the new resolution would still be wrapped in
+        a MediaSampleGStreamer using the old resolution by the main thread
+        running consumeAppsinkAvailableSamples() before appsinkCapsChanged()
+        is dispatched.
+
+        This patch fixes these problems by updating m_presentationSize in
+        appsinkCapsChanged() and making the streaming thread block until the
+        main thread has dispatched appsinkCapsChanged(). This way the handling
+        of caps changes is serialized with the handling of frames.
+
+        Test: media/media-source/media-source-samples-resolution-change.html
+
+        * platform/graphics/gstreamer/mse/AppendPipeline.cpp:
+        (WebCore::AppendPipeline::AppendPipeline):
+        (WebCore::AppendPipeline::appsinkCapsChanged):
+
 2019-03-20  Joanmarie Diggs  <jdiggs@igalia.com>
 

trunk/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp

@@ -173 +173 @@
         }
 
-        appendPipeline->m_taskQueue.enqueueTask([appendPipeline]() {
+        // The streaming thread has just received a new caps and is about to let samples using the
+        // new caps flow. Let's block it until the main thread has consumed the samples with the old
+        // caps and has processed the caps change.
+        appendPipeline->m_taskQueue.enqueueTaskAndWait<AbortableTaskQueue::Void>([appendPipeline]() {
             appendPipeline->appsinkCapsChanged();
+            return AbortableTaskQueue::Void();
         });
     }), this);
@@ -408 +412 @@
     ASSERT(isMainThread());
 
+    // Consume any pending samples with the previous caps.
+    consumeAppsinkAvailableSamples();
+
     GRefPtr<GstPad> pad = adoptGRef(gst_element_get_static_pad(m_appsink.get(), "sink"));
     GRefPtr<GstCaps> caps = adoptGRef(gst_pad_get_current_caps(pad.get()));
@@ -413 +420 @@
     if (!caps)
         return;
+
+    if (doCapsHaveType(caps.get(), GST_VIDEO_CAPS_TYPE_PREFIX)) {
+        Optional<FloatSize> size = getVideoResolutionFromCaps(caps.get());
+        if (size.hasValue())
+            m_presentationSize = size.value();
+    }
 
     // This means that we're right after a new track has appeared. Otherwise, it's a caps change inside the same track.