[webkit-reviews] review denied: [Bug 185948] [EME][GStreamer] Fix regression after update to GStreamer 1.14.1 in jhbuild r232060 : [Attachment 341271] Added a new patch to GStreamer (that I will merge once this one is accepted) that moves back to previous behaviour

[bugzilla-daemon at webkit.org]

Xabier Rodríguez Calvar <calvaris at igalia.com> has denied Thibault Saunier
<tsaunier at gnome.org>'s request for review:
Bug 185948: [EME][GStreamer] Fix regression after update to GStreamer 1.14.1 in
jhbuild r232060
https://bugs.webkit.org/show_bug.cgi?id=185948

Attachment 341271: Added a new patch to GStreamer (that I will merge once this
one is accepted) that moves back to previous behaviour

https://bugs.webkit.org/attachment.cgi?id=341271&action=review




--- Comment #7 from Xabier Rodríguez Calvar <calvaris at igalia.com> ---
Comment on attachment 341271
  --> https://bugs.webkit.org/attachment.cgi?id=341271
Added a new patch to GStreamer (that I will merge once this one is accepted)
that moves back to previous behaviour

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

> Source/WebCore/ChangeLog:3
> +	   [GStreamer] Handle changes in the
"drm-preferred-decryption-system-id" NEED_CONTEXT message.

This line should reflect the bug title. If you want to keep this informative
text I'd suggest to move it down to the long description section.

> Source/WebCore/ChangeLog:12
> +	   Some tests already exist and where failling (crashing) without that
patch.

I would list the tests here, something like:
Tests: imported/w3c/web-platform-tests/encrypted-media/clearkey-mp4-*

Side comment: "where failling"	-> "were failing"

> Tools/ChangeLog:3
> +	   [GStreamer] Handle changes in the
"drm-preferred-decryption-system-id" NEED_CONTEXT message.

Ditto.

> Tools/ChangeLog:10
> +	   properly taken into account in the qtdemux patch that uses it and
got upstreamed. Those 3 new patches fixes that and changes the

... Those 3 new patches fix that and change ...

> Tools/gstreamer/jhbuild.modules:73
> +	 <patch
file="gst-plugins-good-0002-qtdemux-Do-not-unref-a-NULL-stream_tags.patch"
strip="1" /> <!-- Merged as 43a540b1cd9f162d3dae5d50e36703dfaf558a3e -->

I don't understand why this patch is needed here. Besides, it looks like we are
missing
Tools/gstreamer/patches/gst-plugins-good-0003-qtdemux-Clarify-field-name-about-
stream-encryption-s.patch