From 4e2be0d00f2c1f2a80a1465ab464dc35b6a340d1 Mon Sep 17 00:00:00 2001 From: Cody Barnes Date: Mon, 8 Jun 2026 23:08:21 -0700 Subject: [PATCH 1/5] Refactor media GetNative ownership with NativeWrapper composition BugzId:61 --- WebRtcInterop/Media/MediaStream.cpp | 63 ++++++++++++++---------- WebRtcInterop/Media/MediaStream.h | 7 ++- WebRtcInterop/Media/MediaStreamTrack.cpp | 30 +++++------ WebRtcInterop/Media/MediaStreamTrack.h | 7 +-- WebRtcInterop/NativeWrapper.h | 11 +++-- WebRtcNet.Api/Media/MediaStream.cs | 9 ---- WebRtcNet.Api/Media/MediaStreamTrack.cs | 9 ---- 7 files changed, 65 insertions(+), 71 deletions(-) diff --git a/WebRtcInterop/Media/MediaStream.cpp b/WebRtcInterop/Media/MediaStream.cpp index ea49293..10621af 100644 --- a/WebRtcInterop/Media/MediaStream.cpp +++ b/WebRtcInterop/Media/MediaStream.cpp @@ -13,7 +13,7 @@ using namespace WebRtcNet; namespace WebRtcInterop::Media { MediaStream::MediaStream(WebRtcNet::Media::MediaStream^ stream) - : _rpMediaStreamInterface(nullptr), + : _nativeStreamInterface(nullptr), on_active_(nullptr), on_inactive_(nullptr), on_add_track_(nullptr), @@ -25,13 +25,12 @@ namespace WebRtcInterop::Media throw gcnew InvalidCastException("The provided stream must be a WebRtcInterop::Media::MediaStream instance."); } - auto native_stream = interop_stream->GetNativeMediaStreamInterface(true); - _rpMediaStreamInterface = new webrtc::scoped_refptr( - reinterpret_cast(native_stream.ToPointer())); + _nativeStreamInterface = gcnew NativeWrapper( + interop_stream->GetNativeMediaStreamInterface(true)); } MediaStream::MediaStream(webrtc::scoped_refptr stream) - : _rpMediaStreamInterface(new webrtc::scoped_refptr(stream)), + : _nativeStreamInterface(gcnew NativeWrapper(stream)), on_active_(nullptr), on_inactive_(nullptr), on_add_track_(nullptr), @@ -46,30 +45,30 @@ namespace WebRtcInterop::Media MediaStream::!MediaStream() { - delete _rpMediaStreamInterface; - _rpMediaStreamInterface = nullptr; + delete _nativeStreamInterface; + _nativeStreamInterface = nullptr; } - IntPtr MediaStream::GetNativeMediaStreamInterface(bool throwOnDisposed) + webrtc::scoped_refptr MediaStream::GetNativeMediaStreamInterface(bool throwOnDisposed) { - if (_rpMediaStreamInterface == nullptr || _rpMediaStreamInterface->get() == nullptr) + if (_nativeStreamInterface == nullptr || !_nativeStreamInterface->HasValue()) { if (throwOnDisposed) throw gcnew ObjectDisposedException(NAMEOF(MediaStream)); - return IntPtr::Zero; + return webrtc::scoped_refptr(); } - return IntPtr(_rpMediaStreamInterface->get()); + return _nativeStreamInterface->GetScopedRef(); } String^ MediaStream::Id::get() { - const auto native = _rpMediaStreamInterface->get(); + const auto native = _nativeStreamInterface->Get(); return marshal_as(native->id()); } IEnumerable^ MediaStream::GetAudioTracks() { - const auto native = _rpMediaStreamInterface->get(); + const auto native = _nativeStreamInterface->Get(); auto tracks = gcnew List(); for (const auto& track : native->GetAudioTracks()) @@ -82,7 +81,7 @@ namespace WebRtcInterop::Media IEnumerable^ MediaStream::GetVideoTracks() { - const auto native = _rpMediaStreamInterface->get(); + const auto native = _nativeStreamInterface->Get(); auto tracks = gcnew List(); for (const auto& track : native->GetVideoTracks()) @@ -105,7 +104,7 @@ namespace WebRtcInterop::Media { if (trackId == nullptr) throw gcnew ArgumentNullException(NAMEOF(trackId)); - const auto native = _rpMediaStreamInterface->get(); + const auto native = _nativeStreamInterface->Get(); const auto native_track_id = marshal_as(trackId); const auto audio = native->FindAudioTrack(native_track_id); @@ -121,19 +120,25 @@ namespace WebRtcInterop::Media { if (track == nullptr) throw gcnew ArgumentNullException(NAMEOF(track)); - const auto native_stream = _rpMediaStreamInterface->get(); - const auto native_track = reinterpret_cast( - track->GetNativeMediaStreamTrackInterface(true).ToPointer()); + const auto native_stream = _nativeStreamInterface->Get(); + const auto interop_track = dynamic_cast(track); + if (interop_track == nullptr) + { + throw gcnew InvalidCastException("The provided track must be a WebRtcInterop::Media::MediaStreamTrack instance."); + } + + const auto native_track_ref = interop_track->GetNativeMediaStreamTrackInterface(true); + const auto native_track = native_track_ref.get(); if (native_track->kind() == webrtc::MediaStreamTrackInterface::kAudioKind) { native_stream->AddTrack( - webrtc::scoped_refptr(static_cast(native_track))); + webrtc::scoped_refptr(static_cast(native_track_ref.get()))); } else if (native_track->kind() == webrtc::MediaStreamTrackInterface::kVideoKind) { native_stream->AddTrack( - webrtc::scoped_refptr(static_cast(native_track))); + webrtc::scoped_refptr(static_cast(native_track_ref.get()))); } else { @@ -148,19 +153,25 @@ namespace WebRtcInterop::Media { if (track == nullptr) throw gcnew ArgumentNullException(NAMEOF(track)); - const auto native_stream = _rpMediaStreamInterface->get(); - const auto native_track = reinterpret_cast( - track->GetNativeMediaStreamTrackInterface(true).ToPointer()); + const auto native_stream = _nativeStreamInterface->Get(); + const auto interop_track = dynamic_cast(track); + if (interop_track == nullptr) + { + throw gcnew InvalidCastException("The provided track must be a WebRtcInterop::Media::MediaStreamTrack instance."); + } + + const auto native_track_ref = interop_track->GetNativeMediaStreamTrackInterface(true); + const auto native_track = native_track_ref.get(); if (native_track->kind() == webrtc::MediaStreamTrackInterface::kAudioKind) { native_stream->RemoveTrack( - webrtc::scoped_refptr(static_cast(native_track))); + webrtc::scoped_refptr(static_cast(native_track_ref.get()))); } else if (native_track->kind() == webrtc::MediaStreamTrackInterface::kVideoKind) { native_stream->RemoveTrack( - webrtc::scoped_refptr(static_cast(native_track))); + webrtc::scoped_refptr(static_cast(native_track_ref.get()))); } else { @@ -178,7 +189,7 @@ namespace WebRtcInterop::Media Boolean MediaStream::Active::get() { - const auto native = _rpMediaStreamInterface->get(); + const auto native = _nativeStreamInterface->Get(); for (const auto& track : native->GetAudioTracks()) { diff --git a/WebRtcInterop/Media/MediaStream.h b/WebRtcInterop/Media/MediaStream.h index c7b27cf..97d1f81 100644 --- a/WebRtcInterop/Media/MediaStream.h +++ b/WebRtcInterop/Media/MediaStream.h @@ -1,6 +1,7 @@ #pragma once #include +#include "../NativeWrapper.h" namespace webrtc { @@ -54,12 +55,10 @@ namespace WebRtcInterop::Media internal: MediaStream(webrtc::scoped_refptr stream); !MediaStream(); - - public: - IntPtr GetNativeMediaStreamInterface(bool throwOnDisposed) override; + webrtc::scoped_refptr GetNativeMediaStreamInterface(bool throwOnDisposed); private: - webrtc::scoped_refptr* _rpMediaStreamInterface; + NativeWrapper^ _nativeStreamInterface; EventHandler^ on_active_; EventHandler^ on_inactive_; EventHandler^ on_add_track_; diff --git a/WebRtcInterop/Media/MediaStreamTrack.cpp b/WebRtcInterop/Media/MediaStreamTrack.cpp index ed5955a..581f403 100644 --- a/WebRtcInterop/Media/MediaStreamTrack.cpp +++ b/WebRtcInterop/Media/MediaStreamTrack.cpp @@ -14,7 +14,7 @@ using namespace WebRtcNet::Media; namespace WebRtcInterop::Media { MediaStreamTrack::MediaStreamTrack() - : _rpMediaStreamTrackInterface(nullptr), + : _nativeMediaStreamTrackInterface(nullptr), applied_constraints_(gcnew MediaTrackConstraints()), on_mute_(nullptr), on_unmute_(nullptr), @@ -23,7 +23,7 @@ namespace WebRtcInterop::Media } MediaStreamTrack::MediaStreamTrack(webrtc::scoped_refptr track) - : _rpMediaStreamTrackInterface(new webrtc::scoped_refptr(track)), + : _nativeMediaStreamTrackInterface(gcnew NativeWrapper(track)), applied_constraints_(gcnew MediaTrackConstraints()), on_mute_(nullptr), on_unmute_(nullptr), @@ -39,30 +39,30 @@ namespace WebRtcInterop::Media MediaStreamTrack::!MediaStreamTrack() { - delete _rpMediaStreamTrackInterface; - _rpMediaStreamTrackInterface = nullptr; + delete _nativeMediaStreamTrackInterface; + _nativeMediaStreamTrackInterface = nullptr; } - IntPtr MediaStreamTrack::GetNativeMediaStreamTrackInterface(bool throwOnDisposed) + webrtc::scoped_refptr MediaStreamTrack::GetNativeMediaStreamTrackInterface(bool throwOnDisposed) { - if (_rpMediaStreamTrackInterface == nullptr || _rpMediaStreamTrackInterface->get() == nullptr) + if (_nativeMediaStreamTrackInterface == nullptr || !_nativeMediaStreamTrackInterface->HasValue()) { if (throwOnDisposed) throw gcnew ObjectDisposedException("MediaStreamTrack"); - return IntPtr::Zero; + return webrtc::scoped_refptr(); } - return IntPtr(_rpMediaStreamTrackInterface->get()); + return _nativeMediaStreamTrackInterface->GetScopedRef(); } MediaStreamTrackKind MediaStreamTrack::Kind::get() { - const auto native = _rpMediaStreamTrackInterface->get(); + const auto native = _nativeMediaStreamTrackInterface->Get(); return marshal_as(native->kind()); } String^ MediaStreamTrack::Id::get() { - const auto native = _rpMediaStreamTrackInterface->get(); + const auto native = _nativeMediaStreamTrackInterface->Get(); return marshal_as(native->id()); } @@ -73,19 +73,19 @@ namespace WebRtcInterop::Media bool MediaStreamTrack::Enabled::get() { - const auto native = _rpMediaStreamTrackInterface->get(); + const auto native = _nativeMediaStreamTrackInterface->Get(); return native->enabled(); } void MediaStreamTrack::Enabled::set(bool value) { - const auto native = _rpMediaStreamTrackInterface->get(); + const auto native = _nativeMediaStreamTrackInterface->Get(); native->set_enabled(value); } bool MediaStreamTrack::Muted::get() { - const auto native = _rpMediaStreamTrackInterface->get(); + const auto native = _nativeMediaStreamTrackInterface->Get(); if (native->kind() == webrtc::MediaStreamTrackInterface::kAudioKind) { @@ -106,13 +106,13 @@ namespace WebRtcInterop::Media MediaStreamTrackState MediaStreamTrack::ReadyState::get() { - const auto native = _rpMediaStreamTrackInterface->get(); + const auto native = _nativeMediaStreamTrackInterface->Get(); return marshal_as(native->state()); } WebRtcNet::Media::MediaStreamTrack^ MediaStreamTrack::Clone() { - return gcnew MediaStreamTrack(*_rpMediaStreamTrackInterface); + return gcnew MediaStreamTrack(_nativeMediaStreamTrackInterface->GetScopedRef()); } void MediaStreamTrack::Stop() diff --git a/WebRtcInterop/Media/MediaStreamTrack.h b/WebRtcInterop/Media/MediaStreamTrack.h index 3cfb67f..13a2be1 100644 --- a/WebRtcInterop/Media/MediaStreamTrack.h +++ b/WebRtcInterop/Media/MediaStreamTrack.h @@ -1,6 +1,7 @@ #pragma once #include +#include "../NativeWrapper.h" namespace webrtc { @@ -53,11 +54,11 @@ namespace WebRtcInterop::Media MediaTrackSettings^ GetSettings() override; void ApplyConstraints(MediaTrackConstraints^ constraints) override; - public: - IntPtr GetNativeMediaStreamTrackInterface(bool throwOnDisposed) override; + internal: + webrtc::scoped_refptr GetNativeMediaStreamTrackInterface(bool throwOnDisposed); private: - webrtc::scoped_refptr* _rpMediaStreamTrackInterface; + NativeWrapper^ _nativeMediaStreamTrackInterface; MediaTrackConstraints^ applied_constraints_; EventHandler^ on_mute_; EventHandler^ on_unmute_; diff --git a/WebRtcInterop/NativeWrapper.h b/WebRtcInterop/NativeWrapper.h index 69525f0..ce6d305 100644 --- a/WebRtcInterop/NativeWrapper.h +++ b/WebRtcInterop/NativeWrapper.h @@ -5,7 +5,7 @@ namespace WebRtcInterop #include template - public ref class NativeWrapper abstract + public ref class NativeWrapper { public: NativeWrapper(T* native) @@ -14,16 +14,17 @@ namespace WebRtcInterop } NativeWrapper(webrtc::scoped_refptr rp_native) - : rp_native_(rp_native) + : rp_native_(new webrtc::scoped_refptr(rp_native)) { } ~NativeWrapper() { this->!NativeWrapper(); } - T* Get() { return rp_native_; } + bool HasValue() { return rp_native_ != nullptr && rp_native_->get() != nullptr; } + T* Get() { return rp_native_ == nullptr ? nullptr : rp_native_->get(); } + webrtc::scoped_refptr GetScopedRef() { return rp_native_ == nullptr ? webrtc::scoped_refptr() : *rp_native_; } - explicit operator bool() { return rp_native_ != nullptr && rp_native_->get() != nullptr; } - T* operator->() { return rp_native_; } + explicit operator bool() { return HasValue(); } internal: !NativeWrapper() diff --git a/WebRtcNet.Api/Media/MediaStream.cs b/WebRtcNet.Api/Media/MediaStream.cs index b51b67a..2daa730 100644 --- a/WebRtcNet.Api/Media/MediaStream.cs +++ b/WebRtcNet.Api/Media/MediaStream.cs @@ -22,15 +22,6 @@ protected MediaStream() /// public abstract string Id { get; } - /// - /// Returns the native media stream interface used by WebRtcInterop. - /// - /// - /// This method is intended for internal use by WebRtcInterop implementations only. - /// - /// True to throw when the stream has already been disposed. - public abstract IntPtr GetNativeMediaStreamInterface(bool throwOnDisposed); - /// /// Returns a sequence of MediaStreamTrack objects representing the audio tracks in this stream. /// diff --git a/WebRtcNet.Api/Media/MediaStreamTrack.cs b/WebRtcNet.Api/Media/MediaStreamTrack.cs index adc0fb4..91f9cec 100644 --- a/WebRtcNet.Api/Media/MediaStreamTrack.cs +++ b/WebRtcNet.Api/Media/MediaStreamTrack.cs @@ -64,15 +64,6 @@ public abstract class MediaStreamTrack /// public abstract MediaStreamTrackKind Kind { get; } - /// - /// Returns the native media stream track interface used by WebRtcInterop. - /// - /// - /// This method is intended for internal use by WebRtcInterop implementations only. - /// - /// True to throw when the track has already been disposed. - public abstract IntPtr GetNativeMediaStreamTrackInterface(bool throwOnDisposed); - /// /// A generated identifier for the track. /// From f99cb030f5f1954db9f4bef337cef7f5ed586e34 Mon Sep 17 00:00:00 2001 From: Cody Barnes Date: Mon, 8 Jun 2026 23:11:57 -0700 Subject: [PATCH 2/5] Refactor RtcDataChannel GetNative to scoped_refptr BugzId:62 --- WebRtcInterop/RtcDataChannel.cpp | 11 +++-------- WebRtcInterop/RtcDataChannel.h | 6 +++--- WebRtcNet.Api/RtcDataChannel.cs | 9 --------- 3 files changed, 6 insertions(+), 20 deletions(-) diff --git a/WebRtcInterop/RtcDataChannel.cpp b/WebRtcInterop/RtcDataChannel.cpp index 58cd3ca..3f4e720 100644 --- a/WebRtcInterop/RtcDataChannel.cpp +++ b/WebRtcInterop/RtcDataChannel.cpp @@ -40,21 +40,16 @@ namespace WebRtcInterop rp_data_channel_interface_ = nullptr; } - webrtc::DataChannelInterface* RtcDataChannel::GetNativeDataChannelInterface(const bool throwOnDisposed) + webrtc::scoped_refptr RtcDataChannel::GetNativeDataChannelInterface(const bool throwOnDisposed) { const auto result = rp_data_channel_interface_.Get(); if (result == nullptr) { if (throwOnDisposed) throw gcnew ObjectDisposedException(NAMEOF(RtcDataChannel)); - return nullptr; + return webrtc::scoped_refptr(); } - return result; - } - - IntPtr RtcDataChannel::GetNativeDataChannelHandle(bool throwOnDisposed) - { - return IntPtr(GetNativeDataChannelInterface(throwOnDisposed)); + return webrtc::scoped_refptr(result); } String^ RtcDataChannel::Label::get() diff --git a/WebRtcInterop/RtcDataChannel.h b/WebRtcInterop/RtcDataChannel.h index e6b8691..257e2c8 100644 --- a/WebRtcInterop/RtcDataChannel.h +++ b/WebRtcInterop/RtcDataChannel.h @@ -4,6 +4,8 @@ namespace webrtc { + template + class scoped_refptr; class DataChannelInterface; } @@ -69,12 +71,10 @@ namespace WebRtcInterop void Send(Collections::Generic::IEnumerable^ data) override; void Send(array^ data) override; - IntPtr GetNativeDataChannelHandle(bool throwOnDisposed) override; - internal: RtcDataChannel(webrtc::DataChannelInterface* data_channel_interface); !RtcDataChannel(); - webrtc::DataChannelInterface* GetNativeDataChannelInterface(bool throwOnDisposed); + webrtc::scoped_refptr GetNativeDataChannelInterface(bool throwOnDisposed); internal: //Event invocation diff --git a/WebRtcNet.Api/RtcDataChannel.cs b/WebRtcNet.Api/RtcDataChannel.cs index 7b12568..786a413 100644 --- a/WebRtcNet.Api/RtcDataChannel.cs +++ b/WebRtcNet.Api/RtcDataChannel.cs @@ -99,15 +99,6 @@ protected RtcDataChannel() { } - /// - /// Returns the native data channel handle used by WebRtcInterop. - /// - /// - /// This method is intended for internal use by WebRtcInterop implementations only. - /// - /// True to throw when the data channel has already been disposed. - public abstract IntPtr GetNativeDataChannelHandle(bool throwOnDisposed); - /// /// The Label represents a label that can be used to distinguish this RtcDataChannel object from other RtcDataChannel /// objects. Applications are allowed to create multiple RtcDataChannel objects with the same label. From c2960c0d086a8a0a15e7422e9e7445a3ae611e2d Mon Sep 17 00:00:00 2001 From: Cody Barnes Date: Mon, 8 Jun 2026 23:15:07 -0700 Subject: [PATCH 3/5] Refactor RtcIceTransport GetNative to scoped_refptr BugzId:63 --- WebRtcInterop/RtcIceTransport.cpp | 12 +++--------- WebRtcInterop/RtcIceTransport.h | 6 +++--- WebRtcNet.Api/RtcIceTransport.cs | 8 -------- 3 files changed, 6 insertions(+), 20 deletions(-) diff --git a/WebRtcInterop/RtcIceTransport.cpp b/WebRtcInterop/RtcIceTransport.cpp index 3a35bed..7366d22 100644 --- a/WebRtcInterop/RtcIceTransport.cpp +++ b/WebRtcInterop/RtcIceTransport.cpp @@ -25,21 +25,16 @@ namespace WebRtcInterop rp_ice_transport_interface_ = nullptr; } - webrtc::IceTransportInterface* RtcIceTransport::GetNativeIceTransportInterface(bool throwOnDisposed) + webrtc::scoped_refptr RtcIceTransport::GetNativeIceTransportInterface(bool throwOnDisposed) { const auto result = rp_ice_transport_interface_.Get(); if (result == nullptr) { if (throwOnDisposed) throw gcnew ObjectDisposedException(NAMEOF(RtcIceTransport)); - return nullptr; + return webrtc::scoped_refptr(); } - return result; - } - - IntPtr RtcIceTransport::GetNativeIceTransportHandle(bool throwOnDisposed) - { - return IntPtr(GetNativeIceTransportInterface(throwOnDisposed)); + return webrtc::scoped_refptr(result); } RtcIceRole RtcIceTransport::Role::get() @@ -88,4 +83,3 @@ namespace WebRtcInterop } } - diff --git a/WebRtcInterop/RtcIceTransport.h b/WebRtcInterop/RtcIceTransport.h index c915d5e..022690d 100644 --- a/WebRtcInterop/RtcIceTransport.h +++ b/WebRtcInterop/RtcIceTransport.h @@ -4,6 +4,8 @@ namespace webrtc { + template + class scoped_refptr; class IceTransportInterface; } @@ -44,12 +46,10 @@ namespace WebRtcInterop void remove(EventHandler^ value) override { on_selected_candidate_pair_change_ -= value; } } - IntPtr GetNativeIceTransportHandle(bool throwOnDisposed) override; - internal: RtcIceTransport(webrtc::IceTransportInterface* ice_transport_interface); !RtcIceTransport(); - webrtc::IceTransportInterface* GetNativeIceTransportInterface(bool throwOnDisposed); + webrtc::scoped_refptr GetNativeIceTransportInterface(bool throwOnDisposed); internal: void FireOnStateChange() { if (on_state_change_ != nullptr) on_state_change_(this, EventArgs::Empty); } diff --git a/WebRtcNet.Api/RtcIceTransport.cs b/WebRtcNet.Api/RtcIceTransport.cs index 317e9a6..6edc0cd 100644 --- a/WebRtcNet.Api/RtcIceTransport.cs +++ b/WebRtcNet.Api/RtcIceTransport.cs @@ -241,14 +241,6 @@ protected RtcIceTransport() { } - /// - /// Returns the native ICE transport interface used by WebRtcInterop. - /// - /// - /// This method is intended for internal use by WebRtcInterop implementations only. - /// - /// True to throw when the transport has already been disposed. - public abstract IntPtr GetNativeIceTransportHandle(bool throwOnDisposed); /// /// The role of this transport. /// From d682e31a950a1ae18947728473f99f935eaa3081 Mon Sep 17 00:00:00 2001 From: Cody Barnes Date: Mon, 8 Jun 2026 23:48:18 -0700 Subject: [PATCH 4/5] Refactor peer and stub GetNative APIs for scoped_refptr BugzId:64 --- WebRtcInterop/RtcPeerConnection.cpp | 4 ++-- WebRtcInterop/RtcPeerConnection.h | 6 ++++-- WebRtcNet.Api/RtcDtlsTransport.cs | 9 --------- WebRtcNet.Api/RtcDtmfSender.cs | 9 --------- WebRtcNet.Api/RtcPeerConnection.cs | 9 --------- WebRtcNet.Api/RtcRtpReceiver.cs | 9 --------- WebRtcNet.Api/RtcRtpSender.cs | 9 --------- WebRtcNet.Api/RtcRtpTransceiver.cs | 9 --------- WebRtcNet.Api/RtcSctpTransport.cs | 9 --------- 9 files changed, 6 insertions(+), 67 deletions(-) diff --git a/WebRtcInterop/RtcPeerConnection.cpp b/WebRtcInterop/RtcPeerConnection.cpp index 87322ef..46074d3 100644 --- a/WebRtcInterop/RtcPeerConnection.cpp +++ b/WebRtcInterop/RtcPeerConnection.cpp @@ -32,12 +32,12 @@ namespace WebRtcInterop memberName)); } - IntPtr RtcPeerConnection::GetNativePeerConnectionHandle(bool throwOnDisposed) + webrtc::scoped_refptr RtcPeerConnection::GetNativePeerConnectionInterface(bool throwOnDisposed) { if (throwOnDisposed && is_closed_) throw gcnew ObjectDisposedException(NAMEOF(RtcPeerConnection)); - return IntPtr::Zero; + return webrtc::scoped_refptr(); } Nullable RtcPeerConnection::LocalDescription::get() { return Nullable(); } diff --git a/WebRtcInterop/RtcPeerConnection.h b/WebRtcInterop/RtcPeerConnection.h index a1e8f44..9f0b9af 100644 --- a/WebRtcInterop/RtcPeerConnection.h +++ b/WebRtcInterop/RtcPeerConnection.h @@ -1,5 +1,7 @@ #pragma once +#include + namespace WebRtcInterop { using namespace System; @@ -90,8 +92,8 @@ namespace WebRtcInterop void RemoveTrack(RtcRtpSender^ sender) override; RtcDataChannel^ CreateDataChannel(String^ label, [System::Runtime::InteropServices::Optional] RtcDataChannelInit^ dataChannelInit) override; - public: - IntPtr GetNativePeerConnectionHandle(bool throwOnDisposed) override; + internal: + webrtc::scoped_refptr GetNativePeerConnectionInterface(bool throwOnDisposed); private: void ThrowShimNotImplemented(String^ memberName); diff --git a/WebRtcNet.Api/RtcDtlsTransport.cs b/WebRtcNet.Api/RtcDtlsTransport.cs index 7a7a9b8..bbdc77a 100644 --- a/WebRtcNet.Api/RtcDtlsTransport.cs +++ b/WebRtcNet.Api/RtcDtlsTransport.cs @@ -68,15 +68,6 @@ protected RtcDtlsTransport() { } - /// - /// Returns the native DTLS transport handle used by WebRtcInterop. - /// - /// - /// This method is intended for internal use by WebRtcInterop implementations only. - /// - /// True to throw when the transport has already been disposed. - public abstract IntPtr GetNativeDtlsTransportHandle(bool throwOnDisposed); - /// /// The IceTransport property is the underlying transport that is used to send and /// receive packets. The underlying transport may not be shared between multiple diff --git a/WebRtcNet.Api/RtcDtmfSender.cs b/WebRtcNet.Api/RtcDtmfSender.cs index 364a1f2..71e9e6d 100644 --- a/WebRtcNet.Api/RtcDtmfSender.cs +++ b/WebRtcNet.Api/RtcDtmfSender.cs @@ -15,15 +15,6 @@ protected RtcDtmfSender() { } - /// - /// Returns the native DTMF sender interface used by WebRtcInterop. - /// - /// - /// This method is intended for internal use by WebRtcInterop implementations only. - /// - /// True to throw when the sender has already been disposed. - public abstract IntPtr GetNativeDtmfSenderHandle(bool throwOnDisposed); - /// /// Indicates if the RTCDTMFSender is capable of sending DTMF. /// diff --git a/WebRtcNet.Api/RtcPeerConnection.cs b/WebRtcNet.Api/RtcPeerConnection.cs index ce6ad4b..ee5f10f 100644 --- a/WebRtcNet.Api/RtcPeerConnection.cs +++ b/WebRtcNet.Api/RtcPeerConnection.cs @@ -179,15 +179,6 @@ protected RtcPeerConnection() { } - /// - /// Returns the native peer connection handle used by WebRtcInterop. - /// - /// - /// This method is intended for internal use by WebRtcInterop implementations only. - /// - /// True to throw when the peer connection has already been disposed. - public abstract IntPtr GetNativePeerConnectionHandle(bool throwOnDisposed); - /// /// The local RTCSessionDescription that was successfully set using /// , plus any local candidates that have been diff --git a/WebRtcNet.Api/RtcRtpReceiver.cs b/WebRtcNet.Api/RtcRtpReceiver.cs index 707b884..2172bcc 100644 --- a/WebRtcNet.Api/RtcRtpReceiver.cs +++ b/WebRtcNet.Api/RtcRtpReceiver.cs @@ -18,15 +18,6 @@ protected RtcRtpReceiver() { } - /// - /// Returns the native RTP receiver interface used by WebRtcInterop. - /// - /// - /// This method is intended for internal use by WebRtcInterop implementations only. - /// - /// True to throw when the receiver has already been disposed. - public abstract IntPtr GetNativeRtpReceiverHandle(bool throwOnDisposed); - /// /// The Track property is the track that is associated with this RTCRtpReceiver /// object receiver. diff --git a/WebRtcNet.Api/RtcRtpSender.cs b/WebRtcNet.Api/RtcRtpSender.cs index 979d0de..14987c7 100644 --- a/WebRtcNet.Api/RtcRtpSender.cs +++ b/WebRtcNet.Api/RtcRtpSender.cs @@ -19,15 +19,6 @@ protected RtcRtpSender() { } - /// - /// Returns the native RTP sender interface used by WebRtcInterop. - /// - /// - /// This method is intended for internal use by WebRtcInterop implementations only. - /// - /// True to throw when the sender has already been disposed. - public abstract IntPtr GetNativeRtpSenderHandle(bool throwOnDisposed); - /// /// The Track property is the track associated with this RTCRtpSender object. If /// the track is ended, or if the track's output is disabled, i.e. the track is diff --git a/WebRtcNet.Api/RtcRtpTransceiver.cs b/WebRtcNet.Api/RtcRtpTransceiver.cs index 6210249..6dd68f7 100644 --- a/WebRtcNet.Api/RtcRtpTransceiver.cs +++ b/WebRtcNet.Api/RtcRtpTransceiver.cs @@ -24,15 +24,6 @@ protected RtcRtpTransceiver() { } - /// - /// Returns the native RTP transceiver interface used by WebRtcInterop. - /// - /// - /// This method is intended for internal use by WebRtcInterop implementations only. - /// - /// True to throw when the transceiver has already been disposed. - public abstract IntPtr GetNativeRtpTransceiverHandle(bool throwOnDisposed); - /// /// The mid-attribute is the media stream "identification-tag" negotiated and /// present in the local and remote descriptions. diff --git a/WebRtcNet.Api/RtcSctpTransport.cs b/WebRtcNet.Api/RtcSctpTransport.cs index 01e9c67..58286c2 100644 --- a/WebRtcNet.Api/RtcSctpTransport.cs +++ b/WebRtcNet.Api/RtcSctpTransport.cs @@ -37,15 +37,6 @@ protected RtcSctpTransport() { } - /// - /// Returns the native SCTP transport interface used by WebRtcInterop. - /// - /// - /// This method is intended for internal use by WebRtcInterop implementations only. - /// - /// True to throw when the transport has already been disposed. - public abstract IntPtr GetNativeSctpTransportHandle(bool throwOnDisposed); - /// /// Gets the underlying DTLS transport, if available. /// From 70b0d1e35d1e6054c1cc4fdd9862e6d9331c7040 Mon Sep 17 00:00:00 2001 From: Cody Barnes Date: Tue, 9 Jun 2026 00:36:50 -0700 Subject: [PATCH 5/5] Document GetNative refactor pattern BugzId:65 --- .github/copilot-instructions.md | 4 +- ...etnative-pattern-internal-scoped-refptr.md | 62 +++++++++++++++++++ docs/adr/README.md | 2 + 3 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 docs/adr/0004-getnative-pattern-internal-scoped-refptr.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 2a7ef20..6b34ead 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -62,7 +62,7 @@ Skip slow stages during iteration: `-SkipToolchain`, `-SkipSync`. - `WebRtcInterop.Core.vcxproj` — targets .NET Core (`CLRSupport=NetCore`, experimental) - Both share source via `WebRtcInterop.Shared.vcxitems` - `RtcPeerConnectionFactory` owns the native `PeerConnectionFactoryInterface` singleton and the signaling thread. -- `RtcPeerConnection`, `RtcDataChannel`, `MediaStream`, and related wrappers hold native `scoped_refptr` handles via the `NativeWrapper` template base class (`NativeWrapper.h`). +- `RtcPeerConnection`, `RtcDataChannel`, `MediaStream`, and related wrappers keep internal `GetNative*` helpers inside WebRtcInterop and return `scoped_refptr` by value; native ownership is handled by `NativeWrapper` composition where needed (`NativeWrapper.h`, ADR-0004). - `WebRtcInterop\Marshaling\` contains all `marshal_as<>` specializations for converting between managed DTOs/enums and native WebRTC types. Cross-boundary value translation belongs here, not inline in business logic. - `WebRtcInterop\Observers\` adapts native callbacks into managed events via observer classes that call `FireOn...` helpers on the wrapper types. - `WebRtcInterop\MediaStream.cpp` contains the concrete `Media::GetUserMedia` implementation; device enumeration and native audio/video source creation happen here, not in C#. @@ -91,7 +91,7 @@ Individual-stage Dockerfiles in `docker\` — not a monolithic file: - `WebRtcNet.Api` is the contract assembly. If you add API surface there, you also need matching wrapper, marshalling, and observer work in `WebRtcInterop`. - Do not assume every interface member is implemented. Many interop methods still throw `NotImplementedException`, especially around stream enumeration, stats, identity, and parts of peer connection setup. - Managed argument validation uses `System.Diagnostics.Contracts.Contract.Requires(...)` rather than ad hoc null checks. -- Interop wrapper classes follow the same lifetime pattern: destructor/finalizer pair, and a `GetNative...(throwOnDisposed)` helper that throws `ObjectDisposedException` when the native handle is gone. +- Interop wrapper classes follow the same lifetime pattern: destructor/finalizer pair, and an internal `GetNative...(throwOnDisposed)` helper that returns `scoped_refptr` and throws `ObjectDisposedException` when the native handle is gone. - Some interop methods require concrete wrapper instances, not arbitrary interface implementations. Peer connection stream operations `dynamic_cast` `IMediaStream` to `WebRtcInterop::MediaStream` before unwrapping the native object. - `marshal_as<>` specializations in `Marshaling\` use either switch-based dispatch (simple enums) or bidirectional `std::map` helpers (`marshal_mapped_native_type` / `marshal_mapped_managed_type` in `MarshalEnums.h`) for types where both directions are needed. - The solution uses standard `Debug` and `Release` configurations; use these directly for normal development and CI. diff --git a/docs/adr/0004-getnative-pattern-internal-scoped-refptr.md b/docs/adr/0004-getnative-pattern-internal-scoped-refptr.md new file mode 100644 index 0000000..5340889 --- /dev/null +++ b/docs/adr/0004-getnative-pattern-internal-scoped-refptr.md @@ -0,0 +1,62 @@ +# GetNative helpers stay internal and return scoped_refptr + +## Status + +Accepted + +## Date + +2026-06-09 + +## Context + +Issue #61 through #64 refactor the WebRtcInterop boundary so the managed API no longer exposes `GetNative*Handle(bool)` methods and the interop layer no longer round-trips native pointers through `IntPtr`. + +The old pattern forced each wrapper to expose a public contract method that returned `IntPtr`, then immediately re-cast that pointer back into native WebRTC types inside C++/CLI. That added unnecessary marshaling and made the native lifetime contract harder to reason about. + +## Decision + +WebRtcNet will keep `GetNative*` helpers as **internal interop-only members** and have them return `webrtc::scoped_refptr` by value. + +The public `WebRtcNet.Api` contract types no longer define these helpers. Interop types keep the same `throwOnDisposed` behavior, but the native return path now preserves reference counting directly instead of exposing raw pointers or `IntPtr`. + +Native ownership for interop wrappers is handled with `NativeWrapper` composition where needed, so the wrapper can hold and dispose the native reference-counted object safely. + +## Consequences + +### Positive + +- Removes `IntPtr` marshaling for internal native access. +- Preserves reference counting across the managed/native boundary. +- Keeps `throwOnDisposed` semantics intact for existing callers. +- Makes the native access path easier to follow in interop code. + +### Negative + +- Requires a separate internal/native access helper for each wrapper type. +- Adds one more layer of abstraction between the managed API and native implementation. +- `NativeWrapper` and the interop wrappers must stay aligned so ownership remains explicit. + +## Alternatives considered + +1. **Keep returning `IntPtr` from public API methods** + Rejected because it keeps the unnecessary pointer round-trip and exposes internal native mechanics to the public contract. + +2. **Use public wrapper inheritance for ownership** + Rejected because the interop types already inherit the managed API base classes and need a single inheritance chain. + +3. **Use composition only, without `GetNative*` helpers** + Rejected because the interop layer still needs a direct native access path for implementation details. + +## References + +- Issue #61 — Refactor Media types: scoped_refptr GetNative* pattern +- Issue #62 — Refactor RtcDataChannel: scoped_refptr GetNative* pattern +- Issue #63 — Refactor RtcIceTransport: scoped_refptr GetNative* pattern +- Issue #64 — Refactor RtcPeerConnection and stub types: scoped_refptr GetNative* pattern +- Issue #65 — Document GetNative* refactor: create ADR and update documentation +- `WebRtcInterop\Media\MediaStream.cpp` +- `WebRtcInterop\Media\MediaStreamTrack.cpp` +- `WebRtcInterop\RtcDataChannel.cpp` +- `WebRtcInterop\RtcIceTransport.cpp` +- `WebRtcInterop\RtcPeerConnection.cpp` diff --git a/docs/adr/README.md b/docs/adr/README.md index 03bf4a0..3fd9bb2 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -8,6 +8,8 @@ This folder stores architectural decisions that affect implementation direction, | --- | --- | --- | | [0001](0001-host-entry-point-for-api-access.md) | Host entry point for API access | accepted | | [0002](0002-caller-callee-tcp-alignment.md) | Caller/Callee TCP alignment | accepted | +| [0003](0003-logging-infrastructure.md) | Logging infrastructure using Microsoft.Extensions.Logging | accepted | +| [0004](0004-getnative-pattern-internal-scoped-refptr.md) | GetNative helpers stay internal and return scoped_refptr | accepted | ## ADR format (required)