598242 - WebM buffered TimeRanges are not normalized

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

The WebM buffered implementation returns TimeRanges which are in the range [startTime, endTime], but all our other timestamps which we expose to HTML are in the range [0, duration]. We should normalize the WebM buffered timecodes just as we normalize the other media types' TimeRanges, at least until we update our implementation to the current startTime and duration specification.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to comment #1)
> Created attachment 477028 [details] [diff] [review]
> Patch

This is based on top of bug 598217.

Comment 3

[Matthew Gregan [:kinetik]]

It's kind of a shame the reader's can't deal with time in the media stream's timeline and leave the normalization up to the caller.

       aBuffered->Add(aStartTime / MS_PER_S, duration / NS_PER_S);

This case needs to be fixed too.

Add a mochitest for this case please?

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch v2

* With review comments addressed, includes test.

Fixed some issues test showed up:
* nsWebMReader wasn't timestamping decoded audio chunks correctly, the total_samples variable in nsWebMReader::DecodeAudioPacket() which was used to calculate the timestamps of ogg_packets inside a nestegg_packet was not being carried over between packets, meaning every ogg_packet in a nestegg_packet would have the same start time.
* Handle case in nsWebMReader::DecodeVideoFrame() when we assume the last packet ends at the end of the file.
* Renamed nsBuiltinDecoderStateMachine::mGotDurationFromHeader to mGotDurationFromMetaData, and set moved its initalization up into nsBuiltinDecoderStateMachine::LoadMetadata() (from nsOggDecoderStateMachine::LoadMetadata()). This is needed because the *duration* is stored in the WebM metadata (not the end time), and we need to adjust the mEndTime field for WebM in nsBuiltinDecoderStateMachine::FindStartTime() in the same way that the Ogg backend does for a chopped media.

Comment 5

[Matthew Gregan [:kinetik]]

-  { name:"r11025_s16_c1.wav", type:"audio/x-wav", duration:1.0 },
+  { name:"r11025_s16_c1.wav", type:"audio/x-wav", duration:1.0},

Oops?

+      next_tstamp = s->GetEndMediaTime() * NS_PER_MS;

If the WebM file doesn't have a duration, mEndTime is going to be -1 (at least, before UpdatePlaybackPosition is called), so this will compute an invalid next_tstamp, right?

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch v3

Now with less oops.

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

http://hg.mozilla.org/mozilla-central/rev/5f0b56b5b3b3