Closed Bug 1482706 Opened 7 years ago Closed 6 years ago

HE-AAC/AAC-SBR Seek Broken

Categories

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

63 Branch
Unspecified
macOS
enhancement

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox63 --- wontfix
firefox68 --- fixed

People

(Reporter: jamie.woods, Assigned: jya)

References

()

Details

(Keywords: testcase)

Attachments

(1 file)

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.
Has STR: --- → yes
Component: Untriaged → Audio/Video: Playback
Keywords: testcase
Product: Firefox → Core
I think I can hear the same glitch in Safari 11.1.2, Chrome seems fine though.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Severity: normal → enhancement
I guess we could make the aac demuxer seeks slightly earlier.. we do that already for the mp4 and mse demuxer...
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)
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
Flags: needinfo?(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.
Flags: needinfo?(jamie.woods)
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.
Flags: needinfo?(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.
Flags: needinfo?(jamie.woods)
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 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.
Attachment #9001549 - Flags: review+
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.
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

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

Flags: needinfo?(jyavenard)

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.

Flags: needinfo?(jyavenard)
Assignee: nobody → jyavenard

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?

Flags: needinfo?(jyavenard)

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.

Flags: needinfo?(jyavenard)

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

Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX

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

Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Pushed by jyavenard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6f9bdd2e9187 Pre-roll seek by 2112 frames to account for encoder delay. r=kinetik
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Regressions: 1561492
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: