893307 - Audio playback is extremely distorted

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[Leman Bennett [Omega]]

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:25.0) Gecko/20130712 Firefox/25.0 (Nightly/Aurora)
Build ID: 20130712030203

Steps to reproduce:

Using samples at: http://people.xiph.org/~xiphmont/demo/opus/demo3.shtml

The audio playback is extremely distorted and clipped. There is also a thumping audio pattern that plays regardless if the audio is muted or not. Using SafeMode, there's no audio produced on the default device.

This does not happen with Fx22. Other browsers such as Chrome and Internet Explorer are also not affected.

Comment 1

[Leman Bennett [Omega]]

Attachment: Captured playback from Sita sample

This is directly captured audio from my machine using Audacity while playing the Sita sample from the demo page.

Comment 2

[Emanuel Hoogeveen [:ehoogeveen]]

This also happens on my system (using Windows 7 x64).

Comment 3

[u428464]

With a clean profile in Nightly and UX every HTML5 videos have distorted sound. Can't reproduce on Firefox 22 either.

Comment 4

[Trevor Rowbotham [:rowbot]]

Results of mozregression:

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20130718 Firefox/25.0
Last good nightly: 2013-07-10
First bad nightly: 2013-07-11

Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=04d8c309fe72&tochange=dde4dcd6fa46

Suspected: Bug 866675 - Add a WASAPI backend to cubeb. r=kinetik

What's strange about this is that I can reproduce this with Firefox on my desktop running Win7 x64, however, I cannot reproduce this on a virtual machine running Win7 x32. I do not experience this problem with Chrome/Opera/IE.  I suppose that the new WASAPI backend and my sound card drivers are conflicting with each other or maybe differences between 32 and 64 bit Windows are causing the problem. FWIW I am using a Creative Sound Blaster X-Fi XtremeGamer Fatal1ty Pro sound card with driver version 6.0.1.1376 (2.18.0018 Beta) dated 2012/12/07.

Let me know if I can provide you with anymore information.

Comment 5

[Paul Adenot (:padenot)]

I can't set use a 5.1 setup here, so I need the help of people to guess what is happening and write the patch.

Could someone that can repro the problem grab this build [1], unzip it, close every other instance of Firefox, double click on the firefox.exe file that is in the unzipped folder, wait a bit for this debug build of Firefox to run (it will be rather slow compared to a normal Firefox), try to play an HTML5 video/audio and tell me if it crashes? It should assert with something like "stm->mix_params.channels == 2" (in other word, crash).

[1]: http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32-debug/1374194178/firefox-25.0a1.en-US.win32.zip

Comment 6

[Emanuel Hoogeveen [:ehoogeveen]]

The page linked in this bug crashes that debug build upon loading. However, loading http://xiph.org/~giles/2012/opus/ and playing one of the tracks, I indeed get:

"Assertion failed: stm->mix_params.channels == 2, file e:/builds/moz2_slave/m-cen-w32-d-000000000000000000/build/media/libcubeb/src/cubeb_wasapi.cpp, line 617"

In addition, playing the track when the default audio device is set to a virtual audio cable (with 2 channels) and listening to it on my speakers sounds just fine.

Comment 8

[Paul Adenot (:padenot)]

Thanks, that's very helpful. I've tried to write a quick patch, could someone that has the problem try a build from [1] when they appear?

[1]: https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-faeae4c58e55/

Comment 9

[Emanuel Hoogeveen [:ehoogeveen]]

First builds failed due to infrastructure failure. I retriggered them, but it looks like compilation is busted (errors in media/libspeex_resampler/src/resample_sse.h).

Comment 10

[Chris Pearce [:cpearce (Not reading bugmail)]]

We have a 5.1 setup in the Auckland office... On the Opus test page with 5.1 audio setup I get a crash (in a locally built mozilla-central, today's tip):

mozalloc.dll!mozalloc_abort(const char * const msg) Line 30	C++
xul.dll!MSCRTReportHook(int aReportType, char * aMessage, int * oReturnValue) Line 1387	C++
msvcr110d.dll!_VCrtDbgReportW(int nRptType, void * returnAddress, const wchar_t * szFile, int nLine, const wchar_t * szModule, const wchar_t * szFormat, char * arglist) Line 609	C
msvcr110d.dll!_CrtDbgReportWV(int nRptType, void * returnAddress, const wchar_t * szFile, int nLine, const wchar_t * szModule, const wchar_t * szFormat, char * arglist) Line 262	C++
msvcr110d.dll!_CrtDbgReportW(int nRptType, const wchar_t * szFile, int nLine, const wchar_t * szModule, const wchar_t * szFormat, ...) Line 279	C++
msvcr110d.dll!_NMSG_WRITE(int rterrnum) Line 224	C
msvcr110d.dll!abort() Line 62	C
msvcr110d.dll!_wassert(const wchar_t * expr, const wchar_t * filename, unsigned int lineno) Line 155	C
gkmedias.dll!`anonymous namespace'::wasapi_stream_init(cubeb * context, cubeb_stream * * stream, const char * stream_name, cubeb_stream_params stream_params, unsigned int latency, long (cubeb_stream *, void *, void *, long) * data_callback, void (cubeb_stream *, void *, cubeb_state) * state_callback, void * user_ptr) Line 617	C++
gkmedias.dll!cubeb_stream_init(cubeb * context, cubeb_stream * * stream, const char * stream_name, cubeb_stream_params stream_params, unsigned int latency, long (cubeb_stream *, void *, void *, long) * data_callback, void (cubeb_stream *, void *, cubeb_state) * state_callback, void * user_ptr) Line 185	C
xul.dll!mozilla::BufferedAudioStream::Init(int aNumChannels, int aRate, const mozilla::dom::AudioChannelType aAudioChannelType) Line 564	C++
xul.dll!mozilla::MediaDecoderStateMachine::AudioLoop() Line 1047	C++
xul.dll!nsRunnableMethodImpl<void (__thiscall mozilla::MediaDecoderStateMachine::*)(void),1>::Run() Line 351	C++

Comment 11

[(no longer active)]

(In reply to comment #10)
> We have a 5.1 setup in the Auckland office... On the Opus test page with 5.1
> audio setup I get a crash (in a locally built mozilla-central, today's tip):
> 
> mozalloc.dll!mozalloc_abort(const char * const msg) Line 30    C++
> xul.dll!MSCRTReportHook(int aReportType, char * aMessage, int * oReturnValue)
> Line 1387    C++
> msvcr110d.dll!_VCrtDbgReportW(int nRptType, void * returnAddress, const wchar_t
> * szFile, int nLine, const wchar_t * szModule, const wchar_t * szFormat, char *
> arglist) Line 609    C
> msvcr110d.dll!_CrtDbgReportWV(int nRptType, void * returnAddress, const wchar_t
> * szFile, int nLine, const wchar_t * szModule, const wchar_t * szFormat, char *
> arglist) Line 262    C++
> msvcr110d.dll!_CrtDbgReportW(int nRptType, const wchar_t * szFile, int nLine,
> const wchar_t * szModule, const wchar_t * szFormat, ...) Line 279    C++
> msvcr110d.dll!_NMSG_WRITE(int rterrnum) Line 224    C
> msvcr110d.dll!abort() Line 62    C
> msvcr110d.dll!_wassert(const wchar_t * expr, const wchar_t * filename, unsigned
> int lineno) Line 155    C
> gkmedias.dll!`anonymous namespace'::wasapi_stream_init(cubeb * context,
> cubeb_stream * * stream, const char * stream_name, cubeb_stream_params
> stream_params, unsigned int latency, long (cubeb_stream *, void *, void *,
> long) * data_callback, void (cubeb_stream *, void *, cubeb_state) *
> state_callback, void * user_ptr) Line 617    C++
> gkmedias.dll!cubeb_stream_init(cubeb * context, cubeb_stream * * stream, const
> char * stream_name, cubeb_stream_params stream_params, unsigned int latency,
> long (cubeb_stream *, void *, void *, long) * data_callback, void (cubeb_stream
> *, void *, cubeb_state) * state_callback, void * user_ptr) Line 185    C
> xul.dll!mozilla::BufferedAudioStream::Init(int aNumChannels, int aRate, const
> mozilla::dom::AudioChannelType aAudioChannelType) Line 564    C++
> xul.dll!mozilla::MediaDecoderStateMachine::AudioLoop() Line 1047    C++
> xul.dll!nsRunnableMethodImpl<void (__thiscall
> mozilla::MediaDecoderStateMachine::*)(void),1>::Run() Line 351    C++

Hehe, well: <http://mxr.mozilla.org/mozilla-central/source/media/libcubeb/src/cubeb_wasapi.cpp#617>

Comment 12

[Paul Adenot (:padenot)]

Build with an other tentative patch here: http://paul.cx/public/firefox-25.0a1.en-US.win32.zip

This new build has WASAPI logging enabled, maybe someone can tell me if it works, or what is the issue.

Comment 13

[Trevor Rowbotham [:rowbot]]

(In reply to Paul Adenot (:padenot) from comment #12)
> Build with an other tentative patch here:
> http://paul.cx/public/firefox-25.0a1.en-US.win32.zip
> 
> This new build has WASAPI logging enabled, maybe someone can tell me if it
> works, or what is the issue.

I no longer crash with this build, but no sound is played now, including distortions.  I also don't see anything being logged about WASAPI in the Browser/Error Consoles or in the console window that Firefox is running in.  Am I looking for the log messages in the wrong place? Switching the windows environment back to Stereo causes sound play normally without distortions.

Comment 14

[Paul Adenot (:padenot)]

So, I finally remembered my music computer at home has a ESI 1010, and I checked this morning and I can repro the bug there. Hopefully I can get this sorted out quicker.

Comment 15

[dgjunk]

You can set the audio output to 5.1 even without 5.1 speakers as long as the audio device is capable. Control Panel -> Hardware and Sound -> Sound -> Select Speakers, "Configure" button should be enabled, select 5.1

Comment 17

[Paul Adenot (:padenot)]

Attachment: patch

So, I've tried to make WASAPI understand that we want to output sound only in front left/right channels for stereo sound, but either my driver does not accept that, either WASAPI does not handle this (I've read both statement on the internet).

Anyways, this patch works by implementing upmixing ourselve. I've tested that sound output is just fine, now, with 2.1/5.1/7.1/9.1/quad setups (I don't have that much speakers, but I've got a 10 output soundcard with vu-meters so I could check WASAPI is not doing something stupid behind our back).

Comment 18

[Matthew Gregan [:kinetik]]

Comment on attachment 780937 [details] [diff] [review]
patch

Review of attachment 780937 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/libcubeb/src/cubeb_wasapi.cpp
@@ +170,5 @@
> +      out[out_index + j] = in[i + j];
> +    }
> +    for (int j = in_channels; j < out_channels; j++) {
> +      out[out_index + j] = 0.0;
> +    }

It'd be more efficient to reorganize the loop so that only one pass over out is required, but it may not matter in practice.

@@ +502,5 @@
>  
> +/* Based on the mix format and the stream format, try to find a way to play what
> + * the user requested. */
> +static void
> +handle_channel_layout(cubeb_stream * stm,  WAVEFORMATEX ** mix_format, cubeb_stream_params * stream_params)

Assert that the passed mix_format has a wFormatTag of WAVE_FORMAT_EXTENSIBLE.

Make stream_params const (or pass by value).

@@ +515,5 @@
> +  /* The hardware is in surround mode, we want to only use front left and front
> +   * right. Try that, and check if it works. */
> +  WAVEFORMATEXTENSIBLE * format_pcm = reinterpret_cast<WAVEFORMATEXTENSIBLE *>((*mix_format));
> +  format_pcm->Format.wFormatTag = WAVE_FORMAT_EXTENSIBLE;
> +  format_pcm->Format.cbSize = sizeof(*format_pcm) - sizeof(format_pcm->Format);

