[GStreamer] Handle changes in the "drm-preferred-decryption-system-id" NEED_CONTEXT...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 28 May 2018 08:18:35 +0000 (08:18 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 28 May 2018 08:18:35 +0000 (08:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185948

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

Source/WebCore:

- The "stream-encryption-systems" field of the "drm-preferred-decryption-system-id" query was renamed to
  "avalaible-stream-encryption-systems"
- It can now be NULL, meaning there is no decryptor avalaible.

Tests: imported/w3c/web-platform-tests/encrypted-media/clearkey-mp4-*

* platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
(WebCore::extractEventsAndSystemsFromMessage): Handle NULL value for "avalaible-stream-encryption-systems",
moved some code to make the order of the Arrays in the pair clearer.

Tools:

When the patch introducing gst_protection_filter_systems_by_available_decryptors was merged, we changed sensibly its behaviour
concerning the way empty results is represented (now returning NULL instead of an array of 1(NULL) element) and this change was not
properly taken into account in the qtdemux patch that uses it and got upstreamed. Those 3 new patches fixe that and change the
NEED_CONTEXT field names to clarify the meaning of "stream-encryption-systems".

* gstreamer/jhbuild.modules:
* gstreamer/patches/gst-plugins-good-0001-qtdemux-Do-not-run-the-preferred-decryptor-context-q.patch: Added. Merged upstream as 3e063703b3a51b8aaa7f75f36c4660c583a60e93
* gstreamer/patches/gst-plugins-good-0002-qtdemux-Do-not-unref-a-NULL-stream_tags.patch: Added. Merged upstream as 43a540b1cd9f162d3dae5d50e36703dfaf558a3e
* gstreamer/patches/gst-plugins-good-0003-qtdemux-Clarify-field-name-about-stream-encryption-s.patch: Added.

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

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp
Tools/ChangeLog
Tools/gstreamer/jhbuild.modules
Tools/gstreamer/patches/gst-plugins-good-0001-qtdemux-Do-not-run-the-preferred-decryptor-context-q.patch [new file with mode: 0644]
Tools/gstreamer/patches/gst-plugins-good-0002-qtdemux-Do-not-unref-a-NULL-stream_tags.patch [new file with mode: 0644]
Tools/gstreamer/patches/gst-plugins-good-0003-qtdemux-Clarify-field-name-about-stream-encryption-s.patch [new file with mode: 0644]

index c9252d6..b4a593d 100644 (file)
@@ -1,3 +1,20 @@
+2018-05-28  Thibault Saunier  <tsaunier@igalia.com>
+
+        [GStreamer] Handle changes in the "drm-preferred-decryption-system-id" NEED_CONTEXT message.
+        https://bugs.webkit.org/show_bug.cgi?id=185948
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        - The "stream-encryption-systems" field of the "drm-preferred-decryption-system-id" query was renamed to
+          "avalaible-stream-encryption-systems"
+        - It can now be NULL, meaning there is no decryptor avalaible.
+
+        Tests: imported/w3c/web-platform-tests/encrypted-media/clearkey-mp4-*
+
+        * platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
+        (WebCore::extractEventsAndSystemsFromMessage): Handle NULL value for "avalaible-stream-encryption-systems",
+        moved some code to make the order of the Arrays in the pair clearer.
+
 2018-05-27  Dan Bernstein  <mitz@apple.com>
 
         Reverted the changes made for https://webkit.org/b/186016
index 0d9b3e5..1eb4696 100644 (file)
@@ -301,22 +301,23 @@ static std::pair<Vector<GRefPtr<GstEvent>>, Vector<String>> extractEventsAndSyst
 {
     const GstStructure* structure = gst_message_get_structure(message);
 
-    const GValue* streamEncryptionAllowedSystemsValue = gst_structure_get_value(structure, "stream-encryption-systems");
-    ASSERT(streamEncryptionAllowedSystemsValue && G_VALUE_HOLDS(streamEncryptionAllowedSystemsValue, G_TYPE_STRV));
-    const char** streamEncryptionAllowedSystems = reinterpret_cast<const char**>(g_value_get_boxed(streamEncryptionAllowedSystemsValue));
-    ASSERT(streamEncryptionAllowedSystems);
-    Vector<String> streamEncryptionAllowedSystemsVector;
-    unsigned i;
-    for (i = 0; streamEncryptionAllowedSystems[i]; ++i)
-        streamEncryptionAllowedSystemsVector.append(streamEncryptionAllowedSystems[i]);
-
     const GValue* streamEncryptionEventsList = gst_structure_get_value(structure, "stream-encryption-events");
     ASSERT(streamEncryptionEventsList && GST_VALUE_HOLDS_LIST(streamEncryptionEventsList));
     unsigned streamEncryptionEventsListSize = gst_value_list_get_size(streamEncryptionEventsList);
     Vector<GRefPtr<GstEvent>> streamEncryptionEventsVector;
+
+    unsigned i;
     for (i = 0; i < streamEncryptionEventsListSize; ++i)
         streamEncryptionEventsVector.append(GRefPtr<GstEvent>(static_cast<GstEvent*>(g_value_get_boxed(gst_value_list_get_value(streamEncryptionEventsList, i)))));
 
+    Vector<String> streamEncryptionAllowedSystemsVector;
+    const GValue* streamEncryptionAllowedSystemsValue = gst_structure_get_value(structure, "avalaible-stream-encryption-systems");
+    const char** streamEncryptionAllowedSystems = reinterpret_cast<const char**>(g_value_get_boxed(streamEncryptionAllowedSystemsValue));
+    if (streamEncryptionAllowedSystems) {
+        for (i = 0; streamEncryptionAllowedSystems[i]; ++i)
+            streamEncryptionAllowedSystemsVector.append(streamEncryptionAllowedSystems[i]);
+    }
+
     return std::make_pair(streamEncryptionEventsVector, streamEncryptionAllowedSystemsVector);
 }
 #endif
index 3c46efb..ab7261f 100644 (file)
@@ -1,3 +1,20 @@
+2018-05-28  Thibault Saunier  <tsaunier@igalia.com>
+
+        [GStreamer] Handle changes in the "drm-preferred-decryption-system-id" NEED_CONTEXT message.
+        https://bugs.webkit.org/show_bug.cgi?id=185948
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        When the patch introducing gst_protection_filter_systems_by_available_decryptors was merged, we changed sensibly its behaviour
+        concerning the way empty results is represented (now returning NULL instead of an array of 1(NULL) element) and this change was not
+        properly taken into account in the qtdemux patch that uses it and got upstreamed. Those 3 new patches fixe that and change the
+        NEED_CONTEXT field names to clarify the meaning of "stream-encryption-systems".
+
+        * gstreamer/jhbuild.modules:
+        * gstreamer/patches/gst-plugins-good-0001-qtdemux-Do-not-run-the-preferred-decryptor-context-q.patch: Added. Merged upstream as 3e063703b3a51b8aaa7f75f36c4660c583a60e93
+        * gstreamer/patches/gst-plugins-good-0002-qtdemux-Do-not-unref-a-NULL-stream_tags.patch: Added. Merged upstream as 43a540b1cd9f162d3dae5d50e36703dfaf558a3e
+        * gstreamer/patches/gst-plugins-good-0003-qtdemux-Clarify-field-name-about-stream-encryption-s.patch: Added.
+
 2018-05-27  Dan Bernstein  <mitz@apple.com>
 
         Reverted the changes made for https://webkit.org/b/186016
index 4b91a84..c42d426 100644 (file)
@@ -69,6 +69,9 @@
     <branch hash="sha:34ec062ddb766a32377532e039781f4a16fbc3e8b449e642605bacab26a99172" module="gst-plugins-good/gst-plugins-good-${version}.tar.xz" repo="gstreamer" version="1.14.1">
       <patch file="gst-plugins-good-0002-qtdemux-add-context-for-a-preferred-protection.patch" strip="1" /> <!-- Merged as ee4b45da24cb7465b416c230597f8efc7b2c45cb -->
       <patch file="gst-plugins-good-0003-qtdemux-also-push-buffers-without-encryption-info-in.patch" strip="1" /> <!-- Merged as 844423ff99e281fc831303b92861ed43ce5c1518 -->
+      <patch file="gst-plugins-good-0001-qtdemux-Do-not-run-the-preferred-decryptor-context-q.patch" strip="1" /> <!-- Merged as 3e063703b3a51b8aaa7f75f36c4660c583a60e93 -->
+      <patch file="gst-plugins-good-0002-qtdemux-Do-not-unref-a-NULL-stream_tags.patch" strip="1" /> <!-- Merged as 43a540b1cd9f162d3dae5d50e36703dfaf558a3e -->
+      <patch file="gst-plugins-good-0003-qtdemux-Clarify-field-name-about-stream-encryption-s.patch" strip="1" /> <!-- Merged in master, scheduled for 1.16.0 -->
     </branch>
   </autotools>
 
diff --git a/Tools/gstreamer/patches/gst-plugins-good-0001-qtdemux-Do-not-run-the-preferred-decryptor-context-q.patch b/Tools/gstreamer/patches/gst-plugins-good-0001-qtdemux-Do-not-run-the-preferred-decryptor-context-q.patch
new file mode 100644 (file)
index 0000000..0b7ab97
--- /dev/null
@@ -0,0 +1,33 @@
+From 3e063703b3a51b8aaa7f75f36c4660c583a60e93 Mon Sep 17 00:00:00 2001
+From: Thibault Saunier <tsaunier@igalia.com>
+Date: Fri, 25 May 2018 10:17:29 +0200
+Subject: [PATCH 1/3] qtdemux: Do not run the preferred decryptor context query
+ if no decryptor avalaible
+
+Ultimately this avoids a segfault as the code expect a non NULL array
+here.
+---
+ gst/isomp4/qtdemux.c | 7 +++++++
+ 1 file changed, 7 insertions(+)
+
+diff --git a/gst/isomp4/qtdemux.c b/gst/isomp4/qtdemux.c
+index 3e8ce7860..ec4a8adfd 100644
+--- a/gst/isomp4/qtdemux.c
++++ b/gst/isomp4/qtdemux.c
+@@ -8056,6 +8056,13 @@ gst_qtdemux_request_protection_context (GstQTDemux * qtdemux,
+   g_ptr_array_add (qtdemux->protection_system_ids, NULL);
+   filtered_sys_ids = gst_protection_filter_systems_by_available_decryptors (
+       (const gchar **) qtdemux->protection_system_ids->pdata);
++
++  if (!filtered_sys_ids) {
++    GST_INFO_OBJECT (element,
++        "No avalaible decryptor, not worth asking the user to choose.");
++    return;
++  }
++
+   g_ptr_array_remove_index (qtdemux->protection_system_ids,
+       qtdemux->protection_system_ids->len - 1);
+   GST_TRACE_OBJECT (qtdemux, "detected %u protection systems, we have "
+-- 
+2.17.0
+
diff --git a/Tools/gstreamer/patches/gst-plugins-good-0002-qtdemux-Do-not-unref-a-NULL-stream_tags.patch b/Tools/gstreamer/patches/gst-plugins-good-0002-qtdemux-Do-not-unref-a-NULL-stream_tags.patch
new file mode 100644 (file)
index 0000000..ca4d12a
--- /dev/null
@@ -0,0 +1,30 @@
+From 43a540b1cd9f162d3dae5d50e36703dfaf558a3e Mon Sep 17 00:00:00 2001
+From: Thibault Saunier <tsaunier@igalia.com>
+Date: Fri, 25 May 2018 10:49:21 +0200
+Subject: [PATCH 2/3] qtdemux: Do not unref a NULL stream_tags
+
+stream->stream_tags is reset to NULL once we expose the stream and
+these have been consumed, we need to check that when cleaning up
+the stream.
+---
+ gst/isomp4/qtdemux.c | 4 +++-
+ 1 file changed, 3 insertions(+), 1 deletion(-)
+
+diff --git a/gst/isomp4/qtdemux.c b/gst/isomp4/qtdemux.c
+index ec4a8adfd..4116f4c69 100644
+--- a/gst/isomp4/qtdemux.c
++++ b/gst/isomp4/qtdemux.c
+@@ -2600,7 +2600,9 @@ gst_qtdemux_stream_clear (QtDemuxStream * stream)
+     entry->sparse = FALSE;
+   }
+-  gst_tag_list_unref (stream->stream_tags);
++  if (stream->stream_tags)
++    gst_tag_list_unref (stream->stream_tags);
++
+   stream->stream_tags = gst_tag_list_new_empty ();
+   gst_tag_list_set_scope (stream->stream_tags, GST_TAG_SCOPE_STREAM);
+   g_free (stream->redirect_uri);
+-- 
+2.17.0
+
diff --git a/Tools/gstreamer/patches/gst-plugins-good-0003-qtdemux-Clarify-field-name-about-stream-encryption-s.patch b/Tools/gstreamer/patches/gst-plugins-good-0003-qtdemux-Clarify-field-name-about-stream-encryption-s.patch
new file mode 100644 (file)
index 0000000..d5901e1
--- /dev/null
@@ -0,0 +1,57 @@
+From 3de36f28377398e07ad8b90366db9b6b7544c09a Mon Sep 17 00:00:00 2001
+From: Thibault Saunier <tsaunier@igalia.com>
+Date: Fri, 25 May 2018 12:28:04 +0200
+Subject: [PATCH 3/3] qtdemux: Clarify field name about
+ stream-encryption-system
+
+This field is actually only informatory but the user can potentially
+choose something else. EME tests in WebKit testsuite actually doesn't
+take it into and force another encryption system to be used, and expects
+to be given the occasion to be so.
+
+This basically also reverts 3e063703b3a51b8aaa7f75f36c4660c583a60e93.
+---
+ gst/isomp4/qtdemux.c | 13 +++++--------
+ 1 file changed, 5 insertions(+), 8 deletions(-)
+
+diff --git a/gst/isomp4/qtdemux.c b/gst/isomp4/qtdemux.c
+index 4116f4c69..a7ef6e634 100644
+--- a/gst/isomp4/qtdemux.c
++++ b/gst/isomp4/qtdemux.c
+@@ -8059,12 +8059,6 @@ gst_qtdemux_request_protection_context (GstQTDemux * qtdemux,
+   filtered_sys_ids = gst_protection_filter_systems_by_available_decryptors (
+       (const gchar **) qtdemux->protection_system_ids->pdata);
+-  if (!filtered_sys_ids) {
+-    GST_INFO_OBJECT (element,
+-        "No avalaible decryptor, not worth asking the user to choose.");
+-    return;
+-  }
+-
+   g_ptr_array_remove_index (qtdemux->protection_system_ids,
+       qtdemux->protection_system_ids->len - 1);
+   GST_TRACE_OBJECT (qtdemux, "detected %u protection systems, we have "
+@@ -8096,7 +8090,8 @@ gst_qtdemux_request_protection_context (GstQTDemux * qtdemux,
+   query = gst_query_new_context ("drm-preferred-decryption-system-id");
+   st = gst_query_writable_structure (query);
+   gst_structure_set (st, "track-id", G_TYPE_UINT, stream->track_id,
+-      "stream-encryption-systems", G_TYPE_STRV, filtered_sys_ids, NULL);
++      "avalaible-stream-encryption-systems", G_TYPE_STRV, filtered_sys_ids,
++      NULL);
+   gst_structure_set_value (st, "stream-encryption-events", &event_list);
+   if (gst_qtdemux_run_query (element, query, GST_PAD_SRC)) {
+     gst_query_parse_context (query, &ctxt);
+@@ -8120,7 +8115,9 @@ gst_qtdemux_request_protection_context (GstQTDemux * qtdemux,
+         "drm-preferred-decryption-system-id");
+     st = (GstStructure *) gst_message_get_structure (msg);
+     gst_structure_set (st, "track-id", G_TYPE_UINT, stream->track_id,
+-        "stream-encryption-systems", G_TYPE_STRV, filtered_sys_ids, NULL);
++        "avalaible-stream-encryption-systems", G_TYPE_STRV, filtered_sys_ids,
++        NULL);
++
+     gst_structure_set_value (st, "stream-encryption-events", &event_list);
+     gst_element_post_message (element, msg);
+   }
+-- 
+2.17.0
+