Bug 236440

Summary: Safari can't play video completely at bilibili.com
Product: WebKit Reporter: bear.yang
Component: MediaAssignee: Jean-Yves Avenard [:jya] <jean-yves.avenard>
Status: RESOLVED FIXED    
Severity: Major CC: bfulgham, calvaris, eric.carlson, ews-watchlist, glenn, jean-yves.avenard, jer.noble, mcatanzaro, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari 15   
Hardware: Mac (Apple Silicon)   
OS: macOS 12   
URL: www.bilibili.com/video/BV1Cq4y127pq
See Also: https://bugs.webkit.org/show_bug.cgi?id=238201
Attachments:
Description Flags
This is a screenshot of when the video playback failure occurred
none
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description bear.yang 2022-02-10 06:52:55 PST
Created attachment 451525 [details]
This is a screenshot of when the video playback failure occurred

I'm from China and safari can't play this video in its entirety. In the last 3 seconds of the video, the safari screen is stuck and keeps showing that it is buffering data. If I switch to Chrome and Firefox to play this video, there is not the same problem. The address of this video is: https://www.bilibili.com/video/BV1Cq4y127pq
Comment 1 Radar WebKit Bug Importer 2022-02-10 09:48:01 PST
<rdar://problem/88761053>
Comment 2 Jean-Yves Avenard [:jya] 2022-03-21 06:30:39 PDT
Created attachment 455240 [details]
Patch
Comment 3 Jean-Yves Avenard [:jya] 2022-03-22 05:16:31 PDT
Created attachment 455363 [details]
Patch
Comment 4 Jean-Yves Avenard [:jya] 2022-03-22 05:59:15 PDT
Created attachment 455366 [details]
Patch

Add TestExpectations for GTK
Comment 5 Michael Catanzaro 2022-03-22 08:58:36 PDT
Comment on attachment 455366 [details]
Patch

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

> LayoutTests/platform/gtk-wk2/TestExpectations:7
> -# This empty file is needed for flakiness dashboard to work.
> -# FIXME: Remove once the flakiness dashboard is updated to not load this file.
> +# These are the layout test expectations for the GTK2 port of WebKit.
> +#
> +# See http://trac.webkit.org/wiki/TestExpectations for more information on this file.
> +#
> +# Never add any tests to this file without opening a bug on bugzilla.webkit.org.
> +
> +webkit.org/b/238201 media/media-source/media-mp4-hevc-bframes.html [ Skip ]

Can you please use LayoutTests/platform/glib/TestExpectations instead? Thanks.

If it really works for WPE but not GTK, then you can use LayoutTests/platform/gtk/TestExpectations.
Comment 6 Jean-Yves Avenard [:jya] 2022-03-22 14:33:08 PDT
Created attachment 455428 [details]
Patch

Add TestExpectations for GTK attempt #2
Comment 7 Michael Catanzaro 2022-03-22 14:35:43 PDT
Comment on attachment 455428 [details]
Patch

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

> LayoutTests/platform/gtk/TestExpectations:160
> +webkit.org/b/238201 media/media-source/media-mp4-hevc-bframes.html [ Failure ]

It should only be needed in LayoutTests/platform/glib/TestExpectations. WPE and GTK both use GLib, so you only need to touch the GTK expectations if the test is broken for GTK but not for WPE. That should be pretty unusual because the multimedia code is identical.
Comment 8 Michael Catanzaro 2022-03-22 14:35:44 PDT
Comment on attachment 455428 [details]
Patch

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

> LayoutTests/platform/gtk/TestExpectations:160
> +webkit.org/b/238201 media/media-source/media-mp4-hevc-bframes.html [ Failure ]

It should only be needed in LayoutTests/platform/glib/TestExpectations. WPE and GTK both use GLib, so you only need to touch the GTK expectations if the test is broken for GTK but not for WPE. That should be pretty unusual because the multimedia code is identical.
Comment 9 Jean-Yves Avenard [:jya] 2022-03-22 15:32:37 PDT
Created attachment 455439 [details]
Patch

Add TestExpectations for GTK attempt #3
Comment 10 Jean-Yves Avenard [:jya] 2022-03-22 16:13:00 PDT
Created attachment 455449 [details]
Patch

Add TestExpectations for GTK attempt #3
Comment 11 Jer Noble 2022-03-22 16:49:09 PDT
Comment on attachment 455449 [details]
Patch

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

> Source/WebCore/platform/graphics/SourceBufferPrivate.cpp:1262
> +        // Looking up the prior frame would require a computationally intensive operation. So instead, as a workaround

I don't think this is strictly true; we could return a pair of iterators from the `addSample()` code above and step backwards. Perhaps the current behavior is better though, but it might be best to remove that statement from the comment.
Comment 12 Jean-Yves Avenard [:jya] 2022-03-22 17:36:41 PDT
Created attachment 455458 [details]
Patch
Comment 13 Jean-Yves Avenard [:jya] 2022-03-24 05:55:29 PDT
Created attachment 455634 [details]
Patch

fix GTK failures
Comment 14 EWS 2022-03-24 13:52:08 PDT
Committed r291813 (248840@main): <https://commits.webkit.org/248840@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 455634 [details].
Comment 15 Brent Fulgham 2022-05-26 14:53:42 PDT
This fix shipped with Safari 15.5 (all platforms).