[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 (...)
Created attachment 343227 [details] Patch
This assertion failure is annoying. Looking into the code, I've found strange code in MediaPlayerPrivateGStreamer::buffered. I'd like to propose this patch.
LGTM. Alicia, what do you think?
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.
(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.
(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.
Committed r233033: <https://trac.webkit.org/changeset/233033>
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.