Skip to content

[Windows] Fixed the audio capture issue when the built-in AEC enabled #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

cloudwebrtc
Copy link
Member

@cloudwebrtc cloudwebrtc commented May 5, 2022

Fixed the issue when the built-in AEC is valid under Windows, the mic cannot be captured when StartRecording is called before StartPlayout.

Trigger conditions for this bug:
1, windows and supports built-in AEC enabled
2. StartRecording is called before StartPlayOut, such as SendOnly mode, or AudioSendStream is enabled before AudioRecvStream

The raised issue, unable to capture mic and no audio stream sent

…e mic cannot be captured when StartRecording is called before StartPlayout.
@cloudwebrtc cloudwebrtc requested review from davidliu and davidzhao May 5, 2022 15:47
@cloudwebrtc cloudwebrtc changed the title Fixed the issue when the built-in AEC enabled under Windows [Windows] Fixed the issue when the built-in AEC enabled under Windows May 5, 2022
@cloudwebrtc cloudwebrtc changed the title [Windows] Fixed the issue when the built-in AEC enabled under Windows [Windows] Fixed the issue when the built-in AEC enabled May 5, 2022
@cloudwebrtc cloudwebrtc changed the title [Windows] Fixed the issue when the built-in AEC enabled [Windows] Fixed the audio capture issue when the built-in AEC enabled May 5, 2022
@davidliu
Copy link
Contributor

davidliu commented May 6, 2022

Hey, can you do this against the m97_release branch? Main is the old m93 release.

@cloudwebrtc
Copy link
Member Author

sure, I forgot about this. xD

@@ -2356,7 +2356,7 @@ int32_t AudioDeviceWindowsCore::StartRecording() {
}
}

RTC_DCHECK(_hRecThread);
RTC_DCHECK(_hRecThread == NULL);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this is fixed in m97.

@@ -2493,7 +2493,7 @@ int32_t AudioDeviceWindowsCore::StartPlayout() {
MutexLock lockScoped(&mutex_);

// Create thread which will drive the rendering.
RTC_DCHECK(_hPlayThread);
RTC_DCHECK(_hPlayThread == NULL);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this is fixed in m97.

if (!adm->PlayoutIsInitialized()) {
adm->InitPlayout();
}
adm->StartPlayout();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you write a comment explaining why this is needed? Seems weird to require playout for recording.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is caused here https://github.com/webrtc-sdk/webrtc/blob/main/modules/audio_device/win/audio_device_core_win.cc#L2353. If the built-in AEC is enabled, opening the Recording alone will fail to create hRecThread, resulting in no sound captured by the send stream.

But if StartPlayOut is called early, it works both ways.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the code here is a bit strange, with the built-in AEC enabled, recording/playout cannot be started independently. ,

@cloudwebrtc
Copy link
Member Author

new PR here. #29

@cloudwebrtc cloudwebrtc closed this Jun 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants