Bug 185689

Summary: [GStreamer] Update to GStreamer 1.14.1 in jhbuild
Product: WebKit Reporter: Thibault Saunier <tsaunier>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bandou.yacine, bugs-noreply, calvaris, cgarcia, commit-queue, mcatanzaro, pnormand, tsaunier, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
[GStreamer] Update to GStreamer 1.14.0 in jhbuild
none
Fix mixup with patches.
none
- Remove now unnecessary `gstreamer/patches/gst-plugins-good-0001-souphttpsrc-cookie-jar-and-context-query-support.patch`
none
We are now using GStreamer 1.14.1 - specify in the ChangeLog
none
Add upstream commit ID in the .jhbuild file none

Description Thibault Saunier 2018-05-16 11:26:47 PDT
.
Comment 1 Thibault Saunier 2018-05-16 11:29:01 PDT
Created attachment 340506 [details]
[GStreamer] Update to GStreamer 1.14.0 in jhbuild

And update the patches, removing the ones that have been merged upstream.
Comment 2 Philippe Normand 2018-05-16 11:45:22 PDT
Comment on attachment 340506 [details]
[GStreamer] Update to GStreamer 1.14.0 in jhbuild

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

> Tools/ChangeLog:14
> +        * gstreamer/patches/gst-plugins-good-0006-qtdemux-add-context-for-a-preferred-protection.patch: Removed.
> +        * gstreamer/patches/gst-plugins-good-0008-qtdemux-also-push-buffers-without-encryption-info-in.patch: Removed.

Those 2 are still in Bugzilla hibernation. I locally rebased them against git master, I can share them if you want.
Comment 3 Thibault Saunier 2018-05-16 11:48:15 PDT
(In reply to Philippe Normand from comment #2)
> Comment on attachment 340506 [details]
> [GStreamer] Update to GStreamer 1.14.0 in jhbuild
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=340506&action=review
> 
> > Tools/ChangeLog:14
> > +        * gstreamer/patches/gst-plugins-good-0006-qtdemux-add-context-for-a-preferred-protection.patch: Removed.
> > +        * gstreamer/patches/gst-plugins-good-0008-qtdemux-also-push-buffers-without-encryption-info-in.patch: Removed.
> 
> Those 2 are still in Bugzilla hibernation. I locally rebased them against
> git master, I can share them if you want.


Erg, forgot to re add updated version, sorry.
Comment 4 Thibault Saunier 2018-05-16 11:58:43 PDT
Created attachment 340510 [details]
Fix mixup with patches.
Comment 5 Carlos Garcia Campos 2018-05-16 23:01:05 PDT
Comment on attachment 340510 [details]
Fix mixup with patches.

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

> Tools/gstreamer/jhbuild.modules:78
> +      <patch file="gst-plugins-good-0001-souphttpsrc-cookie-jar-and-context-query-support.patch" strip="1"/>
> +      <patch file="gst-plugins-good-0002-qtdemux-add-context-for-a-preferred-protection.patch" strip="1"/>
> +      <patch file="gst-plugins-good-0003-qtdemux-also-push-buffers-without-encryption-info-in.patch" strip="1"/>

Please, add a comment before any patch explaining why they are needed, where they came from and whether they will be included in a future gst release. I know they were here before, but now that we are updating gst, we should document the patches we are using.
Comment 6 Michael Catanzaro 2018-05-17 09:26:41 PDT
(In reply to Carlos Garcia Campos from comment #5)
> Please, add a comment before any patch explaining why they are needed, where
> they came from and whether they will be included in a future gst release. I
> know they were here before, but now that we are updating gst, we should
> document the patches we are using.

I think we need to actually actively remove any patches that are either not upstream, or not on a patch to being upstreamed in the near future. Keeping patches here over a major version upgrade is *not* OK, it indicates not enough effort to upstream the patches between 1.12 and 1.14.
Comment 7 Thibault Saunier 2018-05-17 09:34:59 PDT
(In reply to Michael Catanzaro from comment #6)
> (In reply to Carlos Garcia Campos from comment #5)
> > Please, add a comment before any patch explaining why they are needed, where
> > they came from and whether they will be included in a future gst release. I
> > know they were here before, but now that we are updating gst, we should
> > document the patches we are using.
> 
> I think we need to actually actively remove any patches that are either not
> upstream, or not on a patch to being upstreamed in the near future. Keeping
> patches here over a major version upgrade is *not* OK, it indicates not
> enough effort to upstream the patches between 1.12 and 1.14.

Well those are still being worked on, I will document it more.
Comment 8 Philippe Normand 2018-05-17 09:40:32 PDT
1.14.1 is out today btw, seize the opportunity? :)
Comment 9 Thibault Saunier 2018-05-17 09:51:17 PDT
(In reply to Philippe Normand from comment #8)
> 1.14.1 is out today btw, seize the opportunity? :)

++
Comment 10 Thibault Saunier 2018-05-22 01:44:46 PDT
Created attachment 340962 [details]
- Remove now unnecessary `gstreamer/patches/gst-plugins-good-0001-souphttpsrc-cookie-jar-and-context-query-support.patch`

- All other patches have been merged now, specify in the ChangeLog
Comment 11 Thibault Saunier 2018-05-22 01:48:01 PDT
Created attachment 340963 [details]
We are now using GStreamer 1.14.1 - specify in the ChangeLog
Comment 12 Philippe Normand 2018-05-22 01:58:11 PDT
I'm afraid we still need patches that are not part of 1.14.1 :/
Comment 13 Thibault Saunier 2018-05-22 03:36:57 PDT
(In reply to Philippe Normand from comment #12)
> I'm afraid we still need patches that are not part of 1.14.1 :/

I am keeping the 2 patches that are still required, namely:

> gstreamer/patches/gst-plugins-good-0002-qtdemux-add-context-for-a-preferred-protection.patch: Renamed from Tools/gstreamer/patches/gst-plugins-good-0006-qtdemux-add-context-for-a-preferred-protection.patch
> This patch has now been merged in GStreamer master and will be there in GStreamer 1.16.
> gstreamer/patches/gst-plugins-good-0003-qtdemux-also-push-buffers-without-encryption-info-in.patch: Renamed from Tools/gstreamer/patches/gst-plugins-good-0008-qtdemux-also-push-buffers-without-encryption-info-in.patch.
> This patch has now been merged in GStreamer master and will be there in GStreamer 1.16.
Comment 14 Thibault Saunier 2018-05-22 03:56:20 PDT
Created attachment 340968 [details]
Add upstream commit ID in the .jhbuild file
Comment 15 WebKit Commit Bot 2018-05-22 04:37:21 PDT
Comment on attachment 340968 [details]
Add upstream commit ID in the .jhbuild file

Clearing flags on attachment: 340968

Committed r232060: <https://trac.webkit.org/changeset/232060>
Comment 16 WebKit Commit Bot 2018-05-22 04:37:22 PDT
All reviewed patches have been landed.  Closing bug.
Comment 17 Radar WebKit Bug Importer 2018-05-22 04:38:20 PDT
<rdar://problem/40447628>
Comment 18 Yacine Bandou 2018-05-22 06:25:24 PDT
This patch produces regressions in the LayoutTests.

All the tests: imported/w3c/web-platform-tests/encrypted-media/clearkey-mp4-playback-* crash.

https://build.webkit.org/builders/WPE%20Linux%2064-bit%20Release%20%28Tests%29/builds/7887/steps/layout-test/logs/stdio
Comment 19 Xabier Rodríguez Calvar 2018-05-23 00:21:44 PDT
(In reply to Yacine Bandou from comment #18)
> This patch produces regressions in the LayoutTests.
> 
> All the tests:
> imported/w3c/web-platform-tests/encrypted-media/clearkey-mp4-playback-*
> crash.
> 
> https://build.webkit.org/builders/WPE%20Linux%2064-
> bit%20Release%20%28Tests%29/builds/7887/steps/layout-test/logs/stdio

I guess this to be investigated.