-
Notifications
You must be signed in to change notification settings - Fork 103
iOS SDK の RTCAudioSession に pauseRecording()/resumeRecording() 追加 #130
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
Conversation
This comment was marked as abuse.
This comment was marked as abuse.
This comment was marked as abuse.
This comment was marked as abuse.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as abuse.
This comment was marked as abuse.
This comment was marked as resolved.
This comment was marked as resolved.
ので設計を見直した方が良さそうです。 調べた限り ADM は ObjC 側に公開されていないので、
という感じにすれば RTCAudioDeviceModule の関数を呼んで pauseRecording, resumeRecording が出来そうです。 こうすると、iOS SDK 側の NativePeerChannelFactory.swift#L44-L47 あたりで adm = RTCAudioDeviceModule()
nativeFactory =
RTCPeerConnectionFactory(
encoderFactory: encoder,
decoderFactory: decoder,
audioDeviceModule: adm)みたいに書けて、あとは adm を適切に使えば iOS SDK から pause/resume できるようになるはず。 |
|
RTCAudioDeviceModule を追加する方向で修正します |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as abuse.
This comment was marked as abuse.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
何も回避できてないです。シングルトンはグローバル変数と同じです。 |
|
シングルトンを避ける方向で再度見直し中です |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
WeakPtr 関連は無くても平気なはずです。今までは RTCPeerConnectionFactory が解放された時に ADM が解放されてたのが、RTCPeerConnectionFactory と RTCAudioDeviceModule の両方が解放された時に ADM が解放されるようになるだけです。 |
|
なるほど。了解です。 |
…ording() で initRecording() は必要な場合のみ行う
cda9dea to
9359604
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
@melpon |
patches/ios_audio_pause_resume.patch
Outdated
| + decoderFactory | ||
| + audioDevice:(nullable id<RTC_OBJC_TYPE(RTCAudioDevice)>)audioDevice | ||
| + audioDeviceModule: | ||
| + (nullable RTC_OBJC_TYPE(RTCAudioDeviceModule)*)audioDeviceModule; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この関数は不要そう
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
audioDevice、audioDeviceModule 両方を引数に取る版は削除して、元の実装を維持しつつ
audioDeviceModule 引数版を追加するように修正しました
patches/ios_audio_pause_resume.patch
Outdated
| + // RTCAudioDeviceModule は内部でネイティブ ADM を生成・保持する(scoped_refptr による所有) | ||
| + // RTCPeerConnectionFactory は初期化時に RTCAudioDeviceModule から生ポインタを受け取る | ||
| + // RTCAudioDeviceModule が破棄されると内部 ADM も破棄される | ||
| + __weak RTC_OBJC_TYPE(RTCAudioDeviceModule)* _objcAudioDeviceModule; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ここに ADM 保持する意味ないです
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_objcAudioDeviceModule; メンバーを削除しました
patches/ios_audio_pause_resume.patch
Outdated
|
|
||
| /* Initialize object with injectable video encoder/decoder factories and | ||
| - * injectable ADM */ | ||
| + * injectable ADM (RTCAudioDevice) */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ここの変更は不要そう
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
変更を元に戻しました
patches/ios_audio_pause_resume.patch
Outdated
| +- (instancetype)init; | ||
| + | ||
| +// ネイティブ ADM を取得する(所有権移動なし、未初期化なら nullptr) | ||
| +- (void *)getNativeAudioDeviceModule; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
このメソッドは iOS SDK に公開されるヘッダーなんですが、
- 正しいポインタに戻す方法が無い
- そもそも公開する側で使うメソッドではない
ので使い道が無いです。RTCAudioDeviceModule+Private.h とかそういうファイルを作って、ちゃんと型を明示して C++ から参照できる形にして下さい
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RTCAudioDeviceModule+Private.h を作成し、nativeAudioDeviceModule 定義を移動しました
patches/ios_audio_pause_resume.patch
Outdated
| +// 内部で AudioDeviceModuleIOS を生成する | ||
| +- (instancetype)init; | ||
| + | ||
| +// ネイティブ ADM を取得する(所有権移動なし、未初期化なら nullptr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
コンストラクタで ADM を生成するので未初期化状態にはならないはずです。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
null チェック含めて修正しました
patches/ios_audio_pause_resume.patch
Outdated
| +- (BOOL)pauseRecording { | ||
| + auto ptr = | ||
| + static_cast<webrtc::ios_adm::AudioDeviceModuleIOS*>(_adm_owner.get()); | ||
| + if (!ptr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nullptr にはならないはずです
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
このチェックを外しました
patches/ios_audio_pause_resume.patch
Outdated
| + if (!ptr) { | ||
| + return NO; | ||
| + } | ||
| + return ptr->PauseRecording() == 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
このクラスは webrtc::AudioDeviceModule の ObjC 版なので、変換をせずにそのまま整数で返すべきです
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
呼び出しそのままの値を返すようにしました。関数返り値型を NSInteger に変更しました
patches/ios_audio_pause_resume.patch
Outdated
| + // InitRecording に失敗した場合は pause 状態の解除も recording_ の再立て直しも行えないため | ||
| + // 上位にエラーを返す | ||
| + if (!audio_is_initialized_) { | ||
| + if (InitRecording() != 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resume 時にまだ Init してない場合に Init してあげる必要ありますかね?ちょっとどういう使い方を想定してるのか分からないです。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ここではフラグ操作のみにしました
| return 0; | ||
| } | ||
|
|
||
| +int32_t AudioDeviceIOS::PauseRecording() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PauseRecording/ResumeRecording って、StartRecording/StopRecording と何が違うんでしょうか。見てる感じ StartRecording と StopRecording だけで事足りるように見えます
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StopRecording は内部で ShutdownPlayOrRecord() によりデバイス解放する部分が異なっています
ResumeRecording() は直接 StartRecording() を呼んでよいかもしれないです。確認します
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AudioDeviceIOS 内の PauseRecording/ResumeRecording は recording_ フラグの操作のみを行い、
startRecording() 等を呼ばないように修正しました
patches/ios_audio_pause_resume.patch
Outdated
| } | ||
|
|
||
| -AudioDeviceModuleIOS::~AudioDeviceModuleIOS() { | ||
| +int32_t AudioDeviceModuleIOS::AttachAudioBuffer() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
このあたりどうなってるんだろ、不要な変更に見えます
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
関数の順番だけ入れ替わった不要な変更になっていたので戻しました
|
@melpon |
patches/ios_audio_pause_resume.patch
Outdated
| } | ||
|
|
||
| - rtc_library("core_audio_helpers_objc") { | ||
| +rtc_library("core_audio_helpers_objc") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
インデントが消えてる?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ずれてしまっていたので修正しました
patches/ios_audio_pause_resume.patch
Outdated
| (nullable id<RTC_OBJC_TYPE(RTCAudioDevice)>)audioDevice; | ||
|
|
||
| +/* Initialize object with injectable video encoder/decoder factories and | ||
| + * RTCAudioDeviceModule (pause/resume 用) */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
これ Javadoc 形式でも無いし、普通の C コメントでもない中途半端な書き方なので統一して欲しいです。あと日本語で書いて下さい。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
コメント形式を直して日本語で書き直しました
patches/ios_audio_pause_resume.patch
Outdated
| dependencies.env = webrtc::CreateEnvironment(); | ||
| -#ifdef WEBRTC_IOS | ||
| - dependencies.adm = webrtc::CreateAudioDeviceModule(*dependencies.env); | ||
| -#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
これは消す必要無さそうに見えます
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
消す必要なかったですね。戻しました
patches/ios_audio_pause_resume.patch
Outdated
| + dependencies.video_decoder_factory = | ||
| + webrtc::ObjCToNativeVideoDecoderFactory(decoderFactory); | ||
| + } | ||
| +#ifdef WEBRTC_IOS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iOS でしかパッチを当てないので ifdef は不要そう
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
そもそもパッチターゲットがあるのを失念してました
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RTCAudioDeviceModule を引数に取る initWithEncoderFactory では #ifdef WEBRTC_IOS を削除しました
patches/ios_audio_pause_resume.patch
Outdated
| +#ifdef WEBRTC_IOS | ||
| + if (audioDeviceModule) { | ||
| + auto adm_ptr = [audioDeviceModule nativeAudioDeviceModule]; | ||
| + if (adm_ptr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adm_ptr が nullptr になることは無いはずです
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
その通りなので adm_ptr の null チェックは外しました
patches/ios_audio_pause_resume.patch
Outdated
| +- (instancetype)init; | ||
| + | ||
| +// 録音を一時停止/再開する | ||
| +// 内部で AudioDeviceModuleIOS -> AudioDeviceIOS の pauseRecording()/resumeRecording() を呼び出す |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
内部のことについて書くのはまあ良いとしても、AudioDeviceModuleIOS が AudioDeviceIOS の関数を呼んでいるかどうかに関してはこのクラスは知らない話なので書くべきではないです。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AudioDeviceIOS 部分のコメントを削除して、AudioDeviceModuleIOS の pauseRecording() を呼ぶ、までにしました
また resumeRecording() とそれぞれコメントを分離しました
patches/ios_audio_pause_resume.patch
Outdated
| +#import "RTCAudioDeviceModule.h" | ||
| +#include <objc/NSObjCRuntime.h> | ||
| + | ||
| +#include <arm_acle.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
このヘッダー何に使ってるんだろ。不要に見えます。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不要でした。削除しました
patches/ios_audio_pause_resume.patch
Outdated
| + return self; | ||
| +} | ||
| + | ||
| +#ifdef __cplusplus |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.mm ファイルなのでこの #ifdef いらないです
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
削除しました
patches/ios_audio_pause_resume.patch
Outdated
| if (!playing_.load()) { | ||
| ShutdownPlayOrRecord(); | ||
| } | ||
| + if (recording_.load()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この分岐必要なさそう
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
録音フラグ操作が二重で走っても問題ないので不要でした
patches/ios_audio_pause_resume.patch
Outdated
| return 0; | ||
| } | ||
| - audio_device_buffer_.get()->StartRecording(); | ||
| + if (audio_device_buffer_) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
このパッチで audio_device_buffer_ が nullptr になる可能性が増えてるようには見えないので修正する必要無さそうに見えます。他の分岐も同様です。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
startRecording()/stopRecording() 含めて audio_device_buffer_ の null チェックを外しました
また関数の呼び出しを audio_device_buffer_.get()-> で統一しました
| @class RTC_OBJC_TYPE(RTCVideoSource); | ||
| @class RTC_OBJC_TYPE(RTCVideoTrack); | ||
| @class RTC_OBJC_TYPE(RTCPeerConnectionFactoryOptions); | ||
| +@class RTC_OBJC_TYPE(RTCAudioDeviceModule); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@protocol~ の下に置いていたのを @class~ の並びの方に移動しました
| sources += [ | ||
| "objc/helpers/UIDevice+RTCDevice.h", | ||
| "objc/helpers/UIDevice+RTCDevice.mm", | ||
| + "objc/components/audio/RTCAudioDeviceModule.mm", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
現状では audio components 関連は RTCAudioDeviceModule のみのため新たにターゲットは切らずここに置く
|
@melpon |
CHANGES.md
Outdated
|
|
||
| - 2025-12-12 [ADD] iOS SDK に RTCAudioDeviceModule を追加する | ||
| - iOS 実機のマイクインジケータが消灯状態のミュートをできるようにする | ||
| - RTCPeerConnectionFactory に RTCAudioDeviceModule を引数とする initWithEncoderFactory() を追加する |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ObjC の場合これは initWithEncoderFactory:decoderFactory:audioDeviceModule: というメソッド名になります。
引数の名前まで含めてメソッド名で、C++ みたいな引数の型によるオーバーロードは存在しないです。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
initWithEncoderFactory:decoderFactory:audioDeviceModule: に修正しました。
その他コメント中で pauseRecording() のように書いていた箇所を pauseRecording 等に修正しました。
melpon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
よさそう
|
ありがとうございました🙇 |
iOS SDK での音声ハードミュートのため、pauseRecording()/resumeRecording() を追加します
RTCPeerConnectionFactory
init 時に AudioDeviceModule を受け取るパスを追加しました。
用途として RTCAudioDeviceModule のポインタを保持しライフサイクル管理します。
既存の DeviceModule を受け取るパスも維持しています。
RTCAudioDeviceModule(新規)
AudioDeviceModuleIOS を生成し、pauseRecording()/resumeRecording() するためのラッパーとして今回追加しました。
AudioDeviceModuleIOS
pauseRecording()/resumeRecording() を追加しました。
AudioDeviceIOS の pauseRecording()/resumeRecording() を呼び出します。
AudioDeviceIOS
追加
paused_recording_ フラグを追加して録音停止中であることを管理します。
pauseRecording() では recording_.store(0, std::memory_order_release); で録音フラグを OFF にします。
resumeRecording() では paused_recording_ を false にした上で startRecording() を実行して録音を再開します。
既存の修正
startRecording() はポーズ中 paused_recording_=true で呼ばれた場合は resumeRecording() にフォールスルーするようにしています。
stopRecording() では録音ポーズ中に呼ばれることがあるため paused_recording_ のチェックを追加して、録音停止中でも paused_recording_=true の場合は解放処理を行うようにしています。