Fix data channel error events and bufferedAmount, copy messages once

python-webrtc/cpp/NATIVE.md

@@ -102,7 +102,7 @@ One direction: Python → C++ → libwebrtc.
   unregistered is ended by `Disarm()` at the start of the destructor, which then waits for the callback's thread once.
 - Raw pointers never own: they point to libwebrtc observers and callbacks, or into C APIs. A raw pointer libwebrtc
   hands over to own (`OnSuccess(SessionDescriptionInterface *)`) goes into a `std::unique_ptr` right away.
-- An event's arguments are native values copied at `Emit`; they're converted to Python on the loop thread.
+- An event's arguments are native values copied or moved at `Emit`; they're converted to Python on the loop thread.
 - Registries and the Dispatcher are leaked on purpose (`static auto *x = new ...`, `ForkLocal`): native threads can
   outlive static destructors at exit.
 

python-webrtc/cpp/src/interfaces/peer_connection_factory.cpp

@@ -17,7 +17,6 @@
 #include "../utils/port_allocator.h"
 #include "../utils/task_queue_factory.h"
 
-#include <api/audio/builtin_audio_processing_builder.h>
 #include <api/audio_codecs/builtin_audio_decoder_factory.h>
 #include <api/audio_codecs/builtin_audio_encoder_factory.h>
 #include <api/create_modular_peer_connection_factory.h>
@@ -84,7 +83,7 @@ namespace python_webrtc {
     }
   } // namespace
 
-  // webrtc::CreatePeerConnectionFactory without the RTC event log, which keeps every packet's buffer in memory
+  // webrtc::CreatePeerConnectionFactory without the RTC event log (holds every packet) or audio processing (no capture)
   webrtc::PeerConnectionFactoryDependencies PeerConnectionFactory::Dependencies() {
     webrtc::PeerConnectionFactoryDependencies dependencies;
     dependencies.network_thread = _workerThread.get();
@@ -95,7 +94,6 @@ namespace python_webrtc {
     dependencies.adm = _audioDeviceModule;
     dependencies.audio_encoder_factory = webrtc::CreateBuiltinAudioEncoderFactory();
     dependencies.audio_decoder_factory = webrtc::CreateBuiltinAudioDecoderFactory();
-    dependencies.audio_processing_builder = std::make_unique<webrtc::BuiltinAudioProcessingBuilder>();
     dependencies.video_encoder_factory = std::make_unique<VideoEncoderFactory>();
     dependencies.video_decoder_factory = std::make_unique<VideoDecoderFactory>();
     webrtc::EnableMedia(dependencies);
@@ -117,9 +115,8 @@ namespace python_webrtc {
 
   PeerConnectionFactory::PeerConnectionFactory()
       : _fieldTrials(_trials), _networkIgnoreMask(_loopbackAllowed ? 0 : webrtc::kDefaultNetworkIgnoreMask) {
-    _workerThread = Started(std::make_unique<webrtc::Thread>(std::make_unique<NetworkSocketServer>()),
-                            "PeerConnectionFactory:workerThread");
-    _signalingThread = Started(webrtc::Thread::Create(), "PeerConnectionFactory:signalingThread");
+    _workerThread = Started(std::make_unique<webrtc::Thread>(std::make_unique<NetworkSocketServer>()), "wrtc-network");
+    _signalingThread = Started(webrtc::Thread::Create(), "wrtc-signaling");
     // sanitizer builds check that these threads never enter Python
     _workerThread->PostTask(&TagNativeThread);
     _signalingThread->PostTask(&TagNativeThread);

python-webrtc/cpp/src/interfaces/rtc_data_channel.cpp

@@ -14,6 +14,7 @@
 #include "../exceptions.h"
 #include "../utils/gil.h"
 #include "../utils/libwebrtc_thread.h"
+#include "../utils/parking.h"
 
 namespace python_webrtc {
 
@@ -61,12 +62,13 @@ namespace python_webrtc {
   void RTCDataChannel::Init(pybind11::module &m) {
     pybind11::class_<DataChannelMessage>(m, "DataChannelMessage")
         .def_property_readonly("data", [](const DataChannelMessage &message) -> pybind11::object {
+          const auto *data = message.data.cdata<char>();
           if (message.binary) {
-            return pybind11::bytes(message.data);
+            return pybind11::bytes(data, message.data.size());
           }
           // the upper layers verify UTF-8, as libwebrtc doesn't
           return pybind11::reinterpret_steal<pybind11::object>(
-              PyUnicode_DecodeUTF8(message.data.data(), static_cast<Py_ssize_t>(message.data.size()), "replace"));
+              PyUnicode_DecodeUTF8(data, static_cast<Py_ssize_t>(message.data.size()), "replace"));
         });
 
     DefineBinding(pybind11::class_<RTCDataChannel, Binding, std::shared_ptr<RTCDataChannel>>(m, "RTCDataChannel"))
@@ -84,7 +86,15 @@ namespace python_webrtc {
         .def_property("bufferedAmountLowThreshold", nogil_fn(&RTCDataChannel::GetBufferedAmountLowThreshold),
                       nogil_fn(&RTCDataChannel::SetBufferedAmountLowThreshold))
         .def_property("binaryType", nogil_fn(&RTCDataChannel::GetBinaryType), nogil_fn(&RTCDataChannel::SetBinaryType))
-        .def("send", &RTCDataChannel::Send, nogil(), pybind11::arg("data"), pybind11::arg("binary"))
+        .def(
+            "send",
+            // only bytes: the view must outlive the call without the GIL, which a bytearray's resize breaks
+            [](RTCDataChannel &self, const pybind11::bytes &data, bool binary) {
+              const std::string_view view = data;
+              const gil_release release;
+              return self.Send(view, binary);
+            },
+            pybind11::arg("data"), pybind11::arg("binary"))
         .def("close", &RTCDataChannel::Close, nogil())
         .def("_surfaceState", &RTCDataChannel::SurfaceState, nogil(), pybind11::arg("state"))
         .def("_decreaseBufferedAmount", &RTCDataChannel::DecreaseBufferedAmount, nogil(), pybind11::arg("sent"));
@@ -116,17 +126,22 @@ namespace python_webrtc {
       Emit("open", state);
       break;
     case DataState::kClosing:
+      Parking::At("datachannel.closing");
       Emit("closing", state);
       break;
     case DataState::kClosed: {
       auto error = _channel->error();
-      // closing locally isn't an error, even if queued messages couldn't be sent
       bool closedLocally = false;
       {
         const std::scoped_lock lock(_closeMutex);
         closedLocally = _closeRequested;
       }
-      if (!error.ok() && !closedLocally) {
+      const bool detailed = error.error_detail() != webrtc::RTCErrorDetailType::NONE;
+      // closing locally isn't an error
+      if (!error.ok() && (detailed || !closedLocally)) {
+        if (!detailed) {
+          error.set_error_detail(webrtc::RTCErrorDetailType::DATA_CHANNEL_FAILURE);
+        }
         Emit("error", RTCCallbackException(std::move(error)));
       }
       Emit("close", state);
@@ -141,8 +156,7 @@ namespace python_webrtc {
   }
 
   void RTCDataChannel::OnMessage(const webrtc::DataBuffer &buffer) {
-    Emit("message",
-         DataChannelMessage{.data = std::string(buffer.data.cdata<char>(), buffer.size()), .binary = buffer.binary});
+    Emit("message", DataChannelMessage{.data = buffer.data, .binary = buffer.binary});
   }
 
   void RTCDataChannel::OnBufferedAmountChange(uint64_t sentDataSize) {
@@ -204,7 +218,7 @@ namespace python_webrtc {
   }
 
   webrtc::Priority RTCDataChannel::GetPriority() {
-    // the ranges Chromium maps priority values to
+    // splits at the midpoints between the DCEP priority values of W3C webrtc-priority (128, 256, 512, 1024)
     constexpr int veryLowMax = 192;
     constexpr int lowMax = 384;
     constexpr int mediumMax = 768;
@@ -277,7 +291,7 @@ namespace python_webrtc {
 
   } // namespace
 
-  bool RTCDataChannel::Send(const std::string &data, bool binary) {
+  bool RTCDataChannel::Send(std::string_view data, bool binary) {
     if (GetReadyState() != DataState::kOpen) {
       throw RTCException(webrtc::RTCErrorType::INVALID_STATE, "RTCDataChannel.readyState is not 'open'");
     }
@@ -307,14 +321,8 @@ namespace python_webrtc {
   }
 
   void RTCDataChannel::SendNow(webrtc::DataBuffer buffer) {
-    auto size = buffer.size();
-    // posted to the network thread, in call order; a failed send closes the channel
-    _channel->SendAsync(std::move(buffer), [weak = weak_from_this(), size](const webrtc::RTCError &error) {
-      auto self = weak.lock();
-      if (!error.ok() && self) {
-        self->DecreaseBufferedAmount(size);
-      }
-    });
+    // posted in call order; libwebrtc reports a failed send's bytes through OnBufferedAmountChange too
+    _channel->SendAsync(std::move(buffer), nullptr);
   }
 
   void RTCDataChannel::SetStateLocked(DataState state) {

python-webrtc/cpp/src/interfaces/rtc_data_channel.h

@@ -15,8 +15,10 @@
 #include <mutex>
 #include <optional>
 #include <string>
+#include <string_view>
 
 #include <api/data_channel_interface.h>
+#include <rtc_base/copy_on_write_buffer.h>
 
 #include <pybind11/pybind11.h>
 
@@ -32,7 +34,8 @@ namespace python_webrtc {
 
   // A received message, converted to str or bytes by Python
   struct DataChannelMessage {
-    std::string data;
+    // refcounted and only read, so it's shared across threads without a copy
+    webrtc::CopyOnWriteBuffer data;
     bool binary;
   };
 
@@ -113,8 +116,8 @@ namespace python_webrtc {
     // whether bufferedAmount dropped to the threshold
     bool DecreaseBufferedAmount(uint64_t sent);
 
-    // whether the message is the only one queued: the channel's activity changes only then
-    bool Send(const std::string &data, bool binary);
+    // whether the message is the only one queued: the channel's activity changes only then; data is copied
+    bool Send(std::string_view data, bool binary);
 
     void Close();
 

python-webrtc/cpp/src/interfaces/rtc_peer_connection/rtc_peer_connection.cpp

@@ -32,6 +32,8 @@ namespace python_webrtc {
     auto configuration = webrtc::PeerConnectionInterface::RTCConfiguration();
     configuration.sdp_semantics = webrtc::SdpSemantics::kUnifiedPlan;
     configuration = _configuration.Apply(configuration);
+    // renders each decoded frame at once, without a smoothing queue per video receiver
+    configuration.set_prerenderer_smoothing(false);
 
     webrtc::PeerConnectionDependencies dependencies(this);
     dependencies.allocator = _factory->CreatePortAllocator(configuration);

python-webrtc/cpp/src/interfaces/rtc_rtp_sender.cpp

@@ -22,7 +22,7 @@ namespace python_webrtc {
 
   namespace {
 
-    // only what setParameters may change, as in Chromium
+    // only the encoding members setParameters may change, the rest keep their current values
     void applySettable(const webrtc::RtpEncodingParameters &requested, webrtc::RtpEncodingParameters &current) {
       current.active = requested.active;
       current.request_key_frame = requested.request_key_frame;

python-webrtc/cpp/src/utils/mailbox.h

@@ -130,8 +130,9 @@ namespace python_webrtc {
   template <typename T>
   class Emitter : public Binding {
   protected:
+    // an rvalue argument is moved into the record, and from it into Python
     template <typename... Args>
-    void Emit(const char *name, const Args &...args) {
+    void Emit(const char *name, Args &&...args) {
       CheckNativeThreadDetached("Emitter::Emit");
       if (!IsBound()) {
         return;
@@ -141,8 +142,13 @@ namespace python_webrtc {
       if (!self) {
         return;
       }
-      Post(Record{.source = self, .target = self->Id(), .name = name, .args = [self, args...]() {
-                    return pybind11::make_tuple(self, args...);
+      // Take converts a record once
+      Post(Record{.source = self,
+                  .target = self->Id(),
+                  .name = name,
+                  // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks): the analyzer loses a moved shared_ptr
+                  .args = [self, ... args = std::forward<Args>(args)]() mutable {
+                    return pybind11::make_tuple(self, std::move(args)...);
                   }});
     }
 

python-webrtc/python/webrtc/exceptions.py

@@ -212,7 +212,7 @@ def __init__(self, init: RTCErrorInit, message: str = '') -> None:
 
 
 _BY_RTC_ERROR_TYPE = {
-    # the same mapping as Chromium's
+    # the W3C error names for libwebrtc's RTCErrorType values
     'UNSUPPORTED_OPERATION': OperationError,
     'UNSUPPORTED_PARAMETER': InvalidAccessError,
     'INVALID_PARAMETER': InvalidAccessError,

python-webrtc/python/webrtc/interfaces/rtc_data_channel.py

@@ -23,7 +23,7 @@
 from webrtc.models.events import Event, MessageEvent, RTCErrorEvent, RTCErrorEventInit
 from webrtc.utils.events import AnyHandler, EventTarget, HandlerDecorator
 from webrtc.utils.names import Alias, alias
-from webrtc.utils.strings import usv_string
+from webrtc.utils.strings import usv_utf8
 
 if TYPE_CHECKING:
     import webrtc
@@ -43,7 +43,7 @@ def check_utf8_length(name: str, value: str) -> None:
     Raises:
         TypeError: If it's longer.
     """
-    if len(usv_string(value).encode()) > MAX_UNSIGNED_SHORT:
+    if len(usv_utf8(value)) > MAX_UNSIGNED_SHORT:
         msg = f'{name} is longer than {MAX_UNSIGNED_SHORT} bytes'
         raise TypeError(msg)
 
@@ -399,7 +399,7 @@ def send(self, data: str | bytes | bytearray | memoryview | Blob) -> None:
             webrtc.OperationError: If the send queue is full.
         """
         if isinstance(data, str):
-            message, binary = usv_string(data).encode(), False
+            message, binary = usv_utf8(data), False
         elif isinstance(data, (bytes, bytearray, memoryview, Blob)):
             message, binary = bytes(data), True
         else:

python-webrtc/python/webrtc/models/rtp_parameters.py

@@ -31,7 +31,7 @@
 
 _NativeCodecT = TypeVar('_NativeCodecT', bound='wrtc.RtpCodec')
 
-# the bitrate priorities of libwebrtc for RTCRtpEncodingParameters.priority, as Chromium maps them
+# libwebrtc's bitrate priorities for RTCRtpEncodingParameters.priority, doubling per level from low (the default) at 1.0
 _BITRATE_PRIORITY = {
     RTCPriorityType.very_low: 0.5,
     RTCPriorityType.low: 1.0,

python-webrtc/python/webrtc/utils/strings.py

@@ -17,3 +17,11 @@ def usv_string(value: str) -> str:
     except UnicodeEncodeError:
         return value.encode('utf-16-le', 'surrogatepass').decode('utf-16-le', 'replace')
     return value
+
+
+def usv_utf8(value: str) -> bytes:
+    """Encodes a USVString in UTF-8, encoding only once unless there are lone surrogates."""
+    try:
+        return value.encode()
+    except UnicodeEncodeError:
+        return usv_string(value).encode()

stubs/wrtc/__init__.pyi

@@ -644,7 +644,7 @@ class RTCDataChannel(Binding):
         ...
     def close(self) -> None:
         ...
-    def send(self, data: str | bytes, binary: bool) -> bool:
+    def send(self, data: bytes, binary: bool) -> bool:
         ...
     @property
     def bufferedAmount(self) -> int: