1669503 - Improve mp3 trimming

Status: RESOLVED FIXED

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

Description

[Paul Adenot (:padenot)]

In bug 1566389, mp3 decoding was made almost correct. It's not correct if the end trimming spans two packets, like it is in this file: https://github.com/juj/audio_test_suite/blob/master/8bit_detective/8bit_detective_8000hz_8kbs_mono_lame3.99.mp3, that has a duration of 28.65675s, but Firefox reports 28.6618820861678s, it's missing 41 frames at the end, after having trimmed the last packet, it should have trimmed the previous one.

Comment 1

[Paul Adenot (:padenot)]

Attachment: Bug 1669503 - Handle mp3 encoder padding when the padding duration spans more than one packet. r?padenot

Comment 2

[Paul Adenot (:padenot)]

Attachment: Bug 1669503 - Handle mp3 encoder padding when the padding duration spans more than one packet. r?jya

We can't handle this at the decoder level, because the decoder doesn't know that
a particular packet it's seeing is the second to last packet and it should start
trimming the end of this packet because the encoder padding spans multiple
packet.

Comment 3

[Paul Adenot (:padenot)]

Attachment: Bug 1669503 - Add various test cases for mp3 trimming. r?jya

Depends on D94469

Comment 4

[Paul Adenot (:padenot)]

jya, can you r+ the patches here and then we can land? It's your patch and my test, fixed.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d5cbc458d318
Handle mp3 encoder padding when the padding duration spans more than one packet. r=jya
https://hg.mozilla.org/integration/autoland/rev/863ecb9b15d5
Add various test cases for mp3 trimming. r=jya

Comment 6

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/d5cbc458d318
https://hg.mozilla.org/mozilla-central/rev/863ecb9b15d5

Comment 7

[Razvan Maries]

Backed out for crashing in mozilla::MP3TrackDemuxer::GetNextFrame: https://hg.mozilla.org/mozilla-central/rev/d7d7fd7434a6f8945394e304b21bb23c727bd04b

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

For crashes like bp-d2311904-dee6-4e6c-9ea5-8fcd50201106 - [@ mozilla::MP3TrackDemuxer::GetNextFrame]

Comment 9

[Chris H-C :chutten]

Attachment: Backed out 2 changesets (Bug 1669503) for crashing in mozilla::MP3TrackDemuxer::GetNextFrame. a=backout DONTBUILD

Backed out changeset 863ecb9b15d5 (bug 1669503)
Backed out changeset d5cbc458d318 (bug 1669503)

Comment 10

[Paul Adenot (:padenot)]

Attachment: Bug 1669503 - Don't try to trim an mp3 packet if it's not well formed. r?jya

Comment 11

[Paul Adenot (:padenot)]

Fixed by the big patch series in bug 1817997 and others.