Distortion capturing audio in full_duplex mode on windows from W520 built-in mic

RESOLVED FIXED in Firefox 49



Audio/Video: cubeb
2 years ago
2 years ago


(Reporter: jesup, Assigned: kinetik)


Dependency tree / graph

Firefox Tracking Flags

(firefox48 affected, firefox49 fixed)



(1 attachment, 1 obsolete attachment)



2 years ago
Per kinetik, it's capturing in stereo though we're expecting mono

Comment 1

2 years ago
Created attachment 8744138 [details] [diff] [review]

I could reproduce something (most likely the same issue) with a Xonar U7 and HyperX Cloud headset.  The mic is configured as 2 channel/24-bit/48kHz in Windows.

We try to configure an input stream, requesting 1 channel/float/48kHz.  In setup_wasapi_stream_one_side we get a mix_format back of 2 channel/float/48kHz, hit the <= 2 channel early exit in handle_channel_layout, and go ahead setting up the stream.

For the output side, we up/down mix the audio before handing it off to the render client.  That piece of code was missing on the input side, and with a mismatched stream like we configured here, we'd pass the wrong format to the user data_callback.  up/down mixing at the point we linearize the input packets seemed the most logical place to perform the conversion (and matches the flow on the output side).

There we also a couple of places incorrectly using the stream or mix variant of the params, fixed in this patch.

One part I'm not sure about is the special case to pad the input with silence in refill_callback_duplex, which we used to do only when we had no input and now do if we have *less* input.  The resampler asserts that input frames >= output frames, and I was seeing a case at startup where we'd have 960 input frames and 1036 output frames, triggering the resampler assert.

Testing more locally and waiting to see if this fixes jesup's issue before requesting review.


2 years ago
Attachment #8744138 - Flags: review?(padenot)
Comment on attachment 8744138 [details] [diff] [review]

Review of attachment 8744138 [details] [diff] [review]:

Looks good, apart from the comment.

::: media/libcubeb/src/cubeb_wasapi.cpp
@@ +724,5 @@
>    if (stm->linear_input_buffer.length() == 0) {
> +#endif
> +  if (input_frames < output_frames) {
> +    LOG("padding silence: out=%u in=%u\n", output_frames, input_frames);
> +    stm->linear_input_buffer.push_silence((output_frames - input_frames) * stm->input_stream_params.channels);

If the two streams are not at the same rate, then this can happen. The goal of the resampler is then to resample both streams at the same rate, as they might not be at the same rate (different sound cards or configuration).

I think the easiest is to determine if we're missing frames in the input stream and pad with enough silent frame to realign the streams, taking the resampling ratio into account.

In fact, looking at this, my code was incorrect, it should have been:

> float out_rate_ratio = stm->output_mix_params.rate / stm->output_stream_params.rate;
> stm->linear_input_buffer.push_silence((output_frames * out_rate_ratio - input_frames) * stm->input_mix_params.channels);

so that we add the appropriate number of silent sample to compensate for the missing frames compared to the output. I think it would be better to add them at the beginning, because this seem to happen on startup, so we'd avoid inserting a gap of silence.
Attachment #8744138 - Flags: review?(padenot)

Comment 4

2 years ago
Created attachment 8744689 [details] [diff] [review]
Attachment #8744138 - Attachment is obsolete: true
Attachment #8744689 - Flags: review?(padenot)
Comment on attachment 8744689 [details] [diff] [review]

Review of attachment 8744689 [details] [diff] [review]:

r+ with the fix.

::: media/libcubeb/src/cubeb_utils.h
@@ +141,5 @@
> +  {
> +    if (length_ + length > capacity_) {
> +      reserve(length + length_);
> +    }
> +    PodCopy(data_ + length, data_, length_);

The memory areas for source and destination are overlapping, aren't they ? Best to use PodMove.
Attachment #8744689 - Flags: review?(padenot) → review+

Comment 7

2 years ago
Bug 1266623 - Up/down mix WASAPI capture streams when stream formats don't match.  r=padenot


2 years ago
Rank: 10
Priority: -- → P1

Comment 8

2 years ago
Last Resolved: 2 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48


2 years ago
Target Milestone: mozilla48 → mozilla49
Depends on: 1268719


2 years ago
Depends on: 1273349
You need to log in before you can comment on or make changes to this bug.