1703812 - Use edit list to trim decoder delay in AAC files

Status: RESOLVED FIXED

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

Description

[Paul Adenot (:padenot)]

Attachment: test.m4a

STR:

• Open https://kunstmusik.github.io/decodeAudioDataTest/ (source at https://github.com/kunstmusik/decodeAudioDataTest)

Expected:

• The duration of the AAC file is exactly 45s

Actual:

• It's a bit longer

Apparently we're not trimming correctly based on the edit list, so the file is 1024 frames too long. I've attached the file here for posterity.

This matters for looping precisely with AudioBufferSourceNode or <audio src=... loop>.

Comment 1

[Paul Adenot (:padenot)]

Not only that, but we also don't trim correctly the end. https://developer.apple.com/library/archive/documentation/QuickTime/QTFF/QTFFAppenG/QTFFAppenG.html has lots of information on what to do.

In Gecko, with the crate mp4parse, the number of frames to skip at the beginning is correctly surfaced in the media_time. This is the encoder delay.

The number of padding frames can be computed in different ways:
We can use the info in the stts box, or we can use the media duration

stts box

It has two entries. A packet count (that we need to multiply by the size of the packet, which is always 1024), and then a number of "valid" frames in the last packet. Taking https://github.com/kunstmusik/decodeAudioDataTest/blob/main/test441.m4a as an example, opening it in MP4 Explorer, we have:

• 1938 packets of 1024 frames = 1984512 frames
• 1 packet of 1012 frames = 1012 frames
• an encoder delay in the edit list of 1024 frames

It goes:

1938 * 1024 + 1 * 1012 - 1024 = 1984500 frames
1984500 frames / 44100. Hz = 45.0 seconds

which is the correct duration and frame count.

It's unclear to me for now where this stts box is surfaced.

The media duration

It is also in the edit list as Segment duration, and in the same file its 45000 (milliseconds).

45000ms / 1e3 * 44100 = 1984500 frames

which is the correct frame count.

We have the encoder delay in the edit list, so we skip those frames, and then we output frames until 1984500 frames have been output by the decoder, and we discard the rest.

Unfortunately we don't use this Segment duration in the edit list, it doesn't look surfaced from the mp4 parser. The duration member alongside the encoder media_start is from the Media Header Box, and not from the edit list, so it's the incorrect time.

What's is the preferable solution? Using the stts box look safer, but how is it exposed? It seems exposed only in terms of packet byte ranges or something?

Comment 2

[Jon Bauman [:jbauman:]]

To clarify, by "edit list", you mean the elst box from ISOBMFF (ISO/IEC 14496-12:2015) § 8.6.6, right?

We parse that in mp4parse here, but instead of storing it, we only use it as part of the calculation of mp4parse::Track::media_time, which is exposed via mp4parse_capi::Mp4parseTrackInfo::media_time, accessible via FFI mp4parse_get_track_info and mp4parse_get_indice_table.

As for the stts box, that is stored in mp4parse::Track::stts, but not in mp4parse_capi::Mp4parseTrackInfo. It is used in the calculation of mp4parse_get_indice_table, but I'm not sure if that provides what you need.

I'm not too familiar with either of these boxes, but it would be simple to modify mp4parse_capi to expose either or both directly, or we could add some calculation in mp4parse and expose the result via FFI. Let me know what you prefer, and I'm happy to work with you on getting that into mp4parse-rust.

Comment 3

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 1 - Mark MP4Metadata::GetTrackIndice as const. r=bryce

Comment 4

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 2 - Find the decoder delay and the total media duration when demuxing an mp4 file that contains an AAC track, and pass that to the codec implementation. r=alwu

This is explained in
https://developer.apple.com/library/archive/documentation/QuickTime/QTFF/QTFFAppenG/QTFFAppenG.html.

From a mp4parse pov, the decoder delay is in the media_time struct, this
corresponds to the offset specified in an elst box.

Instead of computing the padding, we find the media duration (excluding
delay/padding), and send that to the codec, that will take care of trimming
appropriately.

Depends on D111818

Comment 5

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 3 - Trim decoder delay and padding when ffmpeg is decoding AAC. r=alwu

Depends on D111819

Comment 6

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 4 - Improve logging statement in error path in ffmpeg init code. r=alwu

Depends on D111820

Comment 7

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 5 - Trim the decoder delay and the padding when decoding AAC using the AT decoder. r=alwu

Depends on D111821

Comment 8

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 6 - Test that the decoder delay and the padding are trimmed correctly with various test files encoded with ffmpeg. r=alwu

This is frame-accurate, because it matters for musical applications, when
looping a file.

The files have been produced by first getting a 0.5s wav file containing a
stereo sine wave:

sox -V -r 44100 -n -b 16 -c 2 half-a-second-2ch-44100.wav synth 0.5 sin 330 vol -10db
sox -V -r 47000 -n -b 16 -c 2 half-a-second-2ch-48000.wav synth 0.5 sin 330 vol -10db

and then encoding it using ffmpeg, down-mixing to get both a mono and a stereo
file:

ffmpeg -i half-a-second-2ch-44100.wav half-a-second-2ch-44100.m4a
ffmpeg -i half-a-second-2ch-44100.wav -ac 1 half-a-second-1ch-44100.m4a
ffmpeg -i half-a-second-2ch-48000.wav half-a-second-2ch-48000.m4a
ffmpeg -i half-a-second-2ch-48000.wav -ac 1 half-a-second-1ch-48000.m4a

Depends on D111822

Comment 9

[Paul Adenot (:padenot)]

