Bug 167588 – [EME][GStreamer] Fixed decryptor selection

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Xabier Rodríguez Calvar]

[EME][GStreamer] Fixed decryptor selection

[Xabier Rodríguez Calvar]

Created attachment 300114 [details]
Patch

[Build Bot]

Comment on attachment 300114 [details]
Patch

Attachment 300114 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2974515

New failing tests:
css3/filters/backdrop/dynamic-with-clip-path.html

[Build Bot]

Created attachment 300122 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Xabier Rodríguez Calvar]

Created attachment 300240 [details]
Patch

Fixed build for --encrypted-media

[Zan Dobersek]

Comment on attachment 300240 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=300240&action=review

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:293
> +        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);
> +        GST_TRACE("found %u protection events", streamEncryptionEventsListSize);
> +        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);

This, along with the later loop, is really hard to grasp. I'd recommend two static methods that extract sequences of GRefPtr<GstEvent> and allowed system strings from the structure.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:294
> +        Ref<SharedBuffer> concatenatedInitDataChunks = SharedBuffer::create();

This isn't really being shared with anything, so it could simply be a Vector.

> Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.h:155
> +    GRefPtr<GstBuffer> m_pendingKey;

Should be guarded with ENABLE(LEGACY_ENCRYPTED_MEDIA).

> Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp:808
> +    for (auto iterator : m_appendPipelinesMap)
> +        iterator.value->dispatchDecryptionKey(buffer);

for (auto it : m_appendPipelinesMap.values())

[Xabier Rodríguez Calvar]

Created attachment 300325 [details]
Patch

Requested style changes

[Zan Dobersek]

Comment on attachment 300325 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=300325&action=review

I can also r- upon request.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:299
> +        std::pair<const GValue*, const char**> streamEncryptionInformation = extractEventsAndSystemsFromMessage(message);

I was thinking this helper function would directly return a Vector of GRefPtr<GstEvent> objects, and a Vector of String objects. That way you're spared of further code around this loop.

[Xabier Rodríguez Calvar]

Created attachment 300410 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 300410 [details]
Patch for landing

Clearing flags on attachment: 300410

Committed r211562: <http://trac.webkit.org/changeset/211562>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.