Closed Bug 893307 Opened 6 years ago Closed 6 years ago

Audio playback is extremely distorted

Categories

(Core :: Audio/Video, defect)

x86_64
Windows 8
defect
Not set

Tracking

()

VERIFIED FIXED
Tracking Status
firefox24 --- unaffected
firefox25 + verified

People

(Reporter: lh.bennett, Assigned: padenot)

References

()

Details

(Keywords: regression)

Attachments

(5 files)

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.
This is directly captured audio from my machine using Audacity while playing the Sita sample from the demo page.
This also happens on my system (using Windows 7 x64).
Status: UNCONFIRMED → NEW
Ever confirmed: true
With a clean profile in Nightly and UX every HTML5 videos have distorted sound. Can't reproduce on Firefox 22 either.
Component: Web Audio → Video/Audio
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.
Blocks: 866675
Keywords: regression
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
Flags: needinfo?(lh.bennett)
Flags: needinfo?(smokey101stair)
Flags: needinfo?(emanuel.hoogeveen)
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.
Flags: needinfo?(emanuel.hoogeveen)
Duplicate of this bug: 894801
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/paul@paul.cx-faeae4c58e55/
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).
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++
(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>
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.
(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.
Flags: needinfo?(smokey101stair)
Assignee: nobody → paul
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.
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
Duplicate of this bug: 897500
Attached patch patchSplinter Review
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).
Attachment #780937 - Flags: review?(kinetik)
Flags: needinfo?(lh.bennett)
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?
Attachment #780937 - Flags: review?(kinetik) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/3de1033e44b7

Thank you all for your help with this one.
https://hg.mozilla.org/mozilla-central/rev/3de1033e44b7
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
This is not completely fixed. Playback is clearer but there's still clipping and static.
Yes, audio is now actually recognizable, but there is quite a bit of static etc.
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
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%.
Depends on: 900380
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.
Attachment #786266 - Flags: review?(kinetik)
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.
Attachment #786270 - Flags: review?(kinetik)
Attachment #786270 - Flags: review?(kinetik) → review+
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.
Attachment #786266 - Flags: review?(kinetik) → review-
Ah, yes, the two patches got folded into the first for some reason.
Duplicate of this bug: 894801
https://hg.mozilla.org/mozilla-central/rev/f74f1e59dc50 (this was not closed when merged because of the wrong bug number)
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: mozilla25 → ---
[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
Attachment #788950 - Flags: approval-mozilla-aurora?
Attachment #788950 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified Fixed on FF 25b2 using Windows 8 x64
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.