Bug 171595 – [GTK] Bump GStreamer version to 1.10.4 in jhbuild

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Carlos Garcia Campos]

Upgrade from 1.8 to 1.10

[Carlos Garcia Campos]

I've tried to upgrade, removing all patches (I don't know if any of those would still be needed with 1.10, though). And the results are not that bad, there some unexpected failures, but also unexpected passes. So, I think we can land the upgrade and deal with the new failures.

The relevant media results are

Regressions: Unexpected text-only failures
  imported/w3c/web-platform-tests/media-source/mediasource-redundant-seek.html [ Failure ]
  imported/w3c/web-platform-tests/media-source/mediasource-removesourcebuffer.html [ Failure ]
  imported/w3c/web-platform-tests/media-source/mediasource-seek-beyond-duration.html [ Failure ]
  imported/w3c/web-platform-tests/media-source/mediasource-seek-during-pending-seek.html [ Failure ]
  media/media-source/media-source-init-segment-duration.html [ Failure ]
  media/video-error-does-not-exist.html [ Failure ]
  media/video-load-networkState.html [ Failure ]
  media/video-source-error.html [ Failure ]
  media/video-source-none-supported.html [ Failure ]

Regressions: Unexpected audio failures
  webaudio/codec-tests/wav/24bit-22khz-resample.html [ Failure ]

Regressions: Unexpected crashes
  http/tests/media/video-accept-encoding.html [ Crash ]

Regressions: Unexpected timeouts
  media/video-source-moved.html [ Timeout ]

Unexpected flakiness: timeouts
  media/W3C/audio/events/event_canplaythrough.html [ Timeout Pass ]
  media/W3C/video/events/event_loadedmetadata_manual.html [ Timeout Pass ]
  media/W3C/video/events/event_timeupdate_manual.html [ Timeout Pass ]
  media/W3C/video/readyState/readyState_during_playing.html [ Timeout Pass ]
  media/track/track-in-band-legacy-api.html [ Failure Timeout ]

[Carlos Garcia Campos]

Created attachment 308892 [details]
Patch

[Enrique Ocaña]

At least these two patches might still be important. We're keeping them in out downstream port, which also uses GStreamer  1.10.4:

* gtk/patches/gst-plugins-good-0004-qtdemux-add-context-for-a-preferred-protection.patch
* gtk/patches/gstreamer-0001-protection-added-function-to-filter-system-ids.patch

[Carlos Garcia Campos]

(In reply to Enrique Ocaña from comment #3)
> At least these two patches might still be important. We're keeping them in
> out downstream port, which also uses GStreamer  1.10.4:
> 
> *
> gtk/patches/gst-plugins-good-0004-qtdemux-add-context-for-a-preferred-
> protection.patch
> *
> gtk/patches/gstreamer-0001-protection-added-function-to-filter-system-ids.
> patch

Thanks Quique! Do you know which tests those patches fix?

[Xabier Rodríguez Calvar]

(In reply to Carlos Garcia Campos from comment #4)
> > *
> > gtk/patches/gst-plugins-good-0004-qtdemux-add-context-for-a-preferred-
> > protection.patch
> > *
> > gtk/patches/gstreamer-0001-protection-added-function-to-filter-system-ids.
> > patch
> 
> Thanks Quique! Do you know which tests those patches fix?

None yet, it's EME code.

[Xabier Rodríguez Calvar]

Comment on attachment 308892 [details]
Patch

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

> Tools/ChangeLog:17
> +        * gtk/patches/gst-plugins-bad-0001-dtls-port-to-OpenSSL-1.1.0.patch: Removed.
> +        * gtk/patches/gst-plugins-bad-0002-dtlscertificate-Fix-error-checking-in-RSA_generate_k.patch: Removed.
> +        * gtk/patches/gst-plugins-good-0001-rtpbin-pipeline-gets-an-EOS-when-any-rtpsources-byes.patch: Removed.
> +        * gtk/patches/gst-plugins-good-0002-rtpbin-avoid-generating-errors-when-rtcp-messages-ar.patch: Removed.
> +        * gtk/patches/gst-plugins-good-0003-rtpbin-receive-bundle-support.patch: Removed.
> +        * gtk/patches/gst-plugins-good-0004-qtdemux-add-context-for-a-preferred-protection.patch: Removed.
> +        * gtk/patches/gst-plugins-good-Revert-qtdemux-expose-streams-with-first-moof-for-fr.patch: Removed.
> +        * gtk/patches/gst-plugins-good-use-the-tfdt-decode-time.patch: Removed.
> +        * gtk/patches/gstreamer-0001-protection-added-function-to-filter-system-ids.patch: Removed.

Please, check if these patches are present in 1.10.4 before removing them. As I said at least two of them are still needed. Please keep me posted.

> Tools/gtk/jhbuild.modules:-361
> -  <autotools id="orc" autogenargs="--disable-gtk-doc" autogen-sh="configure">
> -    <branch module="orc/orc-0.4.17.tar.gz" version="0.4.17"
> -            repo="gstreamer"
> -            hash="sha256:4fc7cca48c59fff23afee78fb642cdbde001f56401c8f47b95a16578d1d5d7e8"
> -            md5sum="af1bf3dab9e69f3c36f389285e2a12a1"/>
> -  </autotools>

Why are you removing orc?

[Xabier Rodríguez Calvar]

(In reply to Xabier Rodríguez Calvar from comment #6)
> Please, check if these patches are present in 1.10.4 before removing them.
> As I said at least two of them are still needed. Please keep me posted.

And a test status update would be interesting too.

[Carlos Garcia Campos]

(In reply to Xabier Rodríguez Calvar from comment #5)
> (In reply to Carlos Garcia Campos from comment #4)
> > > *
> > > gtk/patches/gst-plugins-good-0004-qtdemux-add-context-for-a-preferred-
> > > protection.patch
> > > *
> > > gtk/patches/gstreamer-0001-protection-added-function-to-filter-system-ids.
> > > patch
> > 
> > Thanks Quique! Do you know which tests those patches fix?
> 
> None yet, it's EME code.

We don't maintain patches to fix tests that don't exist.

[Carlos Garcia Campos]

(In reply to Xabier Rodríguez Calvar from comment #6)
> Comment on attachment 308892 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=308892&action=review
> 
> > Tools/ChangeLog:17
> > +        * gtk/patches/gst-plugins-bad-0001-dtls-port-to-OpenSSL-1.1.0.patch: Removed.
> > +        * gtk/patches/gst-plugins-bad-0002-dtlscertificate-Fix-error-checking-in-RSA_generate_k.patch: Removed.
> > +        * gtk/patches/gst-plugins-good-0001-rtpbin-pipeline-gets-an-EOS-when-any-rtpsources-byes.patch: Removed.
> > +        * gtk/patches/gst-plugins-good-0002-rtpbin-avoid-generating-errors-when-rtcp-messages-ar.patch: Removed.
> > +        * gtk/patches/gst-plugins-good-0003-rtpbin-receive-bundle-support.patch: Removed.
> > +        * gtk/patches/gst-plugins-good-0004-qtdemux-add-context-for-a-preferred-protection.patch: Removed.
> > +        * gtk/patches/gst-plugins-good-Revert-qtdemux-expose-streams-with-first-moof-for-fr.patch: Removed.
> > +        * gtk/patches/gst-plugins-good-use-the-tfdt-decode-time.patch: Removed.
> > +        * gtk/patches/gstreamer-0001-protection-added-function-to-filter-system-ids.patch: Removed.
> 
> Please, check if these patches are present in 1.10.4 before removing them.
> As I said at least two of them are still needed. Please keep me posted.

I don't understand why those two tests are needed if they don't fix any exiting test.

> > Tools/gtk/jhbuild.modules:-361
> > -  <autotools id="orc" autogenargs="--disable-gtk-doc" autogen-sh="configure">
> > -    <branch module="orc/orc-0.4.17.tar.gz" version="0.4.17"
> > -            repo="gstreamer"
> > -            hash="sha256:4fc7cca48c59fff23afee78fb642cdbde001f56401c8f47b95a16578d1d5d7e8"
> > -            md5sum="af1bf3dab9e69f3c36f389285e2a12a1"/>
> > -  </autotools>
> 
> Why are you removing orc?

Because the version in recent distros is even higher than that one. 0.4.22 in debian, for example.

[Carlos Alberto Lopez Perez]

Comment on attachment 308892 [details]
Patch

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

> Tools/ChangeLog:13
> +        * gtk/patches/gst-plugins-good-0003-rtpbin-receive-bundle-support.patch: Removed.

This one is not merged and is needed Webrtc. Please recover it.

[Xabier Rodríguez Calvar]

(In reply to Carlos Garcia Campos from comment #8)
> We don't maintain patches to fix tests that don't exist.

The feature is under development and we can't work without it so please, leave those two there.

(In reply to Carlos Garcia Campos from comment #9)
> I don't understand why those two tests are needed if they don't fix any
> exiting test.

As I said, feature is under heavy development and we need them.

> > > Tools/gtk/jhbuild.modules:-361
> > > -  <autotools id="orc" autogenargs="--disable-gtk-doc" autogen-sh="configure">
> > > -    <branch module="orc/orc-0.4.17.tar.gz" version="0.4.17"
> > > -            repo="gstreamer"
> > > -            hash="sha256:4fc7cca48c59fff23afee78fb642cdbde001f56401c8f47b95a16578d1d5d7e8"
> > > -            md5sum="af1bf3dab9e69f3c36f389285e2a12a1"/>
> > > -  </autotools>
> > 
> > Why are you removing orc?
> 
> Because the version in recent distros is even higher than that one. 0.4.22
> in debian, for example.

Ok.

(In reply to Carlos Alberto Lopez Perez from comment #10)
> This one is not merged and is needed Webrtc. Please recover it.

Yes, basically all patches that are not in GStreamer yet are needed, either for tests or for features that are being developed. Please, check the availability of those patches in the required version, rerun the tests and resubmit the patch.

[Carlos Garcia Campos]

(In reply to Xabier Rodríguez Calvar from comment #11)
> (In reply to Carlos Garcia Campos from comment #8)
> > We don't maintain patches to fix tests that don't exist.
> 
> The feature is under development and we can't work without it so please,
> leave those two there.
> 
> (In reply to Carlos Garcia Campos from comment #9)
> > I don't understand why those two tests are needed if they don't fix any
> > exiting test.
> 
> As I said, feature is under heavy development and we need them.
> 
> > > > Tools/gtk/jhbuild.modules:-361
> > > > -  <autotools id="orc" autogenargs="--disable-gtk-doc" autogen-sh="configure">
> > > > -    <branch module="orc/orc-0.4.17.tar.gz" version="0.4.17"
> > > > -            repo="gstreamer"
> > > > -            hash="sha256:4fc7cca48c59fff23afee78fb642cdbde001f56401c8f47b95a16578d1d5d7e8"
> > > > -            md5sum="af1bf3dab9e69f3c36f389285e2a12a1"/>
> > > > -  </autotools>
> > > 
> > > Why are you removing orc?
> > 
> > Because the version in recent distros is even higher than that one. 0.4.22
> > in debian, for example.
> 
> Ok.
> 
> (In reply to Carlos Alberto Lopez Perez from comment #10)
> > This one is not merged and is needed Webrtc. Please recover it.
> 
> Yes, basically all patches that are not in GStreamer yet are needed, either
> for tests or for features that are being developed. Please, check the
> availability of those patches in the required version, rerun the tests and
> resubmit the patch.

I wish I had the time, I don't know when I'll find some more time to do all that

[Carlos Alberto Lopez Perez]

Comment on attachment 308892 [details]
Patch

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

I have checked the patches, I comment below inline which ones should be kept or removed

> Tools/gtk/jhbuild.modules:-374
> -      <patch file="gstreamer-0001-protection-added-function-to-filter-system-ids.patch" strip="1"/>

I don't see it merged anywhere neither any bug related to it. I guess its still a WIP.
Keep this patch.

> Tools/gtk/jhbuild.modules:-406
> -      <patch file="gst-plugins-good-use-the-tfdt-decode-time.patch" strip="1"/>

Already merged in 1.10.4 : https://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/?id=a2adb7e30
Remove it

> Tools/gtk/jhbuild.modules:-407
> -      <patch file="gst-plugins-good-Revert-qtdemux-expose-streams-with-first-moof-for-fr.patch" strip="1"/>

Already merged in 1.10.4 : https://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/?id=707c69cb7
Remove it

> Tools/gtk/jhbuild.modules:-408
> -      <patch file="gst-plugins-good-0001-rtpbin-pipeline-gets-an-EOS-when-any-rtpsources-byes.patch" strip="1"/>

Merged in 1.10.4 : https://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/?id=7ec1ba2bf
Note: This was reverted on master, but not on 1.10.4 .. this means we may need to recover it on the future if we update beyond 1.10.4 and the patch doesn't gets merged again upstream.
For the moment (update to 1.10.4) we should just simply remove it as it is merged on 1.10.4
Remove it

> Tools/gtk/jhbuild.modules:-409
> -      <patch file="gst-plugins-good-0002-rtpbin-avoid-generating-errors-when-rtcp-messages-ar.patch" strip="1"/>

Already merged in 1.10.4 : https://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/?id=7708a1793
Remove it

> Tools/gtk/jhbuild.modules:-410
> -      <patch file="gst-plugins-good-0003-rtpbin-receive-bundle-support.patch" strip="1"/>

This was merged on https://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/?id=dcd3ce9751cdef0b5ab1fa118355f92bdfe82cb3 (>= 1.11.1)
So, we need to keep this patch because its not on 1.10.4
Keep this patch.

> Tools/gtk/jhbuild.modules:-411
> -      <patch file="gst-plugins-good-0004-qtdemux-add-context-for-a-preferred-protection.patch" strip="1"/>

I don't see it merged anywhere neither any bug related to it. I guess its still a WIP.
Keep this patch.

> Tools/gtk/jhbuild.modules:-427
> -      <patch file="gst-plugins-bad-0001-dtls-port-to-OpenSSL-1.1.0.patch" strip="1"/>

Already merged in 1.10.4 : https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=e93893316
Remove it

> Tools/gtk/jhbuild.modules:-428
> -      <patch file="gst-plugins-bad-0002-dtlscertificate-Fix-error-checking-in-RSA_generate_k.patch" strip="1"/>

Already merged in 1.10.4 : https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=b810d09fd
Remove it

[Michael Catanzaro]

So we have two patches without any upstream bug report?

[Carlos Alberto Lopez Perez]

(In reply to Michael Catanzaro from comment #14)
> So we have two patches without any upstream bug report?

According to the previous conversation those patches are EME related, and the feature is still under development. Most likely the patches are still not ready to be proposed to upstream.

But I'm just guessing here, if you want more details ask to the authors of the patches.

[Carlos Garcia Campos]

(In reply to Carlos Alberto Lopez Perez from comment #13)
> Comment on attachment 308892 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=308892&action=review
> 
> I have checked the patches, I comment below inline which ones should be kept
> or removed

Wow, thanks for doing the boring work!

> > Tools/gtk/jhbuild.modules:-374
> > -      <patch file="gstreamer-0001-protection-added-function-to-filter-system-ids.patch" strip="1"/>
> 
> I don't see it merged anywhere neither any bug related to it. I guess its
> still a WIP.
> Keep this patch.

I still don't understand why we keep patches for our dependencies that are not tracked upstream and don't fix any test.

> > Tools/gtk/jhbuild.modules:-406
> > -      <patch file="gst-plugins-good-use-the-tfdt-decode-time.patch" strip="1"/>
> 
> Already merged in 1.10.4 :
> https://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/?id=a2adb7e30
> Remove it
> 
> > Tools/gtk/jhbuild.modules:-407
> > -      <patch file="gst-plugins-good-Revert-qtdemux-expose-streams-with-first-moof-for-fr.patch" strip="1"/>
> 
> Already merged in 1.10.4 :
> https://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/?id=707c69cb7
> Remove it
> 
> > Tools/gtk/jhbuild.modules:-408
> > -      <patch file="gst-plugins-good-0001-rtpbin-pipeline-gets-an-EOS-when-any-rtpsources-byes.patch" strip="1"/>
> 
> Merged in 1.10.4 :
> https://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/?id=7ec1ba2bf
> Note: This was reverted on master, but not on 1.10.4 .. this means we may
> need to recover it on the future if we update beyond 1.10.4 and the patch
> doesn't gets merged again upstream.
> For the moment (update to 1.10.4) we should just simply remove it as it is
> merged on 1.10.4
> Remove it
> 
> > Tools/gtk/jhbuild.modules:-409
> > -      <patch file="gst-plugins-good-0002-rtpbin-avoid-generating-errors-when-rtcp-messages-ar.patch" strip="1"/>
> 
> Already merged in 1.10.4 :
> https://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/?id=7708a1793
> Remove it
> 
> > Tools/gtk/jhbuild.modules:-410
> > -      <patch file="gst-plugins-good-0003-rtpbin-receive-bundle-support.patch" strip="1"/>
> 
> This was merged on
> https://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/
> ?id=dcd3ce9751cdef0b5ab1fa118355f92bdfe82cb3 (>= 1.11.1)
> So, we need to keep this patch because its not on 1.10.4
> Keep this patch.
> 
> > Tools/gtk/jhbuild.modules:-411
> > -      <patch file="gst-plugins-good-0004-qtdemux-add-context-for-a-preferred-protection.patch" strip="1"/>
> 
> I don't see it merged anywhere neither any bug related to it. I guess its
> still a WIP.
> Keep this patch.
> 
> > Tools/gtk/jhbuild.modules:-427
> > -      <patch file="gst-plugins-bad-0001-dtls-port-to-OpenSSL-1.1.0.patch" strip="1"/>
> 
> Already merged in 1.10.4 :
> https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=e93893316
> Remove it
> 
> > Tools/gtk/jhbuild.modules:-428
> > -      <patch file="gst-plugins-bad-0002-dtlscertificate-Fix-error-checking-in-RSA_generate_k.patch" strip="1"/>
> 
> Already merged in 1.10.4 :
> https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=b810d09fd
> Remove it

[Xabier Rodríguez Calvar]

(In reply to Michael Catanzaro from comment #14)
> So we have two patches without any upstream bug report?

There's a bug report in GStreamer.

[Carlos Garcia Campos]

Created attachment 309026 [details]
Updated patch

Results are the same as with the previous patch.

[Xabier Rodríguez Calvar]

Comment on attachment 309026 [details]
Updated patch

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

> Tools/ChangeLog:13
> +        * gtk/patches/gst-plugins-good-0004-qtdemux-add-context-for-a-preferred-protection.patch:

Nit: If you want to mark it as rebased, perfect.

[WebKit Commit Bot]

Comment on attachment 309026 [details]
Updated patch

Clearing flags on attachment: 309026

Committed r216179: <http://trac.webkit.org/changeset/216179>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Michael Catanzaro]

I updated the expectation for webaudio/codec-tests/wav/24bit-22khz-resample-expected.wav in http://trac.webkit.org/changeset/216476/webkit, because I can't hear any difference. Please yell if that was wrong.