Skip to content

Conversation

@voluntas
Copy link
Member

No description provided.

- CHANGES.md の「部分一致」を「完全一致」に修正する
- デバイス識別を完全一致(大文字小文字を区別しない)に戻す
- 未使用の SelectAudioDevice 関数を削除する
@voluntas voluntas changed the title [WIP] Feature/macos audio device selection Feature/macos audio device selection Oct 15, 2025
@voluntas voluntas requested a review from melpon October 15, 2025 02:17
@voluntas voluntas self-assigned this Oct 15, 2025
@voluntas
Copy link
Member Author

!haiku レビューして

@github-actions
Copy link

github-actions bot commented Oct 16, 2025

Claude finished @voluntas's task —— View job


macOS オーディオデバイス選択機能のレビュー

実装概要

macOS で --audio-input-device--audio-output-device オプションを追加し、オーディオデバイスをインデックスまたはデバイス名(完全一致、大文字小文字を区別しない)で指定可能にする実装です。また --list-devices オプションでオーディオ・ビデオデバイスの一覧を表示します。


重要な指摘(優先度順)

1. 一時的な AudioDeviceModule の初期化失敗時の処理が不完全 ⚠️ 重大

場所: src/rtc/rtc_manager.cpp:80-98 (ResolveAudioDeviceIndex 関数)

問題:

auto temp_adm = webrtc::CreateAudioDeviceModule(...);
if (!temp_adm) {
  RTC_LOG(LS_WARNING) << ...;
  return false;
}
if (temp_adm->Init() != 0) {
  RTC_LOG(LS_WARNING) << ...;
  return false;  // ← temp_adm->Terminate() を呼ばずに返却
}

初期化失敗時に temp_adm->Terminate() を呼ばずに関数を終了しています。WebRTC の AudioDeviceModule は初期化後に終了処理が必要です。

修正案:

if (temp_adm->Init() != 0) {
  RTC_LOG(LS_WARNING) << ...;
  temp_adm->Terminate();  // 追加
  return false;
}

2. デバイス名解決が失敗した場合のエラーメッセージが曖昧 🔶 中程度

場所: src/rtc/rtc_manager.cpp:136-148

問題:
不利な状況で、どのオプション(input / output)でどのデバイスが見つからなかったのか、ユーザーに伝わらない可能性があります。

修正案:

std::string device_type = is_input ? "input" : "output";
RTC_LOG(LS_WARNING) << __FUNCTION__
                    << ": Audio " << device_type << " device not found. "
                    << "requested=" << device_spec
                    << " available=[" << available_str << "]";

3. 数値パース時の例外処理が粗い 🔶 中程度

場所: src/rtc/rtc_manager.cpp:159-181 (ResolveDeviceIndex 関数内の数値パース)

問題:

try {
  unsigned long parsed = std::stoul(device_spec);
  // ...
} catch (const std::exception&) {
  RTC_LOG(LS_WARNING) << ...;
  return false;
}

すべての例外を同じく処理しているため、具体的なエラー原因が分からなくなります。

修正案:

try {
  unsigned long parsed = std::stoul(device_spec);
  // ...
} catch (const std::invalid_argument&) {
  RTC_LOG(LS_WARNING) << __FUNCTION__
                      << ": Invalid device index format: " << device_spec;
  return false;
} catch (const std::out_of_range&) {
  RTC_LOG(LS_WARNING) << __FUNCTION__
                      << ": Device index out of range: " << device_spec;
  return false;
}

4. CHANGES.md との表現の整合性 🟡 軽微

場所: CHANGES.md:35-39src/momo_args.h:42-44

CHANGES.md では「デバイスはインデックス番号またはデバイス名(完全一致、大文字小文字を区別しない)で指定可能」と明記されていますが、実装ではデバイス名とデバイス GUID の両方で判定します (MatchDeviceIdentifier 関数)。

GUID で指定されたデバイスが見つかった場合もデバイス名として扱われるため、ドキュメントとの齟齬があります。


良い点

適切なデバイス一覧表示: ListDevices() 関数でオーディオ入力・出力デバイスとビデオデバイスを分かりやすく表示

大文字小文字を区別しない比較: CaseInsensitiveEqual() 関数でユーザーフレンドリーな仕様

デバイス再設定の再初期化: ReapplyAudioDevice() で WebRtcVoiceEngine がデフォルトデバイスに戻すことに対応

引数パース実装: util.cpp-–audio-input-device--audio-output-device オプションを適切に実装


コード品質に関する指摘

  1. IsNumber() の局所性: IsNumber() が名前空間 {} に入っているのは良いですが、ResolveDeviceIndex()ResolveAudioDeviceIndex() の関係性がやや複雑です。関数分割は合理的ですが、コメントを追加するとさらに可読性が向上します。

  2. インクルード: src/rtc/rtc_manager.cpp:13-35 で複数の include があり、モダン C++ の関数が使われています。適切です。


動作検証時の確認項目

  • macOS で --list-devices で全デバイスが表示されるか
  • 存在しないインデックス指定時のエラーメッセージ
  • 存在しないデバイス名指定時のエラーメッセージ
  • 大文字小文字を区別しない指定が実際に機能するか
  • デバイス設定後に正常に音声が送受信できるか

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