1759325 - Some audio files have clicks/pops during the last few seconds of playback

Status: RESOLVED FIXED

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

Description

[h.nuchi]

Attachment: Test page to listen to the problematic audio

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

Steps to reproduce:

I visited the following website: https://midi-ddsp.github.io/#note_expression_control

And clicked "play" on the audio tag marked "Original Sample". (Alternatively, play any other sufficiently loud audio on that page. It's easier to notice with a short audio snippet.)

Alternatively, open the attached file test.html and play the audio tag.

More possibly-relevant details to show up in searches: the audio file in question is a wav file with 1 channel, Linear PCM, 16 bit little-endian signed integer, 16000 Hz.

Actual results:

During the last 1 or 2 seconds of the audio, I can hear clicks overlaid on the audio.

The clicks are still there if I change the playback speed to 0.5x or 2x, and they sound slowed down/sped up respectively.

The clicks are not still there if I set playback to loop. Nor are they there if I (via the console) create an AudioContext, and connect a MediaElementAudioSourceNode with said audio tag to the destination.

Expected results:

I should not hear clicks. (I do not hear clicks when I play the file in Chrome, or Safari, or if I download the audio file and play it in QuickTime Player.)

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 correct in case you think the bot is wrong.

Comment 2

[Jim Mathies [:jimm]]

Matt, curious, can you reproduce this? I don't have a mac for testing.

Comment 3

[h.nuchi]

By the way I just noticed that Bugzilla listed my user agent as "Macintosh; Intel Mac OS X 10.15".

I'm actually on an M1, macos 12.2.1, FF beta channel 99.0b2.

Comment 4

[Matthew Gregan [:kinetik]]

Thanks for the bug report! I can reproduce this on macOS and Windows in a current Nightly. This seems to be an issue with draining the last few seconds of audio streams when the source media uses lower sample rates. Concatenating the file moves the clicking to the last few seconds, and looping the media causes the issue to disappear.

Comment 5

[Matthew Gregan [:kinetik]]

mozregression narrowed this behaviour change down to bug 1752345. Paul, would you mind taking a look please?

Comment 6

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

Set release status flags based on info from the regressing bug 1752345

Comment 7

[Paul Adenot (:padenot)]

Attachment: Bug 1759325 - Use PushProcessedAudio in DrainConverter instead of having almost the same code written twice. r?alwu

Comment 8

[Paul Adenot (:padenot)]

Attachment: Bug 1759325 - Only drain the converter at the end of the queue. r?alwu

Before this change, the converter would be drained after each packet would be
pushed to the queue, introducing discontinuities in the audio, at the end, after
all the decoding operations were finished, but before all the audio packets were
pushed to the queue.

This went unnoticed because there is no converter most of the time, with audio
that has a sample-rate higher or equal to the common 44100Hz value.

Depends on D141356

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/104a5f7de3a6
Use PushProcessedAudio in DrainConverter instead of having almost the same code written twice. r=alwu
https://hg.mozilla.org/integration/autoland/rev/3451746bcd2a
Only drain the converter at the end of the queue. r=alwu

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/104a5f7de3a6
https://hg.mozilla.org/mozilla-central/rev/3451746bcd2a

Comment 11

[Paul Adenot (:padenot)]

Attachment: Bug 1759325 - Don't start with an AudioSink if muted from the start. r?alwu

Depends on D136237

Comment 12

[Phabricator Automation]

Comment on attachment 9269275 [details]
Bug 1759325 - Don't start with an AudioSink if muted from the start. r?alwu

Revision D141990 was moved to bug 1743834. Setting attachment 9269275 [details] to obsolete.