Seeking in an MP3 file plays silence for a while

VERIFIED FIXED in Firefox 54

Status

()

Core
Audio/Video: Playback
VERIFIED FIXED
10 months ago
7 months ago

People

(Reporter: roc, Assigned: esawin)

Tracking

unspecified
mozilla54
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox54 verified)

Details

Attachments

(1 attachment, 1 obsolete attachment)

1) Load http://acpc.org.nz/images/English/Sermon/2017_290117.mp3
2) Click in the controls to seek near the middle of the file

Silence plays for a while and then audio eventually resumes. There are no prolonged periods of silence in this file so this is a bug. You don't get the same silence using, say, Chrome.

mozilla-central opt build 80eac484366a.
Got this warning after seeking: [Child 75554] WARNING: Decoder=12355e800 state=SEEKING Audio not synced after seek, maybe a poorly muxed file?

When I seek to 720s, the timestamp of the 1st audio sample returned is 715768922. The 5s gap is filled with silence when the media element plays from 720s after seeking.

Something wrong to the MP3 demuxer?
Flags: needinfo?(jyavenard)
:esawin is on this
Flags: needinfo?(jyavenard)
timestamps after seeking are always estimated from the average bitrate, so it will never be exactly what you expect it to be.

That the MDSM fill with silence if you were to seek to 715s and got 720s would make sense.

But seeking to 720s, getting 715s as first frame to then fill with silence for 5s certainly doesn't. The MDSM should simply continue to request audio data until it get what it wants.
Sorry, I got it wrong.

The seek target is 710s and the 1st audio sample is 715768922 which is 5s after the seek target.
(Assignee)

Comment 5

10 months ago
Created attachment 8835632 [details] [diff] [review]
0001-Bug-1337556-1.0-Add-rewind-heuristic-when-seeking-pa.patch

For big VBR files, the provided TOC yields highly inaccurate offsets.

In this case, the file is at ~50MB, which yields a TOC resolution of ~505kB or ~636 frames.
In such extreme cases, it would be more accurate to rely on average-frame-based seeking, like we would with CBR files, instead of using TOC-based seeking.

For now, I'm leaving the logic as it is, but improve the ScanUntil correction procedure by adding a heuristic rewind on overshooting.
Assignee: nobody → esawin
Attachment #8835632 - Flags: review?(jyavenard)
Comment on attachment 8835632 [details] [diff] [review]
0001-Bug-1337556-1.0-Add-rewind-heuristic-when-seeking-pa.patch

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

::: dom/media/MP3Demuxer.cpp
@@ +259,4 @@
>    }
>  
>    if (Duration(mFrameIndex) > aTime) {
> +    // We've seeked past the target time, rewind back a little to correct it.

how going back unconditionally corrects it in any way?
seems a bit ugly to me

@@ +259,5 @@
>    }
>  
>    if (Duration(mFrameIndex) > aTime) {
> +    // We've seeked past the target time, rewind back a little to correct it.
> +    const int64_t rewind = 0.01 * aTime.ToMicroseconds();

aTime.ToMicroseconds() / 100;

as otherwise you get a 24 bits (size of a 32 bits float mantissa) rounding which could be bad.
Attachment #8835632 - Flags: review?(jyavenard) → review+
(Assignee)

Comment 7

10 months ago
Created attachment 8836038 [details] [diff] [review]
0001-Bug-1337556-1.1-Add-rewind-heuristic-when-seeking-pa.patch

Addressed comments.

As discussed on IRC, we might want to look into relying more on the TOC, when provided, instead of only using it for offset approximation to allow for time-accurate seeking.
Attachment #8835632 - Attachment is obsolete: true
Attachment #8836038 - Flags: review+

Comment 8

10 months ago
Pushed by esawin@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/13a6a2e4e67e
[1.1] Add rewind heuristic when seeking past the target time. r=jya

Comment 9

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/13a6a2e4e67e
Status: NEW → RESOLVED
Last Resolved: 10 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Flags: qe-verify+
I have reproduced the issue on a Nightly 54.0a1 (id: 20170207110148) build and can confirm that the issue is fixed in Firefox 54.0b1 on an Ubuntu 14.04 x64 OS.
Status: RESOLVED → VERIFIED
status-firefox54: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.