These should already be valid, otherwise the cast is not safe.

@@ +538,5 @@
> +  /* Check if wasapi will accept our channel layout request. */
> +  WAVEFORMATEX * closest;
> +  HRESULT hr = stm->client->IsFormatSupported(AUDCLNT_SHAREMODE_SHARED,
> +                                              *mix_format,
> +                                              reinterpret_cast<WAVEFORMATEX**>(&closest));

Is this cast needed?

Comment 19

[Paul Adenot (:padenot)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3de1033e44b7

Thank you all for your help with this one.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3de1033e44b7

Comment 21

[Leman Bennett [Omega]]

This is not completely fixed. Playback is clearer but there's still clipping and static.

Comment 22

[dgjunk]

Yes, audio is now actually recognizable, but there is quite a bit of static etc.

Comment 23

[dgjunk]

Interestingly the static/clipping isn't apparent in all html5 video/audio. On all Youtube videos it's there, on the opus demo link audio clips it isn't (http://people.xiph.org/~xiphmont/demo/opus/demo3.shtml) and on http://html5media.info/, the example video doesn't have clipping, but the audio only example has.

Comment 24

[Loic]

I'm not sure it's necessary to reopen this bug.
Bug 894801 has been filed about the remaining issue and some interesting details have been posted.

Comment 25

[Leman Bennett [Omega]]

That bug was not linked to this one so there was no way of knowing and also seems to be a duplicate of this bug regardless of the activity.

If the assigned developer feels this bug should remain closed and the work done in the other bug, then it shall be. Otherwise, this bug with the affecting patch is reopened due to the patch not fixing the issue 100%.

Comment 26

[Paul Adenot (:padenot)]

Attachment: wasapi-byteperframe

I forgot to update the way we compute the number of bytes per frame before the upmix, hence the observed glitches when taking the resampling path (the byte_per_frame_before_upmix function is not used in the non-resampling path).

I could repro on my desktop machine before using instruction provided by previous comments  (setting the channel layout to 5.1, and making sure to take the resampling path by playing a file at 48000Hz on my 44100Hz hardware), the sound is perfect for me, now.

Comment 27

[Paul Adenot (:padenot)]

Attachment: Copy the left channel over when playing on surround hardware

I also forgot this case: playing a mono sound in surround speakers should copy the left channel over to the right channel.

This is a temporary hackish fix, I'm writing an generic channel mapper to deal with this in a better way, and also 899050, it should not be too long.

Comment 28

[Matthew Gregan [:kinetik]]

Comment on attachment 786266 [details] [diff] [review]
wasapi-byteperframe

(In reply to Paul Adenot (:padenot) from comment #26)
> Created attachment 786266 [details] [diff] [review]
> wasapi-byteperframe

This patch is empty.

Comment 29

[Paul Adenot (:padenot)]

Ah, yes, the two patches got folded into the first for some reason.

Comment 30

[Paul Adenot (:padenot)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f74f1e59dc50

Comment 32

[Paul Adenot (:padenot)]

https://hg.mozilla.org/mozilla-central/rev/f74f1e59dc50 (this was not closed when merged because of the wrong bug number)

Comment 33

[Paul Adenot (:padenot)]

Attachment: cubeb update for uplift

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 866675

User impact if declined: anyone with a surround sound setup (2.1, 4.0, 5.1, 7.1, etc., not necessarily with that many speakers hooked up) and windows vista, 7 and 8 will have broken audio in <audio>, <video>, WebRTC audio output, WebAudio.

Testing completed (on m-c, etc.): m-c, manual testing by myself and other affected people.

Risk to taking this patch (and alternatives if risky): low, the problem and the solution are pretty straightforward.

String or IDL/UUID changes made by this patch: none

Comment 34

[Paul Adenot (:padenot)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4d5ceb7e190c

Comment 35

[Mihai Morar, (:MihaiMorar)]

Verified Fixed on FF 25b2 using Windows 8 x64