Closed
Bug 1266623
Opened 8 years ago
Closed 8 years ago
Distortion capturing audio in full_duplex mode on windows from W520 built-in mic
Categories
(Core :: Audio/Video: cubeb, defect, P1)
Core
Audio/Video: cubeb
Tracking
()
RESOLVED
FIXED
mozilla49
People
(Reporter: jesup, Assigned: kinetik)
References
Details
Attachments
(1 file, 1 obsolete file)
12.84 KB,
patch
|
padenot
:
review+
|
Details | Diff | Splinter Review |
Per kinetik, it's capturing in stereo though we're expecting mono
Assignee | ||
Comment 1•8 years ago
|
||
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.
Reporter | ||
Comment 2•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ef1dfd2cba1f
Assignee | ||
Updated•8 years ago
|
Attachment #8744138 -
Flags: review?(padenot)
Comment 3•8 years ago
|
||
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.
Attachment #8744138 -
Flags: review?(padenot)
Assignee | ||
Comment 4•8 years ago
|
||
Attachment #8744138 -
Attachment is obsolete: true
Attachment #8744689 -
Flags: review?(padenot)
Comment 5•8 years ago
|
||
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.
Attachment #8744689 -
Flags: review?(padenot) → review+
Assignee | ||
Comment 6•8 years ago
|
||
https://github.com/kinetiknz/cubeb/commit/6e2ac64d76d78597205170131d529bf00569af6d
Assignee | ||
Comment 7•8 years ago
|
||
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
Reporter | ||
Updated•8 years ago
|
Rank: 10
Priority: -- → P1
Comment 8•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c6d33f5c6096
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Updated•8 years ago
|
Target Milestone: mozilla48 → mozilla49
You need to log in
before you can comment on or make changes to this bug.
Description
•