Bug 239387 - [GStreamer] REGRESSION(r285586): we never end the playback of some videos
Summary: [GStreamer] REGRESSION(r285586): we never end the playback of some videos
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-04-15 08:52 PDT by Yacine Bandou
Modified: 2022-04-20 06:50 PDT (History)
6 users (show)

See Also:


Attachments
Patch (2.76 KB, patch)
2022-04-15 09:13 PDT, Yacine Bandou
no flags Details | Formatted Diff | Diff
Patch (34.11 KB, patch)
2022-04-19 09:51 PDT, Yacine Bandou
no flags Details | Formatted Diff | Diff
Patch (34.15 KB, patch)
2022-04-20 04:45 PDT, Yacine Bandou
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yacine Bandou 2022-04-15 08:52:01 PDT
Since https://commits.webkit.org/r285586 we ignore the EOS message with some videos like this one:

https://www.tv5mondeplus.com/bumpers/tv5_bumper.mp4
Comment 1 Yacine Bandou 2022-04-15 09:13:08 PDT
Created attachment 457703 [details]
Patch
Comment 2 Philippe Normand 2022-04-15 10:39:12 PDT
Comment on attachment 457703 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        [GStreamer] REGRESSION(r285586): we never end the playback of some videos

That seems bad :( I wonder how this wasn't detected earlier with layout tests

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1787
> +            GRefPtr<GstPad> SinkPad = adoptGRef(gst_element_get_static_pad(m_videoSink.get(), "sink"));

auto sinkPad = ...

local variables should not begin with a capital letter

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1792
> +            GRefPtr<GstPad> SinkPad = adoptGRef(gst_element_get_static_pad(m_audioSink.get(), "sink"));

ditto
Comment 3 Yacine Bandou 2022-04-19 06:00:09 PDT
(In reply to Philippe Normand from comment #2)
> Comment on attachment 457703 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=457703&action=review
> 
> > Source/WebCore/ChangeLog:3
> > +        [GStreamer] REGRESSION(r285586): we never end the playback of some videos
> 
> That seems bad :( I wonder how this wasn't detected earlier with layout tests
> 

This issue is seen on some MP4 videos that have an inaccurate duration in the mvhd box.
Comment 4 Yacine Bandou 2022-04-19 09:51:21 PDT
Created attachment 457908 [details]
Patch
Comment 5 Philippe Normand 2022-04-20 02:50:32 PDT
Comment on attachment 457908 [details]
Patch

Thanks for providing a test :)
Comment 6 EWS 2022-04-20 02:51:16 PDT
/Volumes/Data/worker/Commit-Queue/build/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).
Comment 7 Philippe Normand 2022-04-20 02:56:29 PDT
Comment on attachment 457908 [details]
Patch

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

> Source/WebCore/ChangeLog:5
> +

https://webkit.org/contributing-code/#changelog-files
Comment 8 Yacine Bandou 2022-04-20 04:45:14 PDT
Created attachment 457976 [details]
Patch
Comment 9 EWS 2022-04-20 05:57:58 PDT
Committed r293091 (249802@main): <https://commits.webkit.org/249802@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 457976 [details].
Comment 10 Radar WebKit Bug Importer 2022-04-20 05:58:26 PDT
<rdar://problem/92022235>