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

Status: RESOLVED FIXED

(Core :: Audio/Video: cubeb, defect, P1)

Description

[Randell Jesup [:jesup] (needinfo me)]

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

Comment 1

[Matthew Gregan [:kinetik]]

Attachment: bug1266623_cubeb_wasapi_input.patch

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.

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

 https://treeherder.mozilla.org/#/jobs?repo=try&revision=ef1dfd2cba1f

Comment 3

[Paul Adenot (:padenot)]

Comment on attachment 8744138 [details] [diff] [review]
bug1266623_cubeb_wasapi_input.patch

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.

Comment 4

[Matthew Gregan [:kinetik]]

Attachment: bug1266623_v2.patch

Comment 5

[Paul Adenot (:padenot)]

Comment on attachment 8744689 [details] [diff] [review]
bug1266623_v2.patch

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.

Comment 6

[Matthew Gregan [:kinetik]]

https://github.com/kinetiknz/cubeb/commit/6e2ac64d76d78597205170131d529bf00569af6d

Comment 7

[Matthew Gregan [:kinetik]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c6d33f5c60967cf715b0526c70d158ef51536c63
Bug 1266623 - Up/down mix WASAPI capture streams when stream formats don't match.  r=padenot

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c6d33f5c6096