Attachment 338981 Details for Bug 185071 – ROLLOUT of r231089

[patch] ROLLOUT of r231089

bug-185071-20180427060516.patch (text/plain), 17.00 KB, created by WebKit Commit Bot on 2018-04-27 03:05:16 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: WebKit Commit Bot

Created: 2018-04-27 03:05:16 PDT

Size: 17.00 KB

patch

obsolete

>Subversion Revision: 231090
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 624d5acb782255ceb167c9f54505c2005b4d8fe8..cf702c9c3955bace7e225c3fb8e6f216c6664281 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,18 @@
>+2018-04-27  Commit Queue  <commit-queue@webkit.org>
>+
>+        Unreviewed, rolling out r231089.
>+        https://bugs.webkit.org/show_bug.cgi?id=185071
>+
>+        Broke and made crash some WPE EME tests (Requested by calvaris
>+        on #webkit).
>+
>+        Reverted changeset:
>+
>+        "[EME][GStreamer] Move the decryptor from AppendPipeline to
>+        PlaybackPipeline."
>+        https://bugs.webkit.org/show_bug.cgi?id=181855
>+        https://trac.webkit.org/changeset/231089
>+
> 2018-04-27  Yacine Bandou  <yacine.bandou_ext@softathome.com>
> 
>         [EME][GStreamer] Move the decryptor from AppendPipeline to PlaybackPipeline.
>diff --git a/Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp
>index 8d1b84e6e1b9861e4e9d385ced1d90beaa3f15a5..33a330aaf6462c607bf5938b0f37689b9f7860be 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp
>+++ b/Source/WebCore/platform/graphics/gstreamer/eme/WebKitCommonEncryptionDecryptorGStreamer.cpp
>@@ -299,6 +299,23 @@ static gboolean webkitMediaCommonEncryptionDecryptSinkEventHandler(GstBaseTransf
>     gboolean result = FALSE;
> 
>     switch (GST_EVENT_TYPE(event)) {
>+    case GST_EVENT_PROTECTION: {
>+        const char* systemId = nullptr;
>+
>+        gst_event_parse_protection(event, &systemId, nullptr, nullptr);
>+        GST_TRACE_OBJECT(self, "received protection event for %s", systemId);
>+
>+        if (!g_strcmp0(systemId, klass->protectionSystemId)) {
>+            GST_DEBUG_OBJECT(self, "sending protection event to the pipeline");
>+            gst_element_post_message(GST_ELEMENT(self),
>+                gst_message_new_element(GST_OBJECT(self),
>+                    gst_structure_new("drm-key-needed", "event", GST_TYPE_EVENT, event, nullptr)));
>+        }
>+
>+        gst_event_unref(event);
>+        result = TRUE;
>+        break;
>+    }
>     case GST_EVENT_CUSTOM_DOWNSTREAM_OOB: {
>         if (klass->handleKeyResponse(self, event)) {
>             GST_DEBUG_OBJECT(self, "key received");
>diff --git a/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp b/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp
>index 691f0c8ac593d4ee49ed8448a83129074d1e996c..c2508c75c9f95c6fca489be9a9b6408ed8f93a2a 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp
>+++ b/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp
>@@ -55,6 +55,8 @@ static const char* dumpAppendState(AppendPipeline::AppendState appendState)
>         return "NotStarted";
>     case AppendPipeline::AppendState::Ongoing:
>         return "Ongoing";
>+    case AppendPipeline::AppendState::KeyNegotiation:
>+        return "KeyNegotiation";
>     case AppendPipeline::AppendState::DataStarve:
>         return "DataStarve";
>     case AppendPipeline::AppendState::Sampling:
>@@ -91,6 +93,13 @@ static void appendPipelineApplicationMessageCallback(GstBus*, GstMessage* messag
>     appendPipeline->handleApplicationMessage(message);
> }
> 
>+#if ENABLE(ENCRYPTED_MEDIA)
>+static void appendPipelineElementMessageCallback(GstBus*, GstMessage* message, AppendPipeline* appendPipeline)
>+{
>+    appendPipeline->handleElementMessage(message);
>+}
>+#endif
>+
> static void appendPipelineStateChangeMessageCallback(GstBus*, GstMessage* message, AppendPipeline* appendPipeline)
> {
>     appendPipeline->handleStateChangeMessage(message);
>@@ -122,6 +131,9 @@ AppendPipeline::AppendPipeline(Ref<MediaSourceClientGStreamerMSE> mediaSourceCli
> 
>     g_signal_connect(m_bus.get(), "sync-message::need-context", G_CALLBACK(appendPipelineNeedContextMessageCallback), this);
>     g_signal_connect(m_bus.get(), "message::application", G_CALLBACK(appendPipelineApplicationMessageCallback), this);
>+#if ENABLE(ENCRYPTED_MEDIA)
>+    g_signal_connect(m_bus.get(), "message::element", G_CALLBACK(appendPipelineElementMessageCallback), this);
>+#endif
>     g_signal_connect(m_bus.get(), "message::state-changed", G_CALLBACK(appendPipelineStateChangeMessageCallback), this);
> 
>     // We assign the created instances here instead of adoptRef() because gst_bin_add_many()
>@@ -263,6 +275,9 @@ void AppendPipeline::handleNeedContextSyncMessage(GstMessage* message)
>     const gchar* contextType = nullptr;
>     gst_message_parse_context_type(message, &contextType);
>     GST_TRACE("context type: %s", contextType);
>+    if (!g_strcmp0(contextType, "drm-preferred-decryption-system-id")
>+        && m_appendState != AppendPipeline::AppendState::KeyNegotiation)
>+        setAppendState(AppendPipeline::AppendState::KeyNegotiation);
> 
>     // MediaPlayerPrivateGStreamerBase will take care of setting up encryption.
>     if (m_playerPrivate)
>@@ -314,6 +329,25 @@ void AppendPipeline::handleApplicationMessage(GstMessage* message)
>     ASSERT_NOT_REACHED();
> }
> 
>+#if ENABLE(ENCRYPTED_MEDIA)
>+void AppendPipeline::handleElementMessage(GstMessage* message)
>+{
>+    ASSERT(WTF::isMainThread());
>+
>+    const GstStructure* structure = gst_message_get_structure(message);
>+    GST_TRACE("%s message from %s", gst_structure_get_name(structure), GST_MESSAGE_SRC_NAME(message));
>+    if (m_playerPrivate && gst_structure_has_name(structure, "drm-key-needed")) {
>+        if (m_appendState != AppendPipeline::AppendState::KeyNegotiation)
>+            setAppendState(AppendPipeline::AppendState::KeyNegotiation);
>+
>+        GST_DEBUG("sending drm-key-needed message from %s to the player", GST_MESSAGE_SRC_NAME(message));
>+        GRefPtr<GstEvent> event;
>+        gst_structure_get(structure, "event", GST_TYPE_EVENT, &event.outPtr(), nullptr);
>+        m_playerPrivate->handleProtectionEvent(event.get());
>+    }
>+}
>+#endif
>+
> void AppendPipeline::handleStateChangeMessage(GstMessage* message)
> {
>     ASSERT(WTF::isMainThread());
>@@ -339,7 +373,7 @@ void AppendPipeline::handleAppsrcNeedDataReceived()
>         return;
>     }
> 
>-    ASSERT(m_appendState == AppendState::Ongoing || m_appendState == AppendState::Sampling);
>+    ASSERT(m_appendState == AppendState::KeyNegotiation || m_appendState == AppendState::Ongoing || m_appendState == AppendState::Sampling);
>     ASSERT(!m_appsrcNeedDataReceived);
> 
>     GST_TRACE("received need-data from appsrc");
>@@ -397,7 +431,8 @@ void AppendPipeline::setAppendState(AppendState newAppendState)
>     // NotStarted-->Ongoing-->DataStarve-->NotStarted
>     //           |         |            `->Aborting-->NotStarted
>     //           |         `->Sampling-Â·Â·Â·->Sampling-->LastSample-->NotStarted
>-    //           |                                               `->Aborting-->NotStarted
>+    //           |         |                                     `->Aborting-->NotStarted
>+    //           |         `->KeyNegotiation-->Ongoing-->[...]
>     //           `->Aborting-->NotStarted
>     AppendState oldAppendState = m_appendState;
>     AppendState nextAppendState = AppendState::Invalid;
>@@ -433,8 +468,19 @@ void AppendPipeline::setAppendState(AppendState newAppendState)
>             break;
>         }
>         break;
>+    case AppendState::KeyNegotiation:
>+        switch (newAppendState) {
>+        case AppendState::Ongoing:
>+        case AppendState::Invalid:
>+            ok = true;
>+            break;
>+        default:
>+            break;
>+        }
>+        break;
>     case AppendState::Ongoing:
>         switch (newAppendState) {
>+        case AppendState::KeyNegotiation:
>         case AppendState::Sampling:
>         case AppendState::Invalid:
>             ok = true;
>@@ -536,7 +582,18 @@ void AppendPipeline::parseDemuxerSrcPadCaps(GstCaps* demuxerSrcPadCaps)
> 
>     m_demuxerSrcPadCaps = adoptGRef(demuxerSrcPadCaps);
>     m_streamType = WebCore::MediaSourceStreamTypeGStreamer::Unknown;
>-
>+#if ENABLE(ENCRYPTED_MEDIA)
>+    if (areEncryptedCaps(m_demuxerSrcPadCaps.get())) {
>+        // Any previous decryptor should have been removed from the pipeline by disconnectFromAppSinkFromStreamingThread()
>+        ASSERT(!m_decryptor);
>+        GstStructure* structure = gst_caps_get_structure(m_demuxerSrcPadCaps.get(), 0);
>+        m_decryptor = GStreamerEMEUtilities::createDecryptor(gst_structure_get_string(structure, "protection-system"));
>+        if (!m_decryptor) {
>+            GST_ERROR("decryptor not found for caps: %" GST_PTR_FORMAT, m_demuxerSrcPadCaps.get());
>+            return;
>+        }
>+    }
>+#endif
>     const char* originalMediaType = capsMediaType(m_demuxerSrcPadCaps.get());
>     if (!MediaPlayerPrivateGStreamerMSE::supportsCodec(originalMediaType)) {
>             m_presentationSize = WebCore::FloatSize();
>@@ -616,6 +673,10 @@ void AppendPipeline::appsinkNewSample(GstSample* sample)
>     {
>         LockHolder locker(m_newSampleLock);
> 
>+        // If we were in KeyNegotiation but samples are coming, assume we're already OnGoing
>+        if (m_appendState == AppendState::KeyNegotiation)
>+            setAppendState(AppendState::Ongoing);
>+
>         // Ignore samples if we're not expecting them. Refuse processing if we're in Invalid state.
>         if (m_appendState != AppendState::Ongoing && m_appendState != AppendState::Sampling) {
>             GST_WARNING("Unexpected sample, appendState=%s", dumpAppendState(m_appendState));
>@@ -948,10 +1009,28 @@ void AppendPipeline::connectDemuxerSrcPadToAppsinkFromAnyThread(GstPad* demuxerS
>             currentSrcPad = parserSrcPad;
>         }
> 
>+#if ENABLE(ENCRYPTED_MEDIA)
>+        if (m_decryptor) {
>+            gst_object_ref(m_decryptor.get());
>+            gst_bin_add(GST_BIN(m_pipeline.get()), m_decryptor.get());
>+            gst_element_sync_state_with_parent(m_decryptor.get());
>+
>+            GRefPtr<GstPad> decryptorSinkPad = adoptGRef(gst_element_get_static_pad(m_decryptor.get(), "sink"));
>+            GRefPtr<GstPad> decryptorSrcPad = adoptGRef(gst_element_get_static_pad(m_decryptor.get(), "src"));
>+
>+            gst_pad_link(currentSrcPad.get(), decryptorSinkPad.get());
>+            currentSrcPad = decryptorSrcPad;
>+        }
>+#endif
>+
>         gst_pad_link(currentSrcPad.get(), appsinkSinkPad.get());
> 
>         gst_element_sync_state_with_parent(m_appsink.get());
> 
>+#if ENABLE(ENCRYPTED_MEDIA)
>+        if (m_pendingDecryptionStructure)
>+            dispatchPendingDecryptionStructure();
>+#endif
>         gst_element_set_state(m_pipeline.get(), GST_STATE_PAUSED);
>         gst_element_sync_state_with_parent(m_appsink.get());
> 
>@@ -1030,6 +1109,14 @@ void AppendPipeline::disconnectDemuxerSrcPadFromAppsinkFromAnyThread(GstPad*)
> 
>     GST_DEBUG("Disconnecting appsink");
> 
>+#if ENABLE(ENCRYPTED_MEDIA)
>+    if (m_decryptor) {
>+        gst_element_set_state(m_decryptor.get(), GST_STATE_NULL);
>+        gst_bin_remove(GST_BIN(m_pipeline.get()), m_decryptor.get());
>+        m_decryptor = nullptr;
>+    }
>+#endif
>+
>     if (m_parser) {
>         gst_element_set_state(m_parser.get(), GST_STATE_NULL);
>         gst_bin_remove(GST_BIN(m_pipeline.get()), m_parser.get());
>@@ -1039,6 +1126,35 @@ void AppendPipeline::disconnectDemuxerSrcPadFromAppsinkFromAnyThread(GstPad*)
>     GST_DEBUG_BIN_TO_DOT_FILE_WITH_TS(GST_BIN(m_pipeline.get()), GST_DEBUG_GRAPH_SHOW_ALL, "pad-removed-after");
> }
> 
>+#if ENABLE(ENCRYPTED_MEDIA)
>+void AppendPipeline::dispatchPendingDecryptionStructure()
>+{
>+    ASSERT(m_decryptor);
>+    ASSERT(m_pendingDecryptionStructure);
>+    ASSERT(m_appendState == AppendState::KeyNegotiation);
>+    GST_TRACE("dispatching key to append pipeline %p", this);
>+
>+    // Release the m_pendingDecryptionStructure object since
>+    // gst_event_new_custom() takes over ownership of it.
>+    gst_element_send_event(m_pipeline.get(), gst_event_new_custom(GST_EVENT_CUSTOM_DOWNSTREAM_OOB, m_pendingDecryptionStructure.release()));
>+
>+    setAppendState(AppendState::Ongoing);
>+}
>+
>+void AppendPipeline::dispatchDecryptionStructure(GUniquePtr<GstStructure>&& structure)
>+{
>+    if (m_appendState == AppendState::KeyNegotiation) {
>+        GST_TRACE("append pipeline %p in key negotiation", this);
>+        m_pendingDecryptionStructure = WTFMove(structure);
>+        if (m_decryptor)
>+            dispatchPendingDecryptionStructure();
>+        else
>+            GST_TRACE("no decryptor yet, waiting for it");
>+    } else
>+        GST_TRACE("append pipeline %p not in key negotiation", this);
>+}
>+#endif
>+
> static void appendPipelineAppsinkCapsChanged(GObject* appsinkPad, GParamSpec*, AppendPipeline* appendPipeline)
> {
>     GstStructure* structure = gst_structure_new_empty("appsink-caps-changed");
>diff --git a/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.h b/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.h
>index 995c12975d94ec4c007ffeb2d79cfa7a45f53894..83304661b482a9cb927d3c70ee0a749d8953240e 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.h
>+++ b/Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.h
>@@ -42,7 +42,7 @@ struct PadProbeInformation {
> 
> class AppendPipeline : public ThreadSafeRefCounted<AppendPipeline> {
> public:
>-    enum class AppendState { Invalid, NotStarted, Ongoing, DataStarve, Sampling, LastSample, Aborting };
>+    enum class AppendState { Invalid, NotStarted, Ongoing, KeyNegotiation, DataStarve, Sampling, LastSample, Aborting };
> 
>     AppendPipeline(Ref<MediaSourceClientGStreamerMSE>, Ref<SourceBufferPrivateGStreamer>, MediaPlayerPrivateGStreamerMSE&);
>     virtual ~AppendPipeline();
>@@ -50,6 +50,9 @@ public:
>     void handleNeedContextSyncMessage(GstMessage*);
>     void handleApplicationMessage(GstMessage*);
>     void handleStateChangeMessage(GstMessage*);
>+#if ENABLE(ENCRYPTED_MEDIA)
>+    void handleElementMessage(GstMessage*);
>+#endif
> 
>     gint id();
>     AppendState appendState() { return m_appendState; }
>@@ -57,6 +60,9 @@ public:
> 
>     GstFlowReturn handleNewAppsinkSample(GstElement*);
>     GstFlowReturn pushNewBuffer(GstBuffer*);
>+#if ENABLE(ENCRYPTED_MEDIA)
>+    void dispatchDecryptionStructure(GUniquePtr<GstStructure>&&);
>+#endif
> 
>     // Takes ownership of caps.
>     void parseDemuxerSrcPadCaps(GstCaps*);
>@@ -92,6 +98,9 @@ private:
>     void handleAppsrcNeedDataReceived();
>     void removeAppsrcDataLeavingProbe();
>     void setAppsrcDataLeavingProbe();
>+#if ENABLE(ENCRYPTED_MEDIA)
>+    void dispatchPendingDecryptionStructure();
>+#endif
> 
>     Ref<MediaSourceClientGStreamerMSE> m_mediaSourceClient;
>     Ref<SourceBufferPrivateGStreamer> m_sourceBufferPrivate;
>@@ -109,6 +118,9 @@ private:
>     GRefPtr<GstElement> m_appsrc;
>     GRefPtr<GstElement> m_demux;
>     GRefPtr<GstElement> m_parser; // Optional.
>+#if ENABLE(ENCRYPTED_MEDIA)
>+    GRefPtr<GstElement> m_decryptor;
>+#endif
>     // The demuxer has one src stream only, so only one appsink is needed and linked to it.
>     GRefPtr<GstElement> m_appsink;
> 
>@@ -143,6 +155,9 @@ private:
>     RefPtr<WebCore::TrackPrivateBase> m_track;
> 
>     GRefPtr<GstBuffer> m_pendingBuffer;
>+#if ENABLE(ENCRYPTED_MEDIA)
>+    GUniquePtr<GstStructure> m_pendingDecryptionStructure;
>+#endif
> };
> 
> } // namespace WebCore.
>diff --git a/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp b/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp
>index 04363258c3d5d096313b9854e1952c33247ef55e..a30811f658223467babb2af6501eb08c169364eb 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp
>+++ b/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp
>@@ -955,7 +955,8 @@ void MediaPlayerPrivateGStreamerMSE::attemptToDecryptWithInstance(CDMInstance& i
>         gst_structure_set_value(structure.get(), "key-ids", &keyIDList);
>         gst_structure_set_value(structure.get(), "key-values", &keyValueList);
> 
>-        gst_element_send_event(m_playbackPipeline->pipeline(), gst_event_new_custom(GST_EVENT_CUSTOM_DOWNSTREAM_OOB, gst_structure_copy(structure.get())));
>+        for (auto it : m_appendPipelinesMap)
>+            it.value->dispatchDecryptionStructure(GUniquePtr<GstStructure>(gst_structure_copy(structure.get())));
>     }
> }
> #endif
>diff --git a/Source/WebCore/platform/graphics/gstreamer/mse/PlaybackPipeline.cpp b/Source/WebCore/platform/graphics/gstreamer/mse/PlaybackPipeline.cpp
>index 2b699718a3e1c7e2edc87395ff88b55783b4c376..06d5862c5063dbde8d0295c0a0e69c395085e576 100644
>--- a/Source/WebCore/platform/graphics/gstreamer/mse/PlaybackPipeline.cpp
>+++ b/Source/WebCore/platform/graphics/gstreamer/mse/PlaybackPipeline.cpp
>@@ -180,9 +180,7 @@ void PlaybackPipeline::attachTrack(RefPtr<SourceBufferPrivateGStreamer> sourceBu
> 
>     GUniquePtr<gchar> parserBinName(g_strdup_printf("streamparser%u", padId));
> 
>-    if (areEncryptedCaps(caps)) {
>-        GST_DEBUG("It's encrypted content, parsers are not needed before decrypting the content");
>-    } else if (!g_strcmp0(mediaType, "video/x-h264")) {
>+    if (!g_strcmp0(mediaType, "video/x-h264")) {
>         GRefPtr<GstCaps> filterCaps = adoptGRef(gst_caps_new_simple("video/x-h264", "alignment", G_TYPE_STRING, "au", nullptr));
>         GstElement* capsfilter = gst_element_factory_make("capsfilter", nullptr);
>         g_object_set(capsfilter, "caps", filterCaps.get(), nullptr);

Actions: View | Formatted Diff | Diff

Attachments on bug 185071: 338981