1694085 - When RDD enabled, sound quality of HE-AAC that comes from MediaSoruce will be degraded.

Status: VERIFIED FIXED

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

Description

[rinsuki]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.16; rv:86.0) Gecko/20100101 Firefox/86.0

Steps to reproduce:

1. Enable RDD (by set true to media.rdd-process.enabled, currently default is true).
2. Listen HE-AAC sound that comes from MediaSource.
   2a. If you live in Japan, you can test with real world service https://radiko.jp/
   2b. generate HLS stream with these command and play it with HLS.js

ffmpeg -i source.m4a -codec aac_at -profile:a 28 -vn -f segment -segment_time 5 -segment_list playlist.m3u8 seg%04d.aac

(If you haven't macOS, you should built ffmpeg with libfdk_aac support and replace aac_at to libfdk_aac)

Note: If play HE-AAC sound without MediaSource (e.g. <audio src="he-aac.aac">), it works fine whatever RDD enabled.

Actual results:

Sound quality is degraded when compared to Chrome, Safari, and Firefox (that RDD disabled).

(maybe HE-AAC's SBR part is completely ignored)

Expected results:

Sound quality does not change whether RDD is enabled or disabled

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Core::Audio/Video: Playback' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Ken Berland]

I can confirm that our Firefox users are experiencing this issue. The output frequencies are capped under 10kHz.
See:

Is there any plan to fix this?

Comment 3

[Ken Berland]

Markdown images didn't seem to work. Here are two images https://kenpublic.s3.amazonaws.com/2021-10-13/608q5JfRZ84go4ER/aac-he-dash.m4a.png and https://kenpublic.s3.amazonaws.com/2021-10-13/KCWUEn4FkkOPTkEb/aac-he-dash-media.rdd-process.false.m4a.png

Comment 4

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Thanks for the further info. There is a plan to fix this, but we also have limited developer time. If you have a page that this reproduces on, it would be useful to have another test case for diagnostics and to ensure we have fixed the issue once we're making code changes.

Comment 5

[Ken Berland]

https://maestro-test-player.s3.us-west-1.amazonaws.com/fx_P50628906/index.html

Comment 6

[Alastor Wu [:alwu]]

Hi, Ken, would you mind to dump the audio by using following method? The instructions differ per OS:

• macOS
  Open a terminal window (command+space, type terminal, enter)
  Copy and paste the following:
  cd ~/Desktop; MOZ_DUMP_AUDIO=1 MOZ_DISABLE_CONTENT_SANDBOX=1 MOZ_LOG=cubeb:5 MOZ_LOG_FILES=firefox-logs /Applications/Firefox.app/Contents/MacOS/firefox
• Linux
  Open a terminal window
  cd ~/Desktop; MOZ_DUMP_AUDIO=1 MOZ_DISABLE_CONTENT_SANDBOX=1 MOZ_LOG=cubeb:5 MOZ_LOG_FILES=firefox-logs /usr/bin/firefox
• Windows
  Open a cmd.exe prompt (Windows+r, cmd.exe, enter)
  Navigate to the desktop:
  cd %userprofile%\desktop
  Copy and paste the following (right click on the title bar to paste) in one line, press enter
  SET MOZ_DUMP_AUDIO=1 & MOZ_DISABLE_CONTENT_SANDBOX=1 & MOZ_LOG=cubeb:5 & MOZ_LOG_FILES=firefox-logs
  Run Firefox, it can be in multiple locations
  C:\Program Files\Mozilla Firefox\firefox.exe (64bit Firefox release on 64bit windows or 32bit Firefox on 32bit Windows)
  C:\Program Files\Firefox Nightly\firefox.exe (64bit Firefox Nightly on 64bit Windows or 32bit Firefox Nightly on 32bit Windows)
  C:\Program Files (x86)\Mozilla Firefox\firefox.exe (32bit Firefox Nightly on 64bit Windows)

Then gather all the files named firefox-logs.* and AudioStream* GraphDriverInput* GraphDriverOutput* that are on the desktop, the * denoting a numerical suffix in a zip, and attach them to the bug.
Thank you so much.

Comment 7

[Paul Adenot (:padenot)]

Attachment: he-aac.zip

Here is a test-case which is roughly what the reporter said. Unzip, serve with http-server, python3 -m http.server, etc. Open the page in Firefox, press the play button.

This plays a file which is HE-AAC via hls.js, that is just white noise (maximum energy on all frequency from 0Hz to 20+kHz) and draws the audio spectrum using the Web Audio API and a canvas. In Firefox there is clearly a very sharp roll-off at 10kHz, not present in other browser. What is expected is the spectrum to be full up to 20+kHz.

I'm thinking maybe we're not shepherding the extradata correctly to the decoder in the media-source case?

Comment 8

[Paul Adenot (:padenot)]

This works correctly on Linux desktop and Windows 11 (I've only tested 11 but I assume it works ok on other versions). This appears to be macOS only so I'll have a look at that.

Comment 9

[Paul Adenot (:padenot)]

Attachment: Bug 1694085 - Serialize and deserialize the codec extra-data when sending an AudioInfo to the RDD. r?bryce

Comment 10

[Nicholas Bilyk]

I've updated the test bed Ken shared earlier to be more pronounced for those of us with poor hearing. The testbed now uses white noise, a 440Hz tone and a 10kHz tone. On the affected combindations the 10kHz tone is inaudible.
https://maestro-test-player.s3.us-west-1.amazonaws.com/fx_P50628906/index.html

Comment 11

[Nicholas Bilyk]

And here's a link to the audio file that reproduces the issue. Feel free to use it however you'd like:
https://maestro-test-player.s3.us-west-1.amazonaws.com/fx_P50628906/aac-he.mp4

Comment 12

[Paul Adenot (:padenot)]

Attachment: Bug 1694085 - Add a test that verifies that HE-AAC plays back correctly including higher frequency content both with MSE and HTTP playback. r?bryce

Depends on D128895

Comment 13

[Paul Adenot (:padenot)]

Nicholas, thanks for the test-cases, I confirm that they now work as they should with the patch attached here, that I'll land soon and uplift to beta, so it reaches users as quickly as possible.

I ended up creating my own test files with sox, ffmpeg and mp4box and this won't regress with the new test, but it's always good to double check :-).

Comment 14

[Nicholas Bilyk]

Beautiful, thank you Paul

Comment 15

[Ken Berland]

Thank you, Paul!

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f80df57030d8
Serialize and deserialize the codec extra-data when sending an AudioInfo to the RDD. r=bryce
https://hg.mozilla.org/integration/autoland/rev/dd1ec2d464cb
Add a test that verifies that HE-AAC plays back correctly including higher frequency content both with MSE and HTTP playback. r=bryce

Comment 17

[Paul Adenot (:padenot)]

Attachment: Bug 1694085 - Smooth the FFT analysis to make the test more robust.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1468166a95c6
Smooth the FFT analysis to make the test more robust.

Comment 19

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ddb11a86bf43
Backed out 3 changesets for causing mda failures on test_HEAAC_extradata.html CLOSED TREE

Comment 20

[Norisz Fay [:noriszfay]]

Backed out for causing mda failures on test_HEAAC_extradata.html

Backout link: https://hg.mozilla.org/integration/autoland/rev/ddb11a86bf436e406bea5d2494cd7a5479effd8e
Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=YfzhZPd8TRyAj5jKjn0lRg.0&author=padenot%40mozilla.com&fromchange=dd1ec2d464cb5e7b766a8b9ae3e96caf507c654b&tochange=1468166a95c6d2dbfe64b78ece2a63e20263201c&searchStr=mda
Failure log: https://treeherder.mozilla.org/logviewer?job_id=355562926&repo=autoland

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/55d5a96152d1
Serialize and deserialize the codec extra-data when sending an AudioInfo to the RDD. r=bryce
https://hg.mozilla.org/integration/autoland/rev/52cf81c3621a
Add a test that verifies that HE-AAC plays back correctly including higher frequency content both with MSE and HTTP playback. r=bryce

Comment 22

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/55d5a96152d1
https://hg.mozilla.org/mozilla-central/rev/52cf81c3621a

Comment 23

[Paul Adenot (:padenot)]

Comment on attachment 9246659 [details]
Bug 1694085 - Serialize and deserialize the codec extra-data when sending an AudioInfo to the RDD. r?bryce

Beta/Release Uplift Approval Request

• User impact if declined: Audio quality greatly reduced on major music and video stream services on macOS.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): No risk, this is a two liner that simply adds a field that was forgotten when sending a struct via IPC. This is now well tested.
• String changes made/needed: none

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9246659 [details]
Bug 1694085 - Serialize and deserialize the codec extra-data when sending an AudioInfo to the RDD. r?bryce

Bit late in the cycle for this given that we're out of betas going into RC week, but the patch seems pretty low risk relative to the user impact. Thanks for including tests. Approved for 94.0rc1.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7c7b7a06f66e
https://hg.mozilla.org/releases/mozilla-beta/rev/0c0941631174

Comment 26

[Hani Yacoub, Desktop QA]

Verified as fixed on macOS 11.6 on Firefox Nightly 95.0a1 and on Firefox 94.0 candidates build 2.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

Should we take this on ESR also?

Comment 28

[Paul Adenot (:padenot)]

Yes, it's really simple and impacts lots of major websites.

Comment 29

[Paul Adenot (:padenot)]

Wait a minute I can't repro on my macbook on current ESR, I'm trying to fing why.

Comment 30

[Keith Gable]

On https://maestro-test-player.s3.us-west-1.amazonaws.com/fx_P50628906/index.html, I can repro this on Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Firefox/91.0 (About says "91.5.0esr (64-bit)"). If I go to about:config and set media.rdd-process.enabled to false, it works as expected.

Comment 31

[Ryan VanderMeulen [:RyanVM]]

Any updates on this, Paul?

Comment 32

[Paul Adenot (:padenot)]

Not sure what's up then. I probably screwed up my testing or something. This should uplift cleanly in any case, let me know if it does not.

Comment 33

[Paul Adenot (:padenot)]

Comment on attachment 9246659 [details]
Bug 1694085 - Serialize and deserialize the codec extra-data when sending an AudioInfo to the RDD. r?bryce

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This fixes an audio quality issue on major websites (essentially, audio or video stream sites).
• User impact if declined: Significantly lower audio quality on macOS when playing AAC (quite common).
• Fix Landed on Version: 95
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Been in release for some time, was uplifted to beta at the time -- no risk, verified by a couple major websites developers as well. There is also a good test.

Comment 34

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9246659 [details]
Bug 1694085 - Serialize and deserialize the codec extra-data when sending an AudioInfo to the RDD. r?bryce

Approved for 91.7esr.

Comment 35

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/6ccfbda2e8a9
https://hg.mozilla.org/releases/mozilla-esr91/rev/385be82d164a

Comment 36

[Hani Yacoub, Desktop QA]

Verified as fixed on Windows 10 x64, macOS 11.6 and on Ubuntu 20.04 x64 on Firefox 91.7.0esr.