RESOLVED FIXED 186880
[GStreamer] ASSERTION FAILED: end.isValid() in PlatformTimeRanges::add 
https://bugs.webkit.org/show_bug.cgi?id=186880
Summary [GStreamer] ASSERTION FAILED: end.isValid() in PlatformTimeRanges::add
Fujii Hironori
Reported 2018-06-21 01:10:02 PDT
[GStreamer] ASSERTION FAILED: end.isValid() in PlatformTimeRanges::add Some LayoutTests fails. See Bug 180253 and Bug 116977 Comment 2 for detail. Callstack: > Thread 1 (Thread 0x7f0b1787af80 (LWP 4274)): > #0 0x00007f0b021173da in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:267 > #1 0x00007f0b115bebab in WebCore::PlatformTimeRanges::add (this=0x55e71ced1de0, start=..., end=...) at ../../Source/WebCore/platform/graphics/PlatformTimeRanges.cpp:139 > #2 0x00007f0b11edb993 in WebCore::MediaPlayerPrivateGStreamer::buffered (this=0x7f0a8b078000) at ../../Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1116 > #3 0x00007f0b115a6e5c in WebCore::MediaPlayer::buffered (this=0x7f0af0054a80) at ../../Source/WebCore/platform/graphics/MediaPlayer.cpp:818 > #4 0x00007f0b10e761c2 in WebCore::HTMLMediaElement::buffered (this=0x7f0a8a2007b0) at ../../Source/WebCore/html/HTMLMediaElement.cpp:5081 > #5 0x00007f0b0fbd3613 in WebCore::jsHTMLMediaElementBufferedGetter (state=..., thisObject=..., throwScope=...) at DerivedSources/WebCore/JSHTMLMediaElement.cpp:686 (...)
Attachments
Patch (5.25 KB, patch)
2018-06-21 02:20 PDT, Fujii Hironori 		calvaris: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Fujii Hironori
Comment 1 2018-06-21 02:20:13 PDT
Created attachment 343227 [details] Patch
Fujii Hironori
Comment 2 2018-06-21 02:23:35 PDT
This assertion failure is annoying. Looking into the code, I've found strange code in MediaPlayerPrivateGStreamer::buffered. I'd like to propose this patch.
Xabier Rodríguez Calvar
Comment 3 2018-06-21 02:30:30 PDT
LGTM. Alicia, what do you think?
Alicia Boya García
Comment 4 2018-06-21 02:58:41 PDT
Comment on attachment 343227 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343227&action=review > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1116 > + if (loaded.isValid() && loaded) LGTM. This will still create an empty [0, 0) range in some situations though. It maybe good to add `&& loaded > MediaTime::zeroTime()` to avoid that.
Alicia Boya García
Comment 5 2018-06-21 03:03:04 PDT
(In reply to Alicia Boya García from comment #4) > Comment on attachment 343227 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=343227&action=review > > > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1116 > > + if (loaded.isValid() && loaded) > > LGTM. This will still create an empty [0, 0) range in some situations > though. It maybe good to add `&& loaded > MediaTime::zeroTime()` to avoid > that. Nevermind, `loaded` already is true if different than zero.
Fujii Hironori
Comment 6 2018-06-21 03:10:20 PDT
(In reply to Alicia Boya García from comment #5) > Nevermind, `loaded` already is true if different than zero. Agreed. Thank you for the review, Alicia.
Fujii Hironori
Comment 7 2018-06-21 03:15:23 PDT
Committed r233033: <https://trac.webkit.org/changeset/233033>
Michael Catanzaro
Comment 8 2018-06-21 08:38:27 PDT
Comment on attachment 343227 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343227&action=review > Source/WebCore/ChangeLog:18 > + No new tests (No behavior change). Nit: if there was no behavior change, then the execution of the program would not be visibly different. So there is definitely a behavior change: you fixed a crash! Instead, I would have mentioned that this fixed existing tests.
Note You need to log in before you can comment on or make changes to this bug.