Skip to content

#6109 Reinit device module on device change - #6347

Open
akleshchev wants to merge 2 commits into
release/26.4from
andreyk/viewer_6109_7
Open

akleshchev wants to merge 2 commits into
release/26.4from
andreyk/viewer_6109_7

Conversation

@akleshchev

Copy link
Copy Markdown
Contributor

Device change is a frequent cause of crashes and issues, reinit mDeviceModule.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Error-return paths added in this PR are still followed by continued startup work (and re-init calls ignore return codes), which can keep executing with a failed/unknown ADM state and makes failures harder to diagnose.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR aims to reduce crashes/issues during audio device changes by reinitializing the WebRTC audio device module (ADM) and adding basic failure logging during recording startup.

Changes:

  • Capture and log SetRecordingDevice() / InitMicrophone() failures when starting recording.
  • On device deploy, force-terminate and re-init the ADM when recording or playout is initialized.
File summaries
File Description
indra/llwebrtc/llwebrtc.cpp Adds device-change re-init logic for the ADM and improves logging around recording device/microphone initialization failures.
Review details

Suppressed comments (1)

indra/llwebrtc/llwebrtc.cpp:696

  • If InitMicrophone fails, continuing into InitRecording/ForceStartRecording risks starting capture with an uninitialized microphone. Logging the error code and returning early avoids subsequent calls on a bad device state.
    result = mDeviceModule->InitMicrophone();
    if (result != 0)
    {
        RTC_LOG(LS_WARNING) << "workerStartRecording: InitMicrophone failed.";
    }
  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread indra/llwebrtc/llwebrtc.cpp
Comment thread indra/llwebrtc/llwebrtc.cpp Outdated
@akleshchev

Copy link
Copy Markdown
Contributor Author

@roxanneskelly dan says that first commit fixed the crash for him, but it causes excessive 'allow microphone' promts. So I'm trying to do the same thing, but only on device removal (not change or addition).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The current coalescing/requeue logic can drop reset_module=true requests during concurrent device deploys, undermining the intended reinit-on-device-loss behavior.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity · 1 Low severity

Open (2)
Resolved since last review (2)

Comment thread indra/llwebrtc/llwebrtc.cpp
Comment thread indra/llwebrtc/llwebrtc.cpp Outdated
@akleshchev
akleshchev force-pushed the andreyk/viewer_6109_7 branch from 727c07f to 1530923 Compare September 19, 2026 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants