Open
Bug 1423834
Opened 7 years ago
Updated 2 years ago
Wrong calculation for mp3 time length as we prevent overflow
Categories
(Core :: Audio/Video: Playback, enhancement, P3)
Core
Audio/Video: Playback
Tracking
()
NEW
People
(Reporter: chunmin, Assigned: chunmin)
Details
The way we prevent overflow will cause the miscalculation of mp3 length.
Suppose we have x_1, x_2, ..., x_n and the new data `x_{n+1}` will lead to overflow.
The average of these n+1 data is: (x_1 + x_2 + ... + x_n + x_{n+1}) / (n + 1)
Currently, we'll halve the summation and the number of data before counting `x_{n+1}` and use the *half* summation and the number to calculate average by[0]:
(X + x_{n+1}) / (N + 1), where X = (x_1 + x_2 + ... + x_n) / 2 and N = n / 2.
Let E be the above estimation, E = (X + x_{n+1}) / (N + 1) =
((x_1 + x_2 + ... + x_n)/2 + x_{n+1}) / (n/2 + 1) =
((x_1 + x_2 + ... + x_n + 2*x_{n+1})/ 2) / ((n+2)/2) =
(x_1 + x_2 + ... + x_n + 2*x_{n+1}) / (n + 2)
Let Avg be the true average, Avg = (x_1 + x_2 + ... + x_n + x_{n+1}) / (n + 1)
We could compare this two values to see if they are equal:
Avg
= (n + 2)(x_1 + x_2 + ... + x_n + x_{n+1}) / (n + 1)(n + 2)
= [(n + 1)(x_1 + x_2 + ... + x_n + x_{n+1}) + (x_1 + x_2 + ... + x_n + x_{n+1})] / (n + 1)(n + 2)
E
= (n + 1)(x_1 + x_2 + ... + x_n + x_{n+1} + x_{n+1}) / (n + 1)(n + 2)
= [(n + 1)(x_1 + x_2 + ... + x_n + x_{n+1}) + (n + 1) * x_{n+1}] / (n + 1)(n + 2)
Avg - E = [(x_1 - x_{n+1}) + (x_2 - x_{n+1}) + ... + (x_n - x_{n+1})] / (n + 1)(n + 2).
They are equal *only* when x_{n+1} = x_1 = x_2 = ... = x_n.
Cumulative moving average[1] is a better choice to avoid the overflow.
Avg(n+1)
= (n * Avg(n) + new-data) / (n+1)
= ((n+1) * Avg(n) - Avg(n) + new-data) / (n+1)
= Avg(n) + (new-data - Avg(n)) / (n+1)
It's a better way to get average if the `n` won't overflow.
(Sorry about the ugly format. I wish I can use mathjax or latex here)
[0] https://searchfox.org/mozilla-central/rev/f5f1c3f294f89cfd242c3af9eb2c40d19d5e04e7/dom/media/mp3/MP3Demuxer.cpp#709-728,756-760
[1] https://en.wikipedia.org/wiki/Moving_average#Cumulative_moving_average
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → cchang
Comment 1•7 years ago
|
||
there's a few classes to calculate an average that won't have the problem you describe.
https://searchfox.org/mozilla-central/source/dom/media/MediaFormatReader.h#587
there's a few more like that.
Updated•7 years ago
|
Priority: -- → P3
Assignee | ||
Comment 2•7 years ago
|
||
To evaluate if this is worth fixing, I test several mp3 files to see if we can have a larger mTotalFrameLen[0] than StreamLength()[1]. The answer is yes. The mTotalFrameLen may be larger than StreamLength() in some file.
(If mTotalFrameLen is always smaller than StreamLength(), StreamLength() will also overflow when mTotalFrameLen overflows. Then the calculation for mp3 duration will be incorrect since StreamLength() is a overflowed value, regardless of the estimated average frame length is correct or not[2].)
[0] https://searchfox.org/mozilla-central/rev/f5f1c3f294f89cfd242c3af9eb2c40d19d5e04e7/dom/media/mp3/MP3Demuxer.h#143
[1] https://searchfox.org/mozilla-central/rev/f5f1c3f294f89cfd242c3af9eb2c40d19d5e04e7/dom/media/mp3/MP3Demuxer.h#50
[2] https://searchfox.org/mozilla-central/rev/f5f1c3f294f89cfd242c3af9eb2c40d19d5e04e7/dom/media/mp3/MP3Demuxer.cpp#389,395
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•