1231855 - Assertion failure: aStart <= aEnd, at ../../../dist/include/Intervals.h:56

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P2)

Description

[Bob Clary [:bc] (inactive)]

1. http://free.txu.com/

2.  Assertion failure: aStart <= aEnd, at ../../../dist/include/Intervals.h:56

Beta/43, Aurora/44, Nightly/45 Linux, OS X, Windows

Comment 1

[Bob Clary [:bc] (inactive)]

forgot to mention similar though different issue with mp4_demuxer

Comment 2

[Jean-Yves Avenard [:jya]]

could you please provide a backtrace or link to the crash report?

Comment 3

[Bob Clary [:bc] (inactive)]

Attachment: minidump crash report

sorry, I forgot.

Comment 4

[Jean-Yves Avenard [:jya]]

I can reproduce it with this video: http://free.txu.com/videos/TXU_HOLIDAY_GIF-WREATH_990x557.webm

It doesn't always happen.
Playing the video 2,3 times sometimes cause the assertion to occur. If not, I clear the cache and history and reload the video.

the issue arises in
WebMBufferedState::CalculateBufferedForRange as duration is negative:
(lldb) print mTimeMapping[end].mTimecode
(uint64_t) $0 = 32013000000
(lldb) print mTimeMapping[end-1].mTimecode
(uint64_t) $1 = 32074000000

Now, we could always return false if the previous cluster time is > the next one time
Matthew is this something you could look into?

Comment 5

[Jean-Yves Avenard [:jya]]

note that i had to disable media.mp4.enabled to reproduce it

Comment 6

[Matthew Gregan [:kinetik]]

Yep, I'll take a look tomorrow.

Comment 7

[Matthew Gregan [:kinetik]]

(In reply to Jean-Yves Avenard [:jya] from comment #4)
> I can reproduce it with this video:
> http://free.txu.com/videos/TXU_HOLIDAY_GIF-WREATH_990x557.webm

That had me confused for a while.  That video above is only 4 second long, but the timestamps in the debug output are around 32s.  The timestamps in the video above are also consecutive and the file appears to be well formed.

It's actually *this* video: http://free.txu.com/videos/holiday.webm

And, indeed, this file is not well formed:

|+ Cluster at 6238473
| + Cluster timecode: 32.000s at 6238478
| + SimpleBlock (track number 1, 1 frame(s), timecode 32.032s = 00:00:32.032) at 6238482
| + SimpleBlock (track number 2, 1 frame(s), timecode 31.990s = 00:00:31.990) at 6238515
| + SimpleBlock (track number 1, 1 frame(s), timecode 32.074s = 00:00:32.074) at 6238522
| + SimpleBlock (track number 2, 1 frame(s), timecode 32.013s = 00:00:32.013) at 6238555

...with timestamps matching the debug output.

Anyway, we obviously shouldn't fatally assert here, so I'll post a fix tomorrow.

Comment 8

[Matthew Gregan [:kinetik]]

The file was produced with:
| + Muxing application: WonderShare Matroska Muxer at 4177

And the timecodes throughout the file are like this (not just the blocks I quoted earlier), where track 2 (A_VORBIS) has blocks after track 1 (V_VP8) but with timecodes before preceding blocks.

This file also violates the spec in another way, e.g.:

| + SimpleBlock (track number 1, 1 frame(s), timecode 31.990s = 00:00:31.990) at 6238440
|+ Cluster at 6238473
| + Cluster timecode: 32.000s at 6238478
[...]
| + SimpleBlock (track number 2, 1 frame(s), timecode 31.990s = 00:00:31.990) at 6238515

...where it has placed the audio for timecode 31.990 in a later Cluster than the corresponding video.

From the spec (note SHOULD is interpreted as MUST for muxers):
- All absolute (block + cluster) timecodes MUST be monotonically increasing.
  - All timecodes are associated with the start time of the block.
- Audio blocks that contain the video key frame's timecode SHOULD be in the same cluster as the video key frame block. 
- Audio blocks that have same absolute timecode as video blocks SHOULD be written before the video blocks. 

FWIW, remuxing with "ffmpeg -i holiday.webm -vcodec copy -acodec copy holidayff.webm" produces a file that looks valid, with correctly ordered blocks.

Comment 9

[Matthew Gregan [:kinetik]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e1874018eb31

Comment 10

[Matthew Gregan [:kinetik]]

Attachment: v0

Avoid inserting entries into the buffered parser's mappings if they violate the mapping's ordering invariants (and the WebM spec).

Comment 11

[Jean-Yves Avenard [:jya]]

Comment on attachment 8701333 [details] [diff] [review]
v0

Review of attachment 8701333 [details] [diff] [review]:
-----------------------------------------------------------------

I'm assuming not having the cluster in the cache has no consequences...

Comment 12

[Matthew Gregan [:kinetik]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2952f6d3bcb

Comment 13

[Matthew Gregan [:kinetik]]

(In reply to Jean-Yves Avenard [:jya] from comment #11)
> I'm assuming not having the cluster in the cache has no consequences...

The parser only tracks blocks, not clusters, and this change causes out-of-order blocks to be excluded.

With an invalid file like this the behaviour was wrong with the old code and remains wrong with the new code, but it now tries to preserve the assumptions made by the parser (and required by the spec).  I'm not sure what else we can do other than flat-out reject the file, but that could happen at an arbitrary point as you may need to parse the entire file before you detect that it's invalid.

(In reply to Matthew Gregan [:kinetik] from comment #12)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2952f6d3bcb

The change to WebMBufferedParser breaks dom/media/gtest/TestWebMBuffered.cpp because the file used in the test contains two out-of-order blocks.  This push updates the test to exclude the out-of-order blocks.

Comment 14

[Matthew Gregan [:kinetik]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1fc8fc0fa9f4a040e11329a1b2d9330294dc5376
Bug 1231855 - Avoid inserting out of (timecode) order entries in WebMBufferedParser.  r=jya

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1fc8fc0fa9f4