[GStreamer] Several buffering fixes
Created attachment 387656 [details] Patch
Comment on attachment 387656 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387656&action=review > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2308 > + GST_TRACE_OBJECT(pipeline(), "did download finish %s", boolForPrinting(m_didDownloadFinish)); did download finish? > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2593 > + gst_query_parse_buffering_percent(query.get(), (gboolean*) &m_isBuffering, NULL); Casting a bool to gboolean is a bad idea, please proxy it thru a gboolean isBuffering. NULL -> nullptr
Comment on attachment 387656 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387656&action=review Looking forward to apply this locally and fixing lots of buffering bugs :-) > Source/WebCore/ChangeLog:25 > + * platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp: Download as fast as possible when My review window is not showing me any changes in WKWS, is this up-to-date? > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2303 > + auto wasBuffering = m_isBuffering; auto is not needed here, please use bool
Created attachment 387782 [details] Patch for landing
Comment on attachment 387782 [details] Patch for landing Rejecting attachment 387782 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 387782, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!. Full output: https://webkit-queues.webkit.org/results/13304751
Created attachment 387785 [details] Patch for landing
Comment on attachment 387785 [details] Patch for landing Clearing flags on attachment: 387785 Committed r254565: <https://trac.webkit.org/changeset/254565>
All reviewed patches have been landed. Closing bug.
<rdar://problem/58604046>
(In reply to WebKit Commit Bot from comment #7) > Comment on attachment 387785 [details] > Patch for landing > > Clearing flags on attachment: 387785 > > Committed r254565: <https://trac.webkit.org/changeset/254565> This has caused lot of timeouts on the GTK port. The bot its aborting early. I double-checked it locally, reverting this patch makes the timeout go away. At least 39 new timeouts, and likely more. I didn't ran the whole layout test suite. Regressions: Unexpected timeouts (39) compositing/geometry/clipped-video-controller.html [ Timeout ] compositing/geometry/video-fixed-scrolling.html [ Timeout ] compositing/geometry/video-opacity-overlay.html [ Timeout ] compositing/layers-inside-overflow-scroll.html [ Timeout ] compositing/overflow/overflow-compositing-descendant.html [ Timeout ] compositing/overflow/scroll-ancestor-update.html [ Timeout ] compositing/reflections/load-video-in-reflection.html [ Timeout ] compositing/self-painting-layers.html [ Timeout ] compositing/shared-backing/clipping-and-shared-backing.html [ Timeout ] compositing/video-page-visibility.html [ Timeout ] compositing/video/poster.html [ Timeout ] compositing/video/video-background-color.html [ Timeout ] compositing/video/video-border-radius-clipping.html [ Timeout ] compositing/video/video-border-radius.html [ Timeout ] compositing/video/video-clip-change-src.html [ Timeout ] compositing/video/video-object-fit.html [ Timeout ] compositing/video/video-object-position.html [ Timeout ] compositing/video/video-poster.html [ Timeout ] compositing/video/video-reflection.html [ Timeout ] fullscreen/full-screen-iframe-legacy.html [ Timeout ] fullscreen/video-controls-timeline.html [ Timeout ] legacy-animation-engine/compositing/reflections/load-video-in-reflection.html [ Timeout ] media/W3C/audio/events/event_canplaythrough.html [ Timeout ] media/W3C/audio/events/event_canplaythrough_manual.html [ Timeout ] media/W3C/audio/events/event_order_canplay_canplaythrough.html [ Timeout ] media/W3C/audio/paused/paused_false_during_play.html [ Timeout ] media/W3C/audio/readyState/readyState_during_canplaythrough.html [ Timeout ] media/W3C/audio/readyState/readyState_during_playing.html [ Timeout ] media/W3C/video/events/event_canplaythrough.html [ Timeout ] media/W3C/video/events/event_order_canplay_canplaythrough.html [ Timeout ] media/W3C/video/networkState/networkState_during_progress.html [ Timeout ] media/W3C/video/paused/paused_false_during_play.html [ Timeout ] media/W3C/video/readyState/readyState_during_canplaythrough.html [ Timeout ] media/W3C/video/readyState/readyState_during_playing.html [ Timeout ] media/accessiblity-describes-video.html [ Timeout ] media/audio-background-playback-playlist.html [ Timeout ] media/media-fragments/TC0001.html [ Timeout ] media/media-fragments/TC0002.html [ Timeout ] media/track/audio/audio-track-mkv-vorbis-addtrack.html [ Timeout ] See: https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Tests%29/builds/12379 Unless you have a quick fix I think we should revert this patch, because the GTK test bot it is in really bad state.
Re-opened since this is blocked by bug 206331
Created attachment 387912 [details] Patch
(In reply to Carlos Alberto Lopez Perez from comment #10) > (In reply to WebKit Commit Bot from comment #7) > > Comment on attachment 387785 [details] > > Patch for landing > > > > Clearing flags on attachment: 387785 > > > > Committed r254565: <https://trac.webkit.org/changeset/254565> > > This has caused lot of timeouts on the GTK port. The bot its aborting early. > I double-checked it locally, reverting this patch makes the timeout go away. > At least 39 new timeouts, and likely more. I didn't ran the whole layout > test suite. > > Regressions: Unexpected timeouts (39) > compositing/geometry/clipped-video-controller.html [ Timeout ] > compositing/geometry/video-fixed-scrolling.html [ Timeout ] > compositing/geometry/video-opacity-overlay.html [ Timeout ] > compositing/layers-inside-overflow-scroll.html [ Timeout ] > compositing/overflow/overflow-compositing-descendant.html [ Timeout ] > compositing/overflow/scroll-ancestor-update.html [ Timeout ] > compositing/reflections/load-video-in-reflection.html [ Timeout ] > compositing/self-painting-layers.html [ Timeout ] > compositing/shared-backing/clipping-and-shared-backing.html [ Timeout ] > compositing/video-page-visibility.html [ Timeout ] > compositing/video/poster.html [ Timeout ] > compositing/video/video-background-color.html [ Timeout ] > compositing/video/video-border-radius-clipping.html [ Timeout ] > compositing/video/video-border-radius.html [ Timeout ] > compositing/video/video-clip-change-src.html [ Timeout ] > compositing/video/video-object-fit.html [ Timeout ] > compositing/video/video-object-position.html [ Timeout ] > compositing/video/video-poster.html [ Timeout ] > compositing/video/video-reflection.html [ Timeout ] > fullscreen/full-screen-iframe-legacy.html [ Timeout ] > fullscreen/video-controls-timeline.html [ Timeout ] > > legacy-animation-engine/compositing/reflections/load-video-in-reflection. > html [ Timeout ] > media/W3C/audio/events/event_canplaythrough.html [ Timeout ] > media/W3C/audio/events/event_canplaythrough_manual.html [ Timeout ] > media/W3C/audio/events/event_order_canplay_canplaythrough.html [ Timeout ] > media/W3C/audio/paused/paused_false_during_play.html [ Timeout ] > media/W3C/audio/readyState/readyState_during_canplaythrough.html [ Timeout > ] > media/W3C/audio/readyState/readyState_during_playing.html [ Timeout ] > media/W3C/video/events/event_canplaythrough.html [ Timeout ] > media/W3C/video/events/event_order_canplay_canplaythrough.html [ Timeout ] > media/W3C/video/networkState/networkState_during_progress.html [ Timeout ] > media/W3C/video/paused/paused_false_during_play.html [ Timeout ] > media/W3C/video/readyState/readyState_during_canplaythrough.html [ Timeout > ] > media/W3C/video/readyState/readyState_during_playing.html [ Timeout ] > media/accessiblity-describes-video.html [ Timeout ] > media/audio-background-playback-playlist.html [ Timeout ] > media/media-fragments/TC0001.html [ Timeout ] > media/media-fragments/TC0002.html [ Timeout ] > media/track/audio/audio-track-mkv-vorbis-addtrack.html [ Timeout ] > > > > See: > https://build.webkit.org/builders/GTK%20Linux%2064- > bit%20Release%20%28Tests%29/builds/12379 > > > Unless you have a quick fix I think we should revert this patch, because the > GTK test bot it is in really bad state. The problem was stupid simple! I was basically saying that fillTimerFired should not fire an Buffering update if the pipeline didn't answer in DOWNLOAD mode, but this leaded to the pipeline never considering downloadDone for local files. I think further refactoring to avoid that codepath all together could be done, but I added that check mainly because we where querying bufferring on the source instead of the pipelines where we could get result from random elements in the pipeline and where getting results sometimes in DOWNLOAD and sometime in STREAM modes, now that we query the pipeline we can keep updating buffering from the fillTimer callback I think. No regression timeout with the new version of the patch.
*** Bug 206338 has been marked as a duplicate of this bug. ***
Comment on attachment 387912 [details] Patch Clearing flags on attachment: 387912 Committed r254684: <https://trac.webkit.org/changeset/254684>