Add audio jitter options to RTCConfiguration #231
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds three audio jitter buffer configuration options to RTCConfiguration to provide finer control over audio latency and buffering behavior in WebRTC peer connections.
- Added
audioJitterBufferMaxPackets,audioJitterBufferFastAccelerate, andaudioJitterBufferMinDelayMsfields - Implemented JNI bindings to bridge Java configuration to native WebRTC configuration
- Defined constant
kAudioJitterBufferMaxPacketswith default value of 200
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| webrtc/src/main/java/dev/onvoid/webrtc/RTCConfiguration.java | Added three public fields for audio jitter buffer configuration with documentation and a constant for the default max packets value |
| webrtc-jni/src/main/cpp/src/api/RTCConfiguration.cpp | Implemented bidirectional conversion between Java and native types for the new audio jitter buffer fields |
| webrtc-jni/src/main/cpp/include/api/RTCConfiguration.h | Declared jfieldID references for the three new audio jitter buffer fields |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** Whether to use the NetEq "fast mode" which will accelerate audio quicker | ||
| * if it falls behind. |
There was a problem hiding this comment.
The JavaDoc comment formatting is inconsistent with the other comments in this file. The opening '/**' should be on its own line, and there should be no leading tabs before the asterisks. This comment should follow the same style as the comments above and below it.
| /** Whether to use the NetEq "fast mode" which will accelerate audio quicker | |
| * if it falls behind. | |
| /** | |
| * Whether to use the NetEq "fast mode" which will accelerate audio quicker | |
| * if it falls behind. |
| env->SetObjectField(config, javaClass->portAllocatorConfig, pac.get()); | ||
|
|
||
| env->SetIntField(config, javaClass->audioJitterBufferMaxPackets, nativeType.audio_jitter_buffer_max_packets); | ||
| env->SetBooleanField(config, javaClass->audioJitterBufferMaxPackets, nativeType.audio_jitter_buffer_fast_accelerate); |
There was a problem hiding this comment.
Incorrect field is being used for the boolean value. The field 'audioJitterBufferMaxPackets' is being set twice (once on line 63 as an int, and again on line 64 as a boolean), while 'audioJitterBufferFastAccelerate' is never set. This should use 'audioJitterBufferFastAccelerate' instead of 'audioJitterBufferMaxPackets'.
| env->SetBooleanField(config, javaClass->audioJitterBufferMaxPackets, nativeType.audio_jitter_buffer_fast_accelerate); | |
| env->SetBooleanField(config, javaClass->audioJitterBufferFastAccelerate, nativeType.audio_jitter_buffer_fast_accelerate); |
| /** | ||
| * The maximum number of packets that can be stored in the NetEq audio | ||
| * jitter buffer. Can be reduced to lower tolerated audio latency. | ||
| */ | ||
| public int audioJitterBufferMaxPackets = kAudioJitterBufferMaxPackets; | ||
|
|
||
| /** Whether to use the NetEq "fast mode" which will accelerate audio quicker | ||
| * if it falls behind. | ||
| */ | ||
| public boolean audioJitterBufferFastAccelerate; | ||
|
|
||
| /** | ||
| * The minimum delay in milliseconds for the audio jitter buffer. | ||
| */ | ||
| public int audioJitterBufferMinDelayMs; |
There was a problem hiding this comment.
The new audio jitter buffer fields lack test coverage. Based on the existing test pattern in RTCPeerConnectionTests.configuration(), these fields should be tested to verify they are properly round-tripped through the JNI layer (set in Java configuration, create peer connection, retrieve configuration, and verify values match).
1 participant
Add audio jitter options below:
int audioJitterBufferMaxPackets
boolean audioJitterBufferFastAccelerate
int audioJitterBufferMinDelayMs