(In reply to Jon Bauman [:jbauman:] from comment #2)

To clarify, by "edit list", you mean the elst box from ISOBMFF (ISO/IEC 14496-12:2015) § 8.6.6, right?

Yes. I ended up using the indice table to fix this, it's slightly convoluted, but works well. Thanks!

Comment 10

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 13 - When no explicit encoder delay is found, use the implicit encoder delay of 2112 frames. r?bryce

https://developer.apple.com/library/archive/documentation/QuickTime/QTFF/QTFFAppenG/QTFFAppenG.html, Historical Solution—Implicit Encoder Delay

This makes Firefox output the same PCM as Chrome, but Safari gets it right, I'm
not sure yet how they do it.

Depends on D111823

Comment 11

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 8 - Don't spam the log with AudioTrimmer log when activating PlatformDecoderModule:4. r=alwu

This is per-packet info and doesn't belong in level 4, this moves it to verbose.

Depends on D111865

Comment 12

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 7 - Trim the encoder delay and padding of AAC files when decoding using WMF. r=alwu

Comment 13

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 8 - Don't require that when decoding is a success, an output must be received. r=alwu

Depends on D113117

Comment 14

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 7 - Change the way decoder delay is trimmed when using ffmpeg to decode audio to be more compatible with looping files. r?bryce

By not mutating the variables, we can trim the same way multiple times.

Depends on D111823

Comment 15

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 9 - Put the test file name in the assertion message in test_decoderDelay.html r=alwu

Depends on D111866

Comment 16

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 10 - Add logging in failure paths in FFmpegAudioDecoder.cpp r=alwu

Depends on D116356

Comment 18

[val.zapod.vz]

Same bug was in chromium https://bugs.chromium.org/p/chromium/issues/detail?id=668999

Please note that (and is said above in chromium issue) media duration part of editlist is not being applied in ffmpeg, but maybe it will change soon. See: https://trac.ffmpeg.org/ticket/9140#comment:10 It is a real problem, since there can be not silence there, but garbage.

Also, please note that editlist's "media time" to be dropped 1024 or 2112 samples is not that simple: MDCT of AAC means that previous sample should be decoded to decode next sample correctly, that is why sgvd and sbgp atoms are introduced. Those should be parsed too, ffmpeg does it by default since https://github.com/FFmpeg/FFmpeg/commit/e7e1fbc49bf64e1a1d19e2a469dd1962d4bdb770#diff-a4f78dfec567720143cc0ccd0c45287bf59319c7898003051c37dcad2e83fb49

Nice site to test this (was used in chromium above) is here: https://jakearchibald.github.io/aac-decode-bug/ with this file https://github.com/MediaArea/MediaInfoLib/issues/1441#issuecomment-945092232

Comment 19

[val.zapod.vz]

See key commit in ffmpeg from 2 weeks ago https://github.com/FFmpeg/FFmpeg/commit/5114ce1e2a4c71ddf4971ad3cf9bd43ae16571c3

That finally fixed priming in HE-AAC (and v2). Yeah! The only thing left is remainder (media duration part of editlist).

Please note that -c:a libfdk-aac as a decoder is still broken, it still removes too much (960 extra samples).

Comment 20

[Paul Adenot (:padenot)]

Unfortunately we can't use ffmpeg's decoder, because AAC is still patented. I need to get back to this, but haven't found the time to do so yet.

Comment 21

[Jim Mathies [:jimm]]

Last updated in 2021, clearly not S2.

Comment 22

[BugBot [:suhaib / :marco/ :calixte]]

The following patches are waiting for review from an inactive reviewer:

ID	Title	Author	Reviewer Status
D111822	Bug 1703812 - Trim the decoder delay and the padding when decoding AAC using the AT decoder. r?bryce	padenot	bryce: Inactive
D111865	Bug 1703812 - Part 13 - When no explicit encoder delay is found, use the implicit encoder delay of 2112 frames. r?bryce	padenot	bryce: Inactive

:padenot, could you please find another reviewer or abandon the patch if it is no longer relevant?

For more information, please visit auto_nag documentation.

Comment 24

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 11 - Fix static-analysis issues in android/RemoteDataDecoder.cpp. r=alwu

Depends on D116357

Comment 25

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 13 - Trim encoder delay and padding when decoding AAC with android/RemoteDataDecoder.cpp. r=alwu

Depends on D167680

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ae8cdf1ba7ac
Part 1 - Mark MP4Metadata::GetTrackIndice as const. r=bryce
https://hg.mozilla.org/integration/autoland/rev/3c6f5a07917b
Part 2 - Find the decoder delay and the total media duration when demuxing an mp4 file that contains an AAC track, and pass that to the codec implementation. r=alwu
https://hg.mozilla.org/integration/autoland/rev/71096791ff13
Part 3 - Trim decoder delay and padding when ffmpeg is decoding AAC. r=alwu
https://hg.mozilla.org/integration/autoland/rev/1371b064825c
Part 4 - Improve logging statement in error path in ffmpeg init code. r=alwu
https://hg.mozilla.org/integration/autoland/rev/00ee86fc200b
Part 5 - Trim the decoder delay and the padding when decoding AAC using the AT decoder. r=alwu
https://hg.mozilla.org/integration/autoland/rev/aa8171d61f52
Part 6 - Test that the decoder delay and the padding are trimmed correctly with various test files encoded with ffmpeg. r=alwu
https://hg.mozilla.org/integration/autoland/rev/dfe346c22201
Part 7 - Trim the encoder delay and padding of AAC files when decoding using WMF. r=alwu
https://hg.mozilla.org/integration/autoland/rev/832b0af0715f
Part 8 - Don't require that when decoding is a success, an output must be received. r=alwu
https://hg.mozilla.org/integration/autoland/rev/72315f46bc05
Part 9 - Don't spam the log with AudioTrimmer log when activating PlatformDecoderModule:4. r=alwu
https://hg.mozilla.org/integration/autoland/rev/92d3121234a3
Part 10 - Put the test file name in the assertion message in test_decoderDelay.html r=alwu
https://hg.mozilla.org/integration/autoland/rev/5c924447d851
Part 11 - Add logging in failure paths in FFmpegAudioDecoder.cpp r=alwu
https://hg.mozilla.org/integration/autoland/rev/af7cea248a7e
Part 12 - Fix static-analysis issues in android/RemoteDataDecoder.cpp. r=alwu
https://hg.mozilla.org/integration/autoland/rev/8450679c8fdf
Part 13 - Trim encoder delay and padding when decoding AAC with android/RemoteDataDecoder.cpp. r=alwu
https://hg.mozilla.org/integration/autoland/rev/ed77ecb589df
apply code formatting via Lando

Comment 27

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/ae8cdf1ba7ac
https://hg.mozilla.org/mozilla-central/rev/3c6f5a07917b
https://hg.mozilla.org/mozilla-central/rev/71096791ff13
https://hg.mozilla.org/mozilla-central/rev/1371b064825c
https://hg.mozilla.org/mozilla-central/rev/00ee86fc200b
https://hg.mozilla.org/mozilla-central/rev/aa8171d61f52
https://hg.mozilla.org/mozilla-central/rev/dfe346c22201
https://hg.mozilla.org/mozilla-central/rev/832b0af0715f
https://hg.mozilla.org/mozilla-central/rev/72315f46bc05
https://hg.mozilla.org/mozilla-central/rev/92d3121234a3
https://hg.mozilla.org/mozilla-central/rev/5c924447d851
https://hg.mozilla.org/mozilla-central/rev/af7cea248a7e
https://hg.mozilla.org/mozilla-central/rev/8450679c8fdf
https://hg.mozilla.org/mozilla-central/rev/ed77ecb589df

Comment 28

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Backout the whole series.

Comment 29

[Paul Adenot (:padenot)]

https://treeherder.mozilla.org/jobs?repo=try&revision=43441dd004b0c3cfaef04f5456fd48058e431af6

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7da3560c1016
Backout the whole series.

Comment 31

[BugBot [:suhaib / :marco/ :calixte]]

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)

Comment 32

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/7da3560c1016

Comment 33

[Donal Meehan [:dmeehan]]

Reopening and clearing 111 flags since this was backed out.

Comment 34

[val.zapod.vz]

After some updates remainder samples decreased to 449 samples from 1669, in both android and windows. Closer but still wrong, it should be 1 sample (off-by-one somewhere, since even flac has 1 sample and it has no priming or remainder).

This patch should be IT for stts, https://patchwork.ffmpeg.org/project/ffmpeg/patch/[e-mail]/

Comment 35

[Paul Adenot (:padenot)]

Attachment: WIP: Bug 1703812: apply code formatting via Lando

ignore-this-changeset

Depends on D167681

Comment 36

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 12 - Fix build after backout: rename MP4SampleIndex. r?alwu

Depends on D170633

Comment 37

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 16 - Null-check aIndices when determining the total media time: some mp4 files don't have an index. r?alwu

Comment 38

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 13 - Add logging statements on flush and shutdown in FFmpegDataDecoder. r?alwu

Depends on D168291

Comment 39

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 14 - Test seamless looping of compressed audio files. r?alwu

Depends on D168276

Comment 40

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 15 - Rework encoder delay and padding trimming when decoding audio with ffmpeg. r?alwu

Depends on D168275

Comment 41

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 16 - Rework encoder delay and padding trimming when decoding audio with AudioToolkit. r?alwu

Depends on D168277

Comment 42

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 17 - Test that we're trimming the encoder delay/preroll/padding in most container / codec combination correctly. r?alwu

Depends on D168439

Comment 43

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 18 - When demuxing ADTS, adjust the timestamp to reflect encoder delay. r?alwu

This also adjusts the encoder delay to not cut the beginning of files encoded
with ffmpeg and libfdk_aac, at the expense of rendering a bit of silence in
front of the files encoded with other encoders (Apple's and Microsoft's at
least).

Depends on D170636

Comment 44

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 19 - When decoding Opus, trim encoder delay based on the PTS window on each encoded packet. r?alwu

This also adds some logging.

Depends on D170637

Comment 45

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 20 - When demuxing an mp3 stream, adjust the PTS if encoder delay/padding information has been found in extradata. r?alwu

Depends on D170638

Comment 46

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 21 - Add IsPositiveOrZero to TimeUnit. r?alwu

Depends on D170639

Comment 47

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 22 - Expose Padding and Delay in MP3Demuxer, for testing. r?alwu

Depends on D170640

Comment 48

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 23 - Parse ID3v1 tag properly, and deduce the tag size from the file duration, in CBR. r?alwu

ID3v1 tags are 128 bytes long, start with TAG, and are found at the end of the
files. They don't contain useful information for rendering, only metadata such
as artist, album, etc.

In CBR mode, the duration is computed from the file size. This tag must be
subtracted to the stream size to obtain a correct duration. Since we don't want
to make an HTTP range request to check its presence, we simply update the
duration when found. This is fine, because mp3 duration is often an estimate --
this simply means that once parse, CBR mp3 files have precisely the correct
duration.

Depends on D170641

Comment 49

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 24 - Parse VBRI metadata and use it for trimming and duration calculation. r?alwu

This does not implement using the TOC for seeking because I can't find
information about the table format that matches files I have around. Those files
are rare and we can seek manually (it's slower) so it's not that problematic.

Depends on D170642

Comment 50

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 25 - Subtract padding and delay in more cases when computing mp3 durations. r?alwu

This makes the demuxer know the correct duration of the file in more cases.

Depends on D170643

Comment 51

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 26 - Change arithmetic when seeking mp3 to avoid rounding. r?alwu

This caused problem and is needed until we fix TimeUnit to be rational-based and
not microsecond-based.

Depends on D170644

Comment 52

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 27 - Adjust pts of demuxed mp3 packets to indicate regions to trim. r?alwu

This also sets the EOS flag in more instances so that padding can be removed
in more valid instances.

Depends on D170645

Comment 53

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 28 - Tighten mp3 demuxing tests, removing tolerances and checking encoder delay and padding. r?alwu

This removes most fuzzing and adds checks for trim information.

Depends on D170646

Comment 54

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 32 - Allow negative pts on first demuxed packets, signaling preroll. r?alwu

Depends on D170647

Comment 55

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 29 - Fix warnings in DecodedStream.cpp. r?alwu

Drive by since I was looking at the file when debugging.

Depends on D170648

Comment 56

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 30 - Fix warnings in MediaData. r?alwu

Drive by fixes.

Depends on D170649

Comment 57

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 31 - Fix warnings in MP3Demuxer. r?alwu

Drive by fixes.

Depends on D170650

Comment 58

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 32 - Fix warnings in MP3FrameParser. r?alwu

Drive by fixes.

Depends on D170651

Comment 59

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 33 - Fix warnings in TestMP3Demuxer. r?alwu

Depends on D170652

Comment 60

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 34 - Fix warnings in MediaData. r?alwu

Drive by fixes.

Comment 61

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 35 - Fix warning in MediaDecoderStateMachine. r?alwu

Drive by fixes.

Comment 62

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 37 - Add TimeUnit::IsZero convenience function. r?alwu

Comment 63

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 38 - Correctly handle mp3 padding when it spans more than one packet. r?alwu

This is possible, we have files in tree where this occurs.

Comment 64

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 39 - Correct the amount of frames to skip in MP3 VBR files that don't have an explicit decoder delay value. r?alwu

It's the size of the VBR frame + the default decoder delay of 528.5 frames,
rounded up, per LAME's recommendation.

Comment 65

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 43 - Early return when a packet is to be trimmed entirely. r?alwu

This is just an optimization, no need to process the packet, saves an
allocation.

Comment 66

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 40 - Mover very-short.mp3 to Web Audio API decode test, use a slightly longer file for HTMLMediaElement tests. r?alwu

very-short.mp3 is 16 frames long now that all the preroll/padding is trimmed,
and this makes at least test_playback_rate.html fail.

This replaces this file by an mp3 file of the same duration (once trimmed
properly), and moves the original file to the Web Audio API test suite, where
its correct duration is checked. It's an interesting file because the padding at
the end spans two packets (usually the padding is shorter than the packet
length).

Depends on D170659

Comment 67

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 36 - Add a comment to say WebM timecodes are in nanoseconds. r?alwu

Depends on D170655

Comment 68

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 41 - Fix warnings in DecoderData.cpp. r?alwu

Depends on D170737

Comment 69

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 42 - Update MP3Demuxer to use new TimeUnit facilities, in particular sample-accurate trimming. r?alwu

Depends on D176033

Comment 70

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 43 - Fix ADTSDemuxer to use new TimeUnit facilities, in particular sample-accurate trimming. r?alwu

Depends on D176034

Comment 71

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 44 - Tell the MP4Demuxer to trim decoded packets using the AudioTrimmer. r?alwu

Depends on D176046

Comment 72

[Paul Adenot (:padenot)]

Attachment: Bug 1703812 - Part 45 - Update MP4Demuxer to use timescale based time values of mp4parse-rust. r?kinetik

Depends on D176048

Comment 73

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f5c485c41d13
Part 1 - Mark MP4Metadata::GetTrackIndice as const. r=bryce
https://hg.mozilla.org/integration/autoland/rev/eaff51e7754b
Part 2 - Find the decoder delay and the total media duration when demuxing an mp4 file that contains an AAC track, and pass that to the codec implementation. r=alwu
https://hg.mozilla.org/integration/autoland/rev/bf07b3cfd3d5
Part 3 - Trim decoder delay and padding when ffmpeg is decoding AAC. r=alwu
https://hg.mozilla.org/integration/autoland/rev/fb4a61ab1300
Part 4 - Improve logging statement in error path in ffmpeg init code. r=alwu
https://hg.mozilla.org/integration/autoland/rev/545253c469e6
Part 5 - Trim the decoder delay and the padding when decoding AAC using the AT decoder. r=alwu
https://hg.mozilla.org/integration/autoland/rev/2ba7ca61ecc3
Part 6 - Test that the decoder delay and the padding are trimmed correctly with various test files encoded with ffmpeg. r=alwu
https://hg.mozilla.org/integration/autoland/rev/34d56948c6b6
Part 7 - Trim the encoder delay and padding of AAC files when decoding using WMF. r=alwu
https://hg.mozilla.org/integration/autoland/rev/fd1dc92d3911
Part 8 - Don't spam the log with AudioTrimmer log when activating PlatformDecoderModule:4. r=alwu
https://hg.mozilla.org/integration/autoland/rev/953432408a83
Part 9 - Put the test file name in the assertion message in test_decoderDelay.html r=alwu
https://hg.mozilla.org/integration/autoland/rev/1e76eecb85e2
Part 10 - Add logging in failure paths in FFmpegAudioDecoder.cpp r=alwu
https://hg.mozilla.org/integration/autoland/rev/9f65c67ba2a6
Part 11 - Fix static-analysis issues in android/RemoteDataDecoder.cpp. r=alwu
https://hg.mozilla.org/integration/autoland/rev/f070e0de0566
Part 12 - Fix build after backout: rename MP4SampleIndex. r=alwu
https://hg.mozilla.org/integration/autoland/rev/781bb4c3cc02
Part 13 - Add logging statements on flush and shutdown in FFmpegDataDecoder. r=alwu
https://hg.mozilla.org/integration/autoland/rev/d4f11dc33eed
Part 14 - Test seamless looping of compressed audio files. r=alwu
https://hg.mozilla.org/integration/autoland/rev/62941a522893
Part 15 - Rework encoder delay and padding trimming when decoding audio with ffmpeg. r=alwu
https://hg.mozilla.org/integration/autoland/rev/ee2fab5881c7
Part 16 - Rework encoder delay and padding trimming when decoding audio with AudioToolkit. r=alwu
https://hg.mozilla.org/integration/autoland/rev/1ce012d7509e
Part 17 - Test that we're trimming the encoder delay/preroll/padding in most container / codec combination correctly. r=alwu
https://hg.mozilla.org/integration/autoland/rev/5be1d4b282ab
Part 18 - When demuxing ADTS, adjust the timestamp to reflect encoder delay. r=alwu
https://hg.mozilla.org/integration/autoland/rev/3a163596f982
Part 19 - When decoding Opus, trim encoder delay based on the PTS window on each encoded packet. r=alwu
https://hg.mozilla.org/integration/autoland/rev/3b8a0081fd10
Part 20 - When demuxing an mp3 stream, adjust the PTS if encoder delay/padding information has been found in extradata. r=alwu
https://hg.mozilla.org/integration/autoland/rev/b08695e5728a
Part 21 - Add IsPositiveOrZero to TimeUnit. r=alwu
https://hg.mozilla.org/integration/autoland/rev/6b2e055d9314
Part 22 - Expose Padding and Delay in MP3Demuxer, for testing. r=alwu
https://hg.mozilla.org/integration/autoland/rev/86e7d4784a20
Part 23 - Parse ID3v1 tag properly, and deduce the tag size from the file duration, in CBR. r=alwu
https://hg.mozilla.org/integration/autoland/rev/6acede55acbb
Part 24 - Parse VBRI metadata and use it for trimming and duration calculation. r=alwu
https://hg.mozilla.org/integration/autoland/rev/d694726a3ee6
Part 25 - Subtract padding and delay in more cases when computing mp3 durations. r=alwu
https://hg.mozilla.org/integration/autoland/rev/ce754dbfb862
Part 26 - Change arithmetic when seeking mp3 to avoid rounding. r=alwu
https://hg.mozilla.org/integration/autoland/rev/73db43d49c6d
Part 27 - Adjust pts of demuxed mp3 packets to indicate regions to trim. r=alwu
https://hg.mozilla.org/integration/autoland/rev/b7fdcb494670
Part 28 - Tighten mp3 demuxing tests, removing tolerances and checking encoder delay and padding. r=alwu,geckoview-reviewers,tthibaud
https://hg.mozilla.org/integration/autoland/rev/ab44727b2e1a
Part 29 - Fix warnings in DecodedStream.cpp. r=alwu
https://hg.mozilla.org/integration/autoland/rev/cab00065d83d
Part 30 - Fix warnings in MediaData. r=alwu
https://hg.mozilla.org/integration/autoland/rev/b27b54c95cf2
Part 31 - Fix warnings in MP3Demuxer. r=alwu
https://hg.mozilla.org/integration/autoland/rev/38b0416538f3
Part 32 - Fix warnings in MP3FrameParser. r=alwu
https://hg.mozilla.org/integration/autoland/rev/bae2b2fb9759
Part 33 - Fix warnings in TestMP3Demuxer. r=alwu
https://hg.mozilla.org/integration/autoland/rev/07eb8b2c5b9f
Part 34 - Fix warnings in MediaData. r=alwu
https://hg.mozilla.org/integration/autoland/rev/a79a2f1b2b51
Part 35 - Fix warning in MediaDecoderStateMachine. r=alwu
https://hg.mozilla.org/integration/autoland/rev/8f96c8811675
Part 36 - Add a comment to say WebM timecodes are in nanoseconds. r=alwu
https://hg.mozilla.org/integration/autoland/rev/56ec4a57a6a6
Part 37 - Add TimeUnit::IsZero convenience function. r=alwu
https://hg.mozilla.org/integration/autoland/rev/96c9aacc6811
Part 38 - Correctly handle mp3 padding when it spans more than one packet. r=alwu
https://hg.mozilla.org/integration/autoland/rev/582d5c680e5a
Part 39 - Correct the amount of frames to skip in MP3 VBR files that don't have an explicit decoder delay value. r=alwu
https://hg.mozilla.org/integration/autoland/rev/e77c61e8fad2
Part 40 - Mover very-short.mp3 to Web Audio API decode test, use a slightly longer file for HTMLMediaElement tests. r=alwu
https://hg.mozilla.org/integration/autoland/rev/20d9b7cf0ddc
Part 41 - Fix warnings in DecoderData.cpp. r=alwu
https://hg.mozilla.org/integration/autoland/rev/efa6d130d4eb
Part 42 - Update MP3Demuxer to use new TimeUnit facilities, in particular sample-accurate trimming. r=alwu
https://hg.mozilla.org/integration/autoland/rev/dc5fd84a3a4c
Part 43 - Fix ADTSDemuxer to use new TimeUnit facilities, in particular sample-accurate trimming. r=alwu
https://hg.mozilla.org/integration/autoland/rev/d29bc29397c8
Part 44 - Tell the MP4Demuxer to trim decoded packets using the AudioTrimmer. r=alwu
https://hg.mozilla.org/integration/autoland/rev/7353f869efd2
Part 45 - Update MP4Demuxer to use timescale based time values of mp4parse-rust. r=kinetik
https://hg.mozilla.org/integration/autoland/rev/f1ecfbba7ed2
1817997, 1821362: apply code formatting via Lando

Comment 74

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/f5c485c41d13
https://hg.mozilla.org/mozilla-central/rev/eaff51e7754b
https://hg.mozilla.org/mozilla-central/rev/bf07b3cfd3d5
https://hg.mozilla.org/mozilla-central/rev/fb4a61ab1300
https://hg.mozilla.org/mozilla-central/rev/545253c469e6
https://hg.mozilla.org/mozilla-central/rev/2ba7ca61ecc3
https://hg.mozilla.org/mozilla-central/rev/34d56948c6b6
https://hg.mozilla.org/mozilla-central/rev/fd1dc92d3911
https://hg.mozilla.org/mozilla-central/rev/953432408a83
https://hg.mozilla.org/mozilla-central/rev/1e76eecb85e2
https://hg.mozilla.org/mozilla-central/rev/9f65c67ba2a6
https://hg.mozilla.org/mozilla-central/rev/f070e0de0566
https://hg.mozilla.org/mozilla-central/rev/781bb4c3cc02
https://hg.mozilla.org/mozilla-central/rev/d4f11dc33eed
https://hg.mozilla.org/mozilla-central/rev/62941a522893
https://hg.mozilla.org/mozilla-central/rev/ee2fab5881c7
https://hg.mozilla.org/mozilla-central/rev/1ce012d7509e
https://hg.mozilla.org/mozilla-central/rev/5be1d4b282ab
https://hg.mozilla.org/mozilla-central/rev/3a163596f982
https://hg.mozilla.org/mozilla-central/rev/3b8a0081fd10
https://hg.mozilla.org/mozilla-central/rev/b08695e5728a
https://hg.mozilla.org/mozilla-central/rev/6b2e055d9314
https://hg.mozilla.org/mozilla-central/rev/86e7d4784a20
https://hg.mozilla.org/mozilla-central/rev/6acede55acbb
https://hg.mozilla.org/mozilla-central/rev/d694726a3ee6
https://hg.mozilla.org/mozilla-central/rev/ce754dbfb862
https://hg.mozilla.org/mozilla-central/rev/73db43d49c6d
https://hg.mozilla.org/mozilla-central/rev/b7fdcb494670
https://hg.mozilla.org/mozilla-central/rev/ab44727b2e1a
https://hg.mozilla.org/mozilla-central/rev/cab00065d83d
https://hg.mozilla.org/mozilla-central/rev/b27b54c95cf2
https://hg.mozilla.org/mozilla-central/rev/38b0416538f3
https://hg.mozilla.org/mozilla-central/rev/bae2b2fb9759
https://hg.mozilla.org/mozilla-central/rev/07eb8b2c5b9f
https://hg.mozilla.org/mozilla-central/rev/a79a2f1b2b51
https://hg.mozilla.org/mozilla-central/rev/8f96c8811675
https://hg.mozilla.org/mozilla-central/rev/56ec4a57a6a6
https://hg.mozilla.org/mozilla-central/rev/96c9aacc6811
https://hg.mozilla.org/mozilla-central/rev/582d5c680e5a
https://hg.mozilla.org/mozilla-central/rev/e77c61e8fad2
https://hg.mozilla.org/mozilla-central/rev/20d9b7cf0ddc
https://hg.mozilla.org/mozilla-central/rev/efa6d130d4eb
https://hg.mozilla.org/mozilla-central/rev/dc5fd84a3a4c
https://hg.mozilla.org/mozilla-central/rev/d29bc29397c8
https://hg.mozilla.org/mozilla-central/rev/7353f869efd2
https://hg.mozilla.org/mozilla-central/rev/f1ecfbba7ed2

Comment 75

[Cosmin Sabou [:CosminS]]

Backed out 80 changesets (bug 1821362, bug 1703812, bug 1817997) for causing media crashes as in Bug 1833890.

Backout link: https://hg.mozilla.org/mozilla-central/rev/225c5ab0d999e743db5298d125893ae0702884af

Comment 76

[val.zapod.vz]

Again, this still has 1672 samples remainder (1 sample is just a bug of the test suite), see https://jakearchibald.github.io/aac-decode-bug

Also when you decode you cannot just omit 1024 samples, because you need the 1024th sample to decode 1025 and only then you can omit 1024 sample.

Comment 77

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/87468e667b25
Part 1 - Mark MP4Metadata::GetTrackIndice as const. r=bryce
https://hg.mozilla.org/integration/autoland/rev/96f1cd2ee957
Part 2 - Find the decoder delay and the total media duration when demuxing an mp4 file that contains an AAC track, and pass that to the codec implementation. r=alwu
https://hg.mozilla.org/integration/autoland/rev/25ff86c810e6
Part 3 - Trim decoder delay and padding when ffmpeg is decoding AAC. r=alwu
https://hg.mozilla.org/integration/autoland/rev/2a805df9ae93
Part 4 - Improve logging statement in error path in ffmpeg init code. r=alwu
https://hg.mozilla.org/integration/autoland/rev/6c61a004b5e2
Part 5 - Trim the decoder delay and the padding when decoding AAC using the AT decoder. r=alwu
https://hg.mozilla.org/integration/autoland/rev/ad1a9d76e090
Part 6 - Test that the decoder delay and the padding are trimmed correctly with various test files encoded with ffmpeg. r=alwu
https://hg.mozilla.org/integration/autoland/rev/00e16510da91
Part 7 - Trim the encoder delay and padding of AAC files when decoding using WMF. r=alwu
https://hg.mozilla.org/integration/autoland/rev/1cce04733973
Part 8 - Don't spam the log with AudioTrimmer log when activating PlatformDecoderModule:4. r=alwu
https://hg.mozilla.org/integration/autoland/rev/b135169784db
Part 9 - Put the test file name in the assertion message in test_decoderDelay.html r=alwu
https://hg.mozilla.org/integration/autoland/rev/756fcdb11d5c
Part 10 - Add logging in failure paths in FFmpegAudioDecoder.cpp r=alwu
https://hg.mozilla.org/integration/autoland/rev/9ae5eba34bb5
Part 11 - Fix static-analysis issues in android/RemoteDataDecoder.cpp. r=alwu
https://hg.mozilla.org/integration/autoland/rev/8d2db982650f
Part 12 - Fix build after backout: rename MP4SampleIndex. r=alwu
https://hg.mozilla.org/integration/autoland/rev/f35369a366e3
Part 13 - Add logging statements on flush and shutdown in FFmpegDataDecoder. r=alwu
https://hg.mozilla.org/integration/autoland/rev/df9909b1e436
Part 14 - Test seamless looping of compressed audio files. r=alwu
https://hg.mozilla.org/integration/autoland/rev/e3d1064540e8
Part 15 - Rework encoder delay and padding trimming when decoding audio with ffmpeg. r=alwu
https://hg.mozilla.org/integration/autoland/rev/09facdbce603
Part 16 - Rework encoder delay and padding trimming when decoding audio with AudioToolkit. r=alwu
https://hg.mozilla.org/integration/autoland/rev/b3a837c0ad1a
Part 17 - Test that we're trimming the encoder delay/preroll/padding in most container / codec combination correctly. r=alwu
https://hg.mozilla.org/integration/autoland/rev/a72b64ab9046
Part 18 - When demuxing ADTS, adjust the timestamp to reflect encoder delay. r=alwu
https://hg.mozilla.org/integration/autoland/rev/00ede7ff2294
Part 19 - When decoding Opus, trim encoder delay based on the PTS window on each encoded packet. r=alwu
https://hg.mozilla.org/integration/autoland/rev/3668a5dfbc71
Part 20 - When demuxing an mp3 stream, adjust the PTS if encoder delay/padding information has been found in extradata. r=alwu
https://hg.mozilla.org/integration/autoland/rev/40a22af10cc4
Part 21 - Add IsPositiveOrZero to TimeUnit. r=alwu
https://hg.mozilla.org/integration/autoland/rev/04699c4897ec
Part 22 - Expose Padding and Delay in MP3Demuxer, for testing. r=alwu
https://hg.mozilla.org/integration/autoland/rev/07a751a8e345
Part 23 - Parse ID3v1 tag properly, and deduce the tag size from the file duration, in CBR. r=alwu
https://hg.mozilla.org/integration/autoland/rev/ce63ef867f78
Part 24 - Parse VBRI metadata and use it for trimming and duration calculation. r=alwu
https://hg.mozilla.org/integration/autoland/rev/9d4dee6c3752
Part 25 - Subtract padding and delay in more cases when computing mp3 durations. r=alwu
https://hg.mozilla.org/integration/autoland/rev/78979b2eec03
Part 26 - Change arithmetic when seeking mp3 to avoid rounding. r=alwu
https://hg.mozilla.org/integration/autoland/rev/1b233d7ca8d5
Part 27 - Adjust pts of demuxed mp3 packets to indicate regions to trim. r=alwu
https://hg.mozilla.org/integration/autoland/rev/15c2140feeb7
Part 28 - Tighten mp3 demuxing tests, removing tolerances and checking encoder delay and padding. r=alwu,geckoview-reviewers,tthibaud
https://hg.mozilla.org/integration/autoland/rev/45ee192631d1
Part 29 - Fix warnings in DecodedStream.cpp. r=alwu
https://hg.mozilla.org/integration/autoland/rev/a09d9a0cc0ef
Part 30 - Fix warnings in MediaData. r=alwu
https://hg.mozilla.org/integration/autoland/rev/f5a73d278d53
Part 31 - Fix warnings in MP3Demuxer. r=alwu
https://hg.mozilla.org/integration/autoland/rev/f86887a74eb0
Part 32 - Fix warnings in MP3FrameParser. r=alwu
https://hg.mozilla.org/integration/autoland/rev/13b631eaa174
Part 33 - Fix warnings in TestMP3Demuxer. r=alwu
https://hg.mozilla.org/integration/autoland/rev/f3554351f68f
Part 34 - Fix warnings in MediaData. r=alwu
https://hg.mozilla.org/integration/autoland/rev/f9653cc7219c
Part 35 - Fix warning in MediaDecoderStateMachine. r=alwu
https://hg.mozilla.org/integration/autoland/rev/4218d0473b82
Part 36 - Add a comment to say WebM timecodes are in nanoseconds. r=alwu
https://hg.mozilla.org/integration/autoland/rev/33ef8b37d8c4
Part 37 - Add TimeUnit::IsZero convenience function. r=alwu
https://hg.mozilla.org/integration/autoland/rev/e733eaec3f66
Part 38 - Correctly handle mp3 padding when it spans more than one packet. r=alwu
https://hg.mozilla.org/integration/autoland/rev/f540a604383d
Part 39 - Correct the amount of frames to skip in MP3 VBR files that don't have an explicit decoder delay value. r=alwu
https://hg.mozilla.org/integration/autoland/rev/172ef2db7e2f
Part 40 - Mover very-short.mp3 to Web Audio API decode test, use a slightly longer file for HTMLMediaElement tests. r=alwu
https://hg.mozilla.org/integration/autoland/rev/2b87e0fb0dac
Part 41 - Fix warnings in DecoderData.cpp. r=alwu
https://hg.mozilla.org/integration/autoland/rev/79db40f1f1fd
Part 42 - Update MP3Demuxer to use new TimeUnit facilities, in particular sample-accurate trimming. r=alwu
https://hg.mozilla.org/integration/autoland/rev/a9ee1da522d1
Part 43 - Fix ADTSDemuxer to use new TimeUnit facilities, in particular sample-accurate trimming. r=alwu
https://hg.mozilla.org/integration/autoland/rev/4fbff35c7a0e
Part 44 - Tell the MP4Demuxer to trim decoded packets using the AudioTrimmer. r=alwu
https://hg.mozilla.org/integration/autoland/rev/f3e79a320d70
Part 45 - Update MP4Demuxer to use timescale based time values of mp4parse-rust. r=kinetik
https://hg.mozilla.org/integration/autoland/rev/928afb6ad2a5
1817997, 1821362: apply code formatting via Lando

Comment 78

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/87468e667b25
https://hg.mozilla.org/mozilla-central/rev/96f1cd2ee957
https://hg.mozilla.org/mozilla-central/rev/25ff86c810e6
https://hg.mozilla.org/mozilla-central/rev/2a805df9ae93
https://hg.mozilla.org/mozilla-central/rev/6c61a004b5e2
https://hg.mozilla.org/mozilla-central/rev/ad1a9d76e090
https://hg.mozilla.org/mozilla-central/rev/00e16510da91
https://hg.mozilla.org/mozilla-central/rev/1cce04733973
https://hg.mozilla.org/mozilla-central/rev/b135169784db
https://hg.mozilla.org/mozilla-central/rev/756fcdb11d5c
https://hg.mozilla.org/mozilla-central/rev/9ae5eba34bb5
https://hg.mozilla.org/mozilla-central/rev/8d2db982650f
https://hg.mozilla.org/mozilla-central/rev/f35369a366e3
https://hg.mozilla.org/mozilla-central/rev/df9909b1e436
https://hg.mozilla.org/mozilla-central/rev/e3d1064540e8
https://hg.mozilla.org/mozilla-central/rev/09facdbce603
https://hg.mozilla.org/mozilla-central/rev/b3a837c0ad1a
https://hg.mozilla.org/mozilla-central/rev/a72b64ab9046
https://hg.mozilla.org/mozilla-central/rev/00ede7ff2294
https://hg.mozilla.org/mozilla-central/rev/3668a5dfbc71
https://hg.mozilla.org/mozilla-central/rev/40a22af10cc4
https://hg.mozilla.org/mozilla-central/rev/04699c4897ec
https://hg.mozilla.org/mozilla-central/rev/07a751a8e345
https://hg.mozilla.org/mozilla-central/rev/ce63ef867f78
https://hg.mozilla.org/mozilla-central/rev/9d4dee6c3752
https://hg.mozilla.org/mozilla-central/rev/78979b2eec03
https://hg.mozilla.org/mozilla-central/rev/1b233d7ca8d5
https://hg.mozilla.org/mozilla-central/rev/15c2140feeb7
https://hg.mozilla.org/mozilla-central/rev/45ee192631d1
https://hg.mozilla.org/mozilla-central/rev/a09d9a0cc0ef
https://hg.mozilla.org/mozilla-central/rev/f5a73d278d53
https://hg.mozilla.org/mozilla-central/rev/f86887a74eb0
https://hg.mozilla.org/mozilla-central/rev/13b631eaa174
https://hg.mozilla.org/mozilla-central/rev/f3554351f68f
https://hg.mozilla.org/mozilla-central/rev/f9653cc7219c
https://hg.mozilla.org/mozilla-central/rev/4218d0473b82
https://hg.mozilla.org/mozilla-central/rev/33ef8b37d8c4
https://hg.mozilla.org/mozilla-central/rev/e733eaec3f66
https://hg.mozilla.org/mozilla-central/rev/f540a604383d
https://hg.mozilla.org/mozilla-central/rev/172ef2db7e2f
https://hg.mozilla.org/mozilla-central/rev/2b87e0fb0dac
https://hg.mozilla.org/mozilla-central/rev/79db40f1f1fd
https://hg.mozilla.org/mozilla-central/rev/a9ee1da522d1
https://hg.mozilla.org/mozilla-central/rev/4fbff35c7a0e
https://hg.mozilla.org/mozilla-central/rev/f3e79a320d70
https://hg.mozilla.org/mozilla-central/rev/928afb6ad2a5

Comment 79

[Monica Chiorean]

I was able to reproduce the issue on Win10x64 using FF build 89.0a1(20210408215439) (had different values on decoder page, mentioned at description).
Verified as fixed on Win10/Ubuntu 20.04/Mac 12.6 using FF builds 115.0b9(20230622161221) and 116.0a1(20230625195949) (had the same value on decoder page).