1482706 - HE-AAC/AAC-SBR Seek Broken

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, enhancement, P3)

Description

[Jamie Woods]

On Mac, when seeking within a high-efficiency AAC audio file (i.e. one that use SBR to achieve such a bitrate), the first frame(s) are not decoded with SBR.

As a result, there is a very perceptible "blip" in audio quality for about 0.1 seconds. 

Steps to reproduce (with a reasonable pair of headphones):

1. Open a HE-AAC audio file (sample: https://cdn.index.hm/f/rHkX0jIzMjFlMWM4ZjMyY/10.wav.aac)
2. Seek within this file
3. Note the 0.1 second blip in audio quality where the decoder does not decode SBR, resulting in effectively no treble. 

Neither Safari or Chromium have this behaviour on Mac. I have not tested other platforms. This may be related to bug 1387127, which oversaw the initial implementation of HE-AAC playback on Mac.

Comment 1

[Matthew Gregan [:kinetik]]

I think I can hear the same glitch in Safari 11.1.2, Chrome seems fine though.

Comment 2

[Jean-Yves Avenard [:jya]]

I guess we could make the aac demuxer seeks slightly earlier.. we do that already for the mp4 and mse demuxer...

Comment 3

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1482706 - Pre-roll seek by 2112 frames to account for encoder delay. r?kinetik

Comment 4

[Jean-Yves Avenard [:jya]]

Officially, the CoreAudio AAC decoder doesn't support implicit signaling of SBR in a AAC stream... What we do has always been a bit hackish...

Seeking 2112 frames prior may not be enough, if the glitch is heard for 0.1s (it's a bit hard to tell for me with this particular sample)

Comment 5

[Jean-Yves Avenard [:jya]]

Could you please test with this version and let us know if it fixes the problem for you..

thank you
https://queue.taskcluster.net/v1/task/TrBXXM06Sqed6xpsjQKcew/runs/0/artifacts/public/build/target.dmg

Comment 6

[Jamie Woods]

The attached build does not seem to solve the issue for me, I can still hear the decoding artefact. It sounds about the same as previous.

Comment 7

[Jean-Yves Avenard [:jya]]

What about this one?
https://queue.taskcluster.net/v1/task/YsifeQnmSe-8OeWyJ_2_RQ/runs/0/artifacts/public/build/target.dmg

It's bizarre that you're still hearing the glitch with the earlier version. The scanning code for inband SBR does find one, and the audio decoder is initialized with it.

Comment 8

[Jamie Woods]

I can hear the glitch when I seek backwards or repeatedly to the same point (i.e. click the same time in the seek bar - it's particularly noticable at ~00:23.5). If I skip forward, about half of the time I do not hear the glitch.

However, it's an improvement.

Comment 9

[Jean-Yves Avenard [:jya]]

How is that file actually encoded?

How spread is the SBR stuff ? Basically I need to know how far back we need to seek for the AAC decoder to be able to fully decode it.

Comment 10

[Matthew Gregan [:kinetik]]

Comment on attachment 9001549 [details]
Bug 1482706 - Pre-roll seek by 2112 frames to account for encoder delay. r?kinetik

Matthew Gregan [:kinetik] has approved the revision.

Comment 11

[Matthew Gregan [:kinetik]]

Copying my comment in Phabricator to here (I should've made it here instead):

Assuming 2112 seems harmless enough. Fishing the correct value out of the MP4 would be more correct, but it's a bit of a mess with three different places to look.

2112 is derived from 2x 1024 frame packets and a hard 64 frame encoder delay. Looking at 10.wav.aac, the HE/HEv2 parts of the tracks have 2048 frame packets - so this file might need
to skip 4160 frames? I'm not sure how HE mode would affect encoder delay, so that's a complete guess!

Curiously, afinfo doesn't list encoder delay for this file. Usually you get something like:

  audio 266176 valid frames + 2112 priming + 0 remainder = 268288

(from gizmo.mp4), but afinfo 10.wav.aac doesn't print that line at all.

Comment 12

[Jamie Woods]

The file is encoded using ffmpeg 3.3.7 compiled with fdk-aac.

ffmpeg is built using the trunk version of fdk-aac (https://github.com/mstorsjo/fdk-aac at  a50eecf65b5ce5d4f03768c5c2cb4b492d2badad). There's a Docker build script for this available at https://raw.githubusercontent.com/InsanityRadio/Nerve/master/docker/Dockerfile.worker which is exactly how this ffmpeg is built. 

Here is an example command used to generate the file. There are no special options, it's a very standard way of generating HE-AAC-v2. The sample rate is intentionally limited. 

> ffmpeg -i 10.wav -ar 44100 -c:a libfdk_aac -profile:a aac_he_v2 -b:a 48k 10.wav.aac

Comment 13

[Matthew Gregan [:kinetik]]

Just noticed the r+ed patch never landed. Do we want to take this fix or do something else, Jean-Yves?

Comment 14

[Jean-Yves Avenard [:jya]]

It never really worked. 2112 worked okay for a particular position but not in another.

I don't think there's much we can do here as we have no control of the AAC decoder and how it behaves, plus it's different per platform.

Comment 15

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

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:jya, could you have a look please?

Comment 16

[Jean-Yves Avenard [:jya]]

The fix attached is a workaround that isn't universal and doesn't actually work.

Short of rewriting and shipping our own AAC decoder, I don't know how we could fix it.

Comment 17

[Jean-Yves Avenard [:jya]]

We have no ability to fix this one as it is platform dependent and we can't ship our own AAC decoder.

Comment 18

[Jean-Yves Avenard [:jya]]

I'll push the current fix, because it does make things better.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6f9bdd2e9187
Pre-roll seek by 2112 frames to account for encoder delay. r=kinetik

Comment 20

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/6f9bdd2e9187