Expose RtpReceiver::SetJitterBufferMinimumDelay on Obj-C and Java - #235
Expose RtpReceiver::SetJitterBufferMinimumDelay on Obj-C and Java#235doracawl wants to merge 5 commits into
Conversation
The C++ RtpReceiverInterface has supported SetJitterBufferMinimumDelay
since long ago (it backs the W3C jitterBufferTarget attribute), but
neither the Java nor the Objective-C API surfaced it. Apps building on
the prebuilt SDKs therefore had no way to trade latency for smoothness
on weak networks without dropping out to a custom build.
This change adds:
* Java: RtpReceiver#setJitterBufferMinimumDelay(double) and
RtpReceiver#clearJitterBufferMinimumDelay(), with matching
nativeSetJitterBufferMinimumDelay / nativeClearJitterBufferMinimumDelay
JNI wiring in sdk/android/src/jni/pc/rtp_receiver.cc.
* ObjC: -[RTCRtpReceiver setJitterBufferMinimumDelay:] taking a
nullable NSNumber*, where nil restores default behaviour.
Both layers forward straight to the existing C++ entry point with
std::optional<double>, mirroring how the browser exposes the property.
There was a problem hiding this comment.
Pull request overview
This PR exposes the existing C++ RtpReceiverInterface::SetJitterBufferMinimumDelay(std::optional<double>) API to mobile wrapper layers so iOS (Obj-C) and Android (Java) apps using the prebuilt SDKs can tune receive-side jitter buffering without a custom build.
Changes:
- Obj-C: add
-[RTCRtpReceiver setJitterBufferMinimumDelay:]acceptingnullable NSNumber*(nilclears to default). - Java: add
RtpReceiver.setJitterBufferMinimumDelay(double)andRtpReceiver.clearJitterBufferMinimumDelay(). - JNI: add bindings that forward the Java calls into
RtpReceiverInterface::SetJitterBufferMinimumDelay.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| sdk/objc/api/peerconnection/RTCRtpReceiver.h | Adds Obj-C API surface for setting/clearing the receiver jitter buffer minimum delay. |
| sdk/objc/api/peerconnection/RTCRtpReceiver.mm | Implements the Obj-C method by forwarding into the native RtpReceiverInterface. |
| sdk/android/api/org/webrtc/RtpReceiver.java | Adds public Java methods and corresponding native declarations. |
| sdk/android/src/jni/pc/rtp_receiver.cc | Implements JNI entry points that call the native C++ receiver API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - (void)setJitterBufferMinimumDelay:(nullable NSNumber *)delaySeconds { | ||
| if (delaySeconds == nil) { | ||
| _nativeRtpReceiver->SetJitterBufferMinimumDelay(std::nullopt); | ||
| } else { | ||
| _nativeRtpReceiver->SetJitterBufferMinimumDelay( | ||
| std::optional<double>(delaySeconds.doubleValue)); | ||
| } | ||
| } |
| static void JNI_RtpReceiver_SetJitterBufferMinimumDelay( | ||
| JNIEnv* jni, | ||
| jlong j_rtp_receiver_pointer, | ||
| jdouble delay_seconds) { | ||
| reinterpret_cast<RtpReceiverInterface*>(j_rtp_receiver_pointer) | ||
| ->SetJitterBufferMinimumDelay(std::optional<double>(delay_seconds)); | ||
| } |
| public void setJitterBufferMinimumDelay(double delaySeconds) { | ||
| checkRtpReceiverExists(); | ||
| nativeSetJitterBufferMinimumDelay(nativeRtpReceiver, delaySeconds); | ||
| } |
hiroshihorie
left a comment
hiroshihorie
left a comment
There was a problem hiding this comment.
Thanks for the contribution! The implementation looks great, both sides forward the optional exactly like the C++ API expects and the Java setter/clear split keeps the hot path allocation free, nice touch. The guard and naming also match the existing patterns in these wrappers.
I have one small suggestion about the docs, see inline. It is about units, the only thing I would really want fixed before merge.
| /** | ||
| * Sets the minimum jitter buffer delay (in seconds) for this receiver. | ||
| * The receiver keeps at least this much media buffered before playout, | ||
| * trading latency for smoothness on weak networks. Wraps the C++ | ||
| * RtpReceiverInterface::SetJitterBufferMinimumDelay. | ||
| */ |
There was a problem hiding this comment.
One small thing on the docs. jitterBufferTarget on the browser side is in milliseconds while this method takes seconds, and I can easily see someone porting browser code getting bitten by a 1000x mistake. A concrete example in the doc should prevent that, and the internal clamp is worth mentioning too since the native side silently caps at 10 seconds.
| /** | |
| * Sets the minimum jitter buffer delay (in seconds) for this receiver. | |
| * The receiver keeps at least this much media buffered before playout, | |
| * trading latency for smoothness on weak networks. Wraps the C++ | |
| * RtpReceiverInterface::SetJitterBufferMinimumDelay. | |
| */ | |
| /** | |
| * Sets the minimum jitter buffer delay in seconds for this receiver. | |
| * The receiver keeps at least this much media buffered before playout, | |
| * trading latency for smoothness on weak networks. Note the unit: the | |
| * equivalent browser attribute, RTCRtpReceiver.jitterBufferTarget, is | |
| * expressed in milliseconds, so a browser value of 500 corresponds to | |
| * 0.5 here. Values are clamped to [0, 10] seconds internally. Wraps the | |
| * C++ RtpReceiverInterface::SetJitterBufferMinimumDelay. | |
| */ |
| /** Sets the minimum jitter buffer delay (in seconds) for this receiver. | ||
| * Pass nil to restore the default behaviour. The receiver keeps at least | ||
| * this much media buffered before playout, trading latency for smoothness | ||
| * on weak networks. Mirrors RTCRtpReceiver.jitterBufferTarget in browser | ||
| * WebRTC; wraps the C++ RtpReceiverInterface::SetJitterBufferMinimumDelay. | ||
| */ |
There was a problem hiding this comment.
Same units note for the ObjC side. Mentioning jitterBufferTarget is great for discoverability, just worth being explicit that the units differ.
| /** Sets the minimum jitter buffer delay (in seconds) for this receiver. | |
| * Pass nil to restore the default behaviour. The receiver keeps at least | |
| * this much media buffered before playout, trading latency for smoothness | |
| * on weak networks. Mirrors RTCRtpReceiver.jitterBufferTarget in browser | |
| * WebRTC; wraps the C++ RtpReceiverInterface::SetJitterBufferMinimumDelay. | |
| */ | |
| /** Sets the minimum jitter buffer delay in seconds for this receiver. | |
| * Pass nil to restore the default behaviour. The receiver keeps at least | |
| * this much media buffered before playout, trading latency for smoothness | |
| * on weak networks. Note the unit: the equivalent browser attribute, | |
| * RTCRtpReceiver.jitterBufferTarget, is expressed in milliseconds, so a | |
| * browser value of 500 corresponds to 0.5 here. Values are clamped to | |
| * [0, 10] seconds internally. Wraps the C++ | |
| * RtpReceiverInterface::SetJitterBufferMinimumDelay. | |
| */ |
|
Thanks for the review and the units catch — the ms-vs-seconds gap against While there, I aligned the Java surface with this repo's own convention for optional numeric setters — I also adopted the Copilot note on non-finite input — it's a genuine crash path ( |
hiroshihorie
left a comment
hiroshihorie
left a comment
There was a problem hiding this comment.
Re-reviewed the latest revision. The units docs are applied on both sides and match the actual behavior in pc/jitter_buffer_delay.cc, including the [0, 10] s clamp. The NaN guard is correct. saturated_cast hits RTC_CHECK_NOTREACHED() on NaN, and the layering (Java throws, JNI and Obj-C map non-finite to nullopt) is sensible. Threading is safe since SetJitterBufferMinimumDelay is proxied to the worker thread, and the JNI naming matches the generated header convention.
One optional nit, not blocking: Java throws on non-finite input while Obj-C silently resets to default, and the Obj-C header doc does not mention that. A one-line addition like "non-finite values are treated as nil" would make it fully self-documenting. Fine to fix in a follow-up or leave as is.
Thanks for addressing the feedback.
|
Added the one-liner to the Obj-C header doc — non-finite values are now documented as being treated as nil (88c4bdb). Thanks for the review! |
Summary
The C++
RtpReceiverInterface::SetJitterBufferMinimumDelayhas been part of the WebRTC public API for a long time (it backs the W3CRTCRtpReceiver.jitterBufferTargetattribute), but neither the Obj-C nor Java wrappers in this fork expose it. Apps building on the prebuilt SDKs therefore have no way to control receive-side jitter buffer minimum delay — i.e. trade latency for smoothness on weak networks — without dropping out to a custom build.This PR adds:
-[RTCRtpReceiver setJitterBufferMinimumDelay:]taking anullable NSNumber *, wherenilrestores default behaviour.RtpReceiver.setJitterBufferMinimumDelay(@Nullable Double), wherenullrestores default behaviour.Both forward straight to the existing C++ entry point with
std::optional<double>, mirroring how the browser exposes the property. The public setter rejects non-finite input; the native boundaries (JNI / Obj-C) defensively treat a non-finite value as "clear" — see Design notes.Files changed
sdk/objc/api/peerconnection/RTCRtpReceiver.h— protocol declarationsdk/objc/api/peerconnection/RTCRtpReceiver.mm— implementationsdk/android/api/org/webrtc/RtpReceiver.java— public Java API +nativedeclssdk/android/src/jni/pc/rtp_receiver.cc— JNI bindingsAdditive only; no behavioural change to existing APIs. The C++ symbol is unchanged and the change is binary-compatible with the current ABI.
Why this matters downstream
We are building a multi-tenant IoT/video-surveillance mobile app on the prebuilt SDKs from this fork (m144). On weak networks our users see frequent video stutter that a 200–400 ms minimum jitter buffer cleanly absorbs. The C++ method is exactly what we need; without this binding we would have to carry a private SDK build forever. Exposing it once here removes that burden.
Design notes
Doubleon Java? It matches this repo's existing convention for optional numeric setters (EncodedImage.setQp(@Nullable Integer), the@Nullable Integerparameters onVideoSource), keeps the Java surface symmetric with the Obj-Cnullable NSNumber *, and maps directly onto the C++std::optional<double>(null/nil→std::nullopt).std::optional? m144 already usesstd::optionalthroughoutapi/rtp_receiver_interface.h, so we match the existing surface.RTC_CHECKinJitterBufferDelay::GetMs(saturated_cast<int>ofdelay * 1000). The Java setter rejects NaN/Inf withIllegalArgumentException; the JNI and Obj-C boundaries treat non-finite asstd::nulloptso a caller reaching native directly cannot crash the engine. Validation usesDouble.isNaN/isInfiniterather than the API 24+Double.isFinite, matching this module'smin_sdk_versionof 23.Testing
RtpReceiver_jni.hderivesJNI_RtpReceiver_<MethodName>from the JavanativeXxxdeclarations).RTCInboundRtpStreamStats.jitterBufferDelayif useful.