Closed Bug 739094 Opened 12 years ago Closed 10 years ago

Invalid timecode calculation in WebM when media starts with an audio frame that has a non-zero start time

Categories

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

Product:
Core
Component:
Audio/Video
Version:
11 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
B2G C2 (20nov-10dec)
Project Flags:
blocking-basecamp -

People

(Reporter: retratserif, Assigned: kinetik)

References

Details

(Whiteboard: [LOE:S])

Attachments

(1 file, 1 obsolete file)

p1.1: push empty AudioData when first WebM audio packet is empty
2.55 KB, patch
cpearce
: review+
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:11.0) Gecko/20100101 Firefox/11.0
Build ID: 20120310010446



Actual results:

Audio and video playback seeking not possible if there isn't "Accept-Ranges: bytes" header in server response.
For example this remote webm video: http://ubuntuone.com/3OI1LmeSsamL7caWH7jlWy

According to the rfc-2616, "origin servers that accept byte-range requests MAY send 'Accept-Ranges: bytes' but are not required to do so. Clients MAY generate byte-range requests without having received this header for the resource involved." ( http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.5 )
OS: Linux → All
Hardware: x86 → All
Component: Untriaged → Video/Audio
Product: Firefox → Core
(In reply to rekcuFniarB from comment #0)
> For example this remote webm video: http://ubuntuone.com/3OI1LmeSsamL7caWH7jlWy

Seeking forward is not possible even if server supports partial content.
QA Contact: untriaged → video.audio
There seem to be a number of things going wrong here.  mkvinfo shows a duration of 1577.052, but Firefox has failed to retrieve it.  buffered.start(0) reports a very bogus looking number (18446744073.70655), but buffered.end(0) looks valid.  With buffered.end(0) reporting ~700, I set currentTime to ~1500 and it started reporting a bogus value (18446744073.70655).
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to rekcuFniarB from comment #0)
> According to the rfc-2616, "origin servers that accept byte-range requests
> MAY send 'Accept-Ranges: bytes' but are not required to do so. Clients MAY
> generate byte-range requests without having received this header for the
> resource involved." (
> http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.5 )

Right.  So we test for range support by starting initial video loads with a range request of "0-", which should cause the server to respond with a 206 (partial response code), or a 200 with Accept-Ranges: bytes.  It's still possible that a server might support ranges but, with this particular request, return a bare 200 and make it impossible for us to detect.

The problem you're seeing is that we don't use this "0-" range request when a video is loaded in a top-level document, because the load is started outside of the media code's control.  That's bug 575459.  If you load the supplied URL via a <video> element, seeking and duration work fine.

Since there's already a bug covering that, I'm going to morph this bug to cover the other problems revealed by the linked video:

1. Duration reported as infinite despite the WebM container providing a valid duration.

2. Start of first buffered range is bogus.

3. Forcing a seek by setting currentTime results in a bogus currentTime.
(In reply to Matthew Gregan [:kinetik] from comment #3)
> 1. Duration reported as infinite despite the WebM container providing a valid duration.

A stream marked as infinite reports its duration as infinite even if it has a valid duration:
http://mxr.mozilla.org/mozilla-central/source/content/media/nsBuiltinDecoder.cpp#88

Stream is set to infinite here:
http://mxr.mozilla.org/mozilla-central/source/content/media/MediaResource.cpp#231

And since it's not seekable, it's not cleared again here:
http://mxr.mozilla.org/mozilla-central/source/content/media/MediaResource.cpp#265

> 2. Start of first buffered range is bogus.

> 3. Forcing a seek by setting currentTime results in a bogus currentTime.

Both of these appear to be caused by the same thing, which is that the decoder state machine calculates the start time of the media as 3ms.  In the buffer calculation, this is subtracted from the timecode of the first buffered range to normalize it on to a 0..duration timeline, but the first range's timecode is 0, resulting in underflow.

This happens because the audio appears first in timecode order:

| + Cluster timecode: 0.000s
| + SimpleBlock (key, track number 2, 1 frame(s), timecode 0.000s = 00:00:00.000) at 4496
|  + Frame with size 1
| + SimpleBlock (key, track number 2, 1 frame(s), timecode 0.003s = 00:00:00.003) at 4503
|  + Frame with size 1
...
| + SimpleBlock (key, track number 1, 1 frame(s), timecode 0.040s = 00:00:00.040) at 27344
|  + Frame with size 22052

But the Vorbis decode does not produce output frames until the second audio packet is fed in (i.e., it has a preroll of 2), so the first decoded AudioData added to the audio queue is produced by the second audio packet, giving it a start time of 3ms.

This bug will occur with any WebM file where the audio precedes the video in timecode order.  I'm surprised it hasn't shown up until now.
(In reply to Matthew Gregan [:kinetik] from comment #3)
>If you load the supplied URL via a <video> element, seeking and duration work fine.
> 

I created a test page with a video from that server, seeking ang duration work only if I reload the page (even with ctrl+f5) but don't work when the page is loaded for first time.
(In reply to rekcuFniarB from comment #5)
> I created a test page with a video from that server, seeking ang duration
> work only if I reload the page (even with ctrl+f5) but don't work when the
> page is loaded for first time.

Forgot to give the link to that page: http://utnubuk.info/html5AVtest/#content
(In reply to Matthew Gregan [:kinetik] from comment #3)
> 1. Duration reported as infinite despite the WebM container providing a
> valid duration.
> 
> 2. Start of first buffered range is bogus.
> 
> 3. Forcing a seek by setting currentTime results in a bogus currentTime.

Same problem occurs with ogg audio. Audio playback is not seekable if the page was opened for the first time. Example page: http://utnubuk.info/html5ATest/#content
Paul do you know if this bug has the same root as bug 740124?
Need to add that this problem doesn't exist on the nightly builds 14 and 16.
Vivien : probably not, see comment 4.
blocking-basecamp: --- → ?
blocking-basecamp: ? → ---
Dominic, can you explain why this is blocking Music app in B2G?
> 1. Duration reported as infinite despite the WebM container providing a valid duration.
>
> 2. Start of first buffered range is bogus.
>
> 3. Forcing a seek by setting currentTime results in a bogus currentTime.

I got same problem with mp3 in B2G.
And I am using window.URL.createObjectURL() which returns a blob as the src of <audio>.
blocking-basecamp: --- → +
Chris Double, can you please take a look?
Assignee: nobody → chris.double
(In reply to Dominic Kuo from comment #12)
> I got same problem with mp3 in B2G.
> And I am using window.URL.createObjectURL() which returns a blob as the src
> of <audio>.

All of these are WebM specific bugs (see comment 4), so whatever you're seeing, it's not this bug.  Please file a new bug, unless you can confirm you're not running into issues caused by bug 730765.
(In reply to Matthew Gregan [:kinetik] from comment #3)
> 1. Duration reported as infinite despite the WebM container providing a
> valid duration.

Spun off bug 788067 for this.

> 2. Start of first buffered range is bogus.
> 
> 3. Forcing a seek by setting currentTime results in a bogus currentTime.

And this bug is now about these, which are the same root cause.  See comment 4.
Summary: html5 audio and video playback seeking issue → Invalid timecode calculation in WebM when media starts with an audio frame that has a non-zero start time
Whiteboard: [WebAPI:P0]
Assignee: chris.double → kinetik
Whiteboard: [WebAPI:P0] → [WebAPI:P0][LOE:S]
I believe this bug was made a blocker to basecamp when we thought this was blocking the Music app (MP3) in B2G.  However, this bug affects WebM only (not MP3).  Do we still consider this a blocker to basecamp?
blocking-basecamp: + → ?
(In reply to Maire Reavy [:mreavy] from comment #16)
> However, this bug affects WebM only
It affects also OGG.
The video player supports webm and I'm pretty sure it's a v1 feature.
blocking-basecamp: ? → +
(In reply to rekcuFniarB from comment #17)
> (In reply to Maire Reavy [:mreavy] from comment #16)
> > However, this bug affects WebM only
> It affects also OGG.

To clarify: which issue (from the bottom of comment 3) are you seeing with Ogg?  Issue #1 became bug 788067, and the others are certainly WebM specific.  Or is it none of those?
(In reply to Matthew Gregan [:kinetik] from comment #19)
> To clarify: which issue (from the bottom of comment 3) are you seeing with Ogg?
Same thing happens, playback seeking and duration aren't available if there is no "Accept-Ranges: bytes" header in the server response.
Example: http://utnubuk.info/html5ATest/
Here is what I see (screenshot): http://storage5.static.itmages.ru/i/12/0928/h_1348855944_2012772_1daa19c66b.png
Okay, thanks.  That issue became bug 788067 and this bug covers the WebM timecode issues.  In hindsight it would've been better to spin off a new bug for the WebM issues, sorry for the confusion.
Priority: -- → P2
Whiteboard: [WebAPI:P0][LOE:S] → [LOE:S]
(In reply to Maire Reavy [:mreavy] from comment #16)
> I believe this bug was made a blocker to basecamp when we thought this was
> blocking the Music app (MP3) in B2G.  However, this bug affects WebM only
> (not MP3).  Do we still consider this a blocker to basecamp?

Yeah, I don't understand why this is a blocker.  Can someone please explain it to me?
Chris Lee, should this be a blocker?  See comment #16 and comment #22.
Flags: needinfo?(clee)
Milestoning for C2 (deadline of 12/10), as this meets the criteria of "known P2 bugs found before or during C1".
Target Milestone: --- → B2G C2 (20nov-10dec)
Based on the comments in this bug it doesn't sound like it blocks B2G v1.  If anyone thinks it does, please re-nom.
blocking-basecamp: + → -
Flags: needinfo?(clee)
Assignee: kinetik → nobody
Blocks: 1072483
Assignee: nobody → kinetik
Status: NEW → ASSIGNED
Push an empty AudioData instance to the AudioQueue when first WebM audio packet is empty.  Without this, the media start time is derived from the first packet that produces audio, but that may have a non-zero start time in badly muxed files.
Attachment #8494940 - Flags: review?(cpearce)
Comment on attachment 8494940 [details] [diff] [review]
p1: push empty AudioData when first WebM audio packet is empty

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

::: content/media/webm/WebMReader.cpp
@@ +591,5 @@
> +      // start time is calculated.  Otherwise we'd end up with a media start
> +      // time derived from the timecode of the first packet that produced
> +      // data.
> +      if (frames == 0 && mAudioFrames == 0) {
> +        AudioQueue().Push(new AudioData(aOffset, 0, 0, 0, nullptr, mChannels, rate));

Is the start time for the first WebM sample always guaranteed to be 0?
No, good catch.  The AudioData was supposed to be initialized with the packet timestamp, but that got lost along the way.

tstamp_usecs is equivalent to time.value() for the regular AudioData init, except that there's no need to add total_duration since we know it must be zero at this point.
Attachment #8494940 - Attachment is obsolete: true
Attachment #8494940 - Flags: review?(cpearce)
Attachment #8494955 - Flags: review?(cpearce)
Attachment #8494955 - Flags: review?(cpearce) → review+
https://hg.mozilla.org/mozilla-central/rev/7194c8a20e00
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: