Bug 201401 – [GStreamer] cnn.com videos don't work

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Michael Catanzaro]

I can't play the videos on https://www.cnn.com/2019/08/30/us/st-louis-child-shooting-arrest/index.html in Tech Preview with 2.25.4.

There's two videos on this page: "At least 12 children have been killed by gunfire in St. Louis since April" and then "Kids remember friends lost to gun violence in St. Louis". Both videos display only a black window, even though we have H.264 support in Tech Preview. The second video sometimes plays a couple seconds of audio, but not always. In both cases, the duration of the video  -- the second number -- changes, which should not happen. After a few seconds, which seems to vary, the first number (the progress) stops changing as well.

[Philippe Normand]

I can reproduce on trunk / MiniBrowser. Going back to the revision before the MSE source element rewrite fixes the issue. So I'm afraid this could be a regression introduced in r249205

[Philippe Normand]

The video track is not enabled. Commenting out the early return in MediaPlayerPrivateGStreamer::enableTrack() line 814, I now see video ;)

[Michael Catanzaro]

But we have 2.25.4 in Tech Preview, and 2.25.4 does not have the MSE source element rewrite. So that cannot be the cause of the bug I'm experiencing. There must be something else wrong too. Right?

[Philippe Normand]

Oh yes, this is a different issue indeed. Here in Ephy TP I have *NO* H.264 decoder:

[📦 org.gnome.Epiphany.Devel ~]$ gst-play-1.0 --videosink "glsinkbin sink=gtkglsink" "https://cnnios-f.akamaihd.net/i/cnn/big/us/2019/08/14/caption/7-year-old-killed-in-st-louis-gunfire-lc-orig.cnn_2782230_ios_,440,650,840,1240,3000,5500,.mp4.csmil/master.m3u8?__b__=650"
Press 'k' to see a list of keyboard shortcuts.
Now playing https://cnnios-f.akamaihd.net/i/cnn/big/us/2019/08/14/caption/7-year-old-killed-in-st-louis-gunfire-lc-orig.cnn_2782230_ios_,440,650,840,1240,3000,5500,.mp4.csmil/master.m3u8?__b__=650
WARNING No decoder available for type 'video/x-h264, stream-format=(string)byte-stream, alignment=(string)nal, parsed=(boolean)true'.
WARNING debug information: ../gst/playback/gsturidecodebin.c(920): unknown_type_cb (): /GstPlayBin:playbin/GstURIDecodeBin:uridecodebin0

And HLS is used, whereas in trunk MSE is used... What a mess.

[Philippe Normand]

So to sum up, in TP 2.25.4, HLS is used but the runtime lost (again?) H.264 decoding support. That's not a WebKit bug.

However in trunk, MSE is used but only audio plays, no video. See comment 2.

[Michael Catanzaro]

(In reply to Philippe Normand from comment #4)
> Oh yes, this is a different issue indeed. Here in Ephy TP I have *NO* H.264
> decoder:

lol....

[Michael Catanzaro]

Let's repurpose this bug for the problem in trunk.

For losing H.264 again: https://gitlab.com/freedesktop-sdk/freedesktop-sdk/issues/858

[Alicia Boya García]

I got a local repro. The bug occurs 100% of the time when the audio initialization segment is appended before the video initialization segment, and 0% of the time when the order is the opposite.

[Michael Catanzaro]

(In reply to Philippe Normand from comment #5)
> So to sum up, in TP 2.25.4, HLS is used but the runtime lost (again?) H.264
> decoding support. That's not a WebKit bug.

This is "fixed," now we support H.264 but any attempt to play it causes the web process to immediately crash:

https://gitlab.com/freedesktop-sdk/freedesktop-sdk/issues/863

I think help from WebKit multimedia experts will be needed to solve it.

[Philippe Normand]

Seems like Mario is your expert, since he wrote(?) this "noopenh264" lib which I had no idea exists, until 5 minutes ago. :)

[Michael Catanzaro]

It looks like freedesktop-sdk developers will deal with it.

It's a bit confusing discussing two different issues in the same place. I'll shut up and leave this one to Alicia.

[Alicia Boya García]

Yeah, that looks like a completely different issue. This one is about the video track not showing about half of the time.

[Alicia Boya García]

Created attachment 378288 [details]
Patch

[Michael Catanzaro]

Comment on attachment 378288 [details]
Patch

Is it really not testable? Or it doesn't fix any existing layout tests?

[Xabier Rodríguez Calvar]

Comment on attachment 378288 [details]
Patch

A test is needed.

[Alicia Boya García]

As far as I know, this is not testable with LayoutTests. I tried using canvas to check the content of the video, but even when the video sink is using the wrong context and the displayed textures in the <video> are wrong, painting them on a canvas will download the correct ones, so testing this can't be automated in JS.

[Xabier Rodríguez Calvar]

(In reply to Alicia Boya García from comment #16)
> As far as I know, this is not testable with LayoutTests. I tried using
> canvas to check the content of the video, but even when the video sink is
> using the wrong context and the displayed textures in the <video> are wrong,
> painting them on a canvas will download the correct ones, so testing this
> can't be automated in JS.

Isn't there any way with the internals?

[Alicia Boya García]

(In reply to Xabier Rodríguez Calvar from comment #17)
> (In reply to Alicia Boya García from comment #16)
> > As far as I know, this is not testable with LayoutTests. I tried using
> > canvas to check the content of the video, but even when the video sink is
> > using the wrong context and the displayed textures in the <video> are wrong,
> > painting them on a canvas will download the correct ones, so testing this
> > can't be automated in JS.
> 
> Isn't there any way with the internals?

Maybe it would work if I had a way to get a screenshot of the whole page (since getting the video alone doesn't reproduce the issue as I explained).

Reftests are supposed to do that, so that may be the way, if they happen to reproduce the bug. I'll give it a look.

[Alicia Boya García]

Created attachment 380299 [details]
Patch

[Alicia Boya García]

Created attachment 380300 [details]
Patch

[Philippe Normand]

The test needs mac-specific expectation files it seems. You can get them from the EWS.

[Alicia Boya García]

Created attachment 380314 [details]
Patch

[Alicia Boya García]

Created attachment 380336 [details]
Patch

[Philippe Normand]

There's a 2 pixel difference now in the mac test results. How come? :(

[Alicia Boya García]

(In reply to Philippe Normand from comment #24)
> There's a 2 pixel difference now in the mac test results. How come? :(

I have no idea. I can't even see it in an image editor using Difference blending mode.

And the difference is near the text, which is produced by identical HTML and CSS. I guess I'll just mark the test as skip in Mac, since I see no way to account for that couple of pixels.

[Philippe Normand]

Go for it. We can't block this forever due to unwilling bots.

[Alicia Boya García]

Created attachment 380535 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 380535 [details]
Patch

Clearing flags on attachment: 380535

Committed r250922: <https://trac.webkit.org/changeset/250922>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/56126329>

[Alicia Boya García]

This patch was almost mechanically reverted temporarily along the rest of the WebKitMediaSrc rework. In theory this should have been OK since this was supposed to fix a regression of that patch... But actually the bug was already causing problems before, the WebKitMediaSrc rework just exposed a more common case.

It has been reported that the same bug arise when adding some CSS styles to the <video> element, and the patch fixes the issue.

And the patch actually makes sense in any case... since without it some perfectly valid state transitions are not covered!

So I'm reintroducing it.

[Alicia Boya García]

Created attachment 381810 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 381810 [details]
Patch

Clearing flags on attachment: 381810

Committed r251544: <https://trac.webkit.org/changeset/251544>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.