WebRTC Client. JVM implementation. #5122
Conversation
WalkthroughAdds comprehensive JVM WebRTC support by implementing native bridge types (JvmWebRtcConnection, JvmWebRtcEngine, media track wrappers), introducing mock test factories, removing desktop/Android test utilities, updating platform target configurations, and adjusting test annotations across multiple platforms. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The diff spans 40+ files with heterogeneous changes: extensive new JVM implementation logic (bridging native types, async handling, lifecycle management), removal of Android/desktop platforms, test infrastructure additions, and API surface restructuring. While many changes follow consistent patterns (e.g., repeated getNative extensions, annotation removals), the core WebRTC integration involves dense logic requiring careful verification of type mappings, coroutine correctness, exception handling, and platform compatibility. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (28)
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ticker.kt (2)
19-28: Consider adding exception documentation.As per coding guidelines, public APIs should document exceptions. The
initblock (line 43) throwsIllegalArgumentExceptionif the interval is not positive, which should be documented with a@throwstag.Apply this diff to add the exception documentation:
* @param interval The duration between tick executions (must be positive) * @param onTick The suspend function to execute on each tick + * @throws IllegalArgumentException if the interval is not positive */
33-40: Consider the naming ofisActivefor clarity.The
isActiveproperty name matches the coroutine extension propertyisActiveused within thestart()method (lines 56, 59). Inside the coroutine launched bystart(), references toisActiverefer to the coroutine's active state, not this Ticker's property. While this works correctly, it could be confusing for maintainers.For improved clarity, consider either:
- Renaming this property (e.g.,
isRunningorisStarted), or- Adding a comment explaining that the coroutine's
isActiveis intentionally used instart()for cancellation supportktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmUtils.kt (3)
16-19: Harden IceServer.urls parsingTrim and ignore blanks; support multiple delimiters (comma/semicolon). Prevents invalid entries.
- server.urls = urls.split(",") + server.urls = urls + .split(',', ';') + .map { it.trim() } + .filter { it.isNotEmpty() }
86-94: Make SDP type mapping exhaustive (explicit PR_ANSWER)Avoid relying on else; future enum additions won’t be mis-mapped.
internal fun RTCSessionDescription.toKtor(): WebRtc.SessionDescription = WebRtc.SessionDescription( - type = when (this.sdpType) { - RTCSdpType.OFFER -> WebRtc.SessionDescriptionType.OFFER - RTCSdpType.ANSWER -> WebRtc.SessionDescriptionType.ANSWER - RTCSdpType.ROLLBACK -> WebRtc.SessionDescriptionType.ROLLBACK - else -> WebRtc.SessionDescriptionType.PROVISIONAL_ANSWER - }, + type = when (this.sdpType) { + RTCSdpType.OFFER -> WebRtc.SessionDescriptionType.OFFER + RTCSdpType.ANSWER -> WebRtc.SessionDescriptionType.ANSWER + RTCSdpType.ROLLBACK -> WebRtc.SessionDescriptionType.ROLLBACK + RTCSdpType.PR_ANSWER -> WebRtc.SessionDescriptionType.PROVISIONAL_ANSWER + }, sdp = this.sdp )
133-135: Avoid "null" error messages in SDP callbacksUse a meaningful default when error is null.
- resumeWithException(WebRtc.SdpException(message = error.toString())) + resumeWithException(WebRtc.SdpException(message = error ?: "Failed to create session description"))- resumeWithException(WebRtc.SdpException(message = error.toString())) + resumeWithException(WebRtc.SdpException(message = error ?: "Failed to set session description"))Also applies to: 143-145
ktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api (1)
81-85: Consider marking getNative helpers as internal/unstablegetNative(...) exposes dev.onvoid types. Mark with @internalapi and add KDoc warning about engine-specific coupling.
Please confirm these are intentionally public and stable; otherwise, annotate and document accordingly. As per coding guidelines.
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcDataChannel.kt (2)
54-61: Prefer state.canSend() over explicit OPEN checkClearer intent and future-proof if more sendable states appear.
- require(state == WebRtc.DataChannel.State.OPEN) { - "Can't send a message when the channel is not open." - } + require(state.canSend()) { "Channel is not open for sending." }Apply to both send overloads.
Also applies to: 62-68
124-126: Mark getNative as internal and make the cast safeAvoid leaking engine details; fail with a clear message.
- public fun WebRtcDataChannel.getNative(): RTCDataChannel { - return (this as JvmWebRtcDataChannel).inner - } + @io.ktor.util.InternalAPI + /** + * Exposes the underlying RTCDataChannel for JVM engine interop. Engine-specific; use with caution. + */ + public fun WebRtcDataChannel.getNative(): RTCDataChannel { + val jvm = this as? JvmWebRtcDataChannel + ?: error("Not a JVM WebRTC data channel") + return jvm.inner + }Note: add
import io.ktor.util.InternalAPIat the top of the file.ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/DefaultAudioFactory.kt (3)
21-25: Guard default capture device availabilityHandle environments without a capture device; fail fast with a clear message.
- override val audioModule: AudioDeviceModule = AudioDeviceModule().apply { - setRecordingDevice(MediaDevices.getDefaultAudioCaptureDevice()) - } + override val audioModule: AudioDeviceModule = AudioDeviceModule().apply { + val device = MediaDevices.getDefaultAudioCaptureDevice() + ?: error("No audio capture device available") + setRecordingDevice(device) + }
32-34: Clamp microphone volume to a valid rangePrevent invalid values when constraints.volume is out of [0.0, 1.0].
- val volumePercentage = constraints.volume?.let { (it * 100).toInt() } ?: 100 + val volumePercentage = (constraints.volume?.times(100)?.toInt() ?: 100).coerceIn(0, 100) audioModule.microphoneVolume = volumePercentage
37-40: Dispose factory before audio moduleAvoid potential use-after-free if factory references the module during disposal.
- audioModule.dispose() - peerConnectionFactory.dispose() + peerConnectionFactory.dispose() + audioModule.dispose()ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/CameraVideoFactory.kt (3)
13-21: Annotate internal API with @internalapi.Per guidelines, mark internal APIs with @internalapi for clarity.
As per coding guidelines
+import io.ktor.util.InternalAPI @@ -internal class VideoTrackCapturer : VideoCapturer { +@InternalAPI +internal class VideoTrackCapturer : VideoCapturer {
69-73: Add KDoc for public defaults.Document the intent/units of DEFAULT_VIDEO_WIDTH/HEIGHT/FRAME_RATE.
As per coding guidelines
10-10: Prefer star import for io.ktor. packages.*Replace single-type import with a star import to follow project style.
As per coding guidelines
-import io.ktor.client.webrtc.WebRtcMedia +import io.ktor.client.webrtc.*ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmSenders.kt (3)
41-55: Fix misleading DTMF KDoc (mentions iOS/protocol).This is JVM code; remove iOS/Protocol references.
23-29: Guard replaceTrack() cast and provide a clear error.Casting arbitrary tracks can crash. Validate the track origin.
- override suspend fun replaceTrack(withTrack: WebRtcMedia.Track?) { - if (withTrack == null) { - return inner.replaceTrack(null) - } - val withTrack = withTrack as JvmMediaTrack - inner.replaceTrack(withTrack.inner) - } + override suspend fun replaceTrack(withTrack: WebRtcMedia.Track?) { + if (withTrack == null) { + inner.replaceTrack(null) + return + } + require(withTrack is JvmMediaTrack) { + "Track must be created by JVM WebRTC engine" + } + inner.replaceTrack(withTrack.inner) + }
75-77: Verify degradationPreference mapping.Hardcoding DISABLED may be wrong if the native stack exposes a value. Confirm library support; if unsupported, document explicitly.
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/MockAudioFactory.kt (1)
17-17: Limit ticker visibility.Make ticker private to reduce test API surface.
- val ticker = Ticker(10.milliseconds) { + private val ticker = Ticker(10.milliseconds) {ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/JvmMediaDevices.kt (2)
48-52: KDoc typo.Remove stray backtick in “library`)” → “library)”.
As per coding guidelines
147-150: Reset recording flag on close.After stop/close, recordingStarted remains true; subsequent reuse won’t re-init recording.
override fun close() { - audioModule.stopRecording() + audioModule.stopRecording() + recordingStarted.value = false audioFactory.close() }ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/JvmMediaTrack.kt (2)
57-61: Add KDoc for public JvmAudioTrack/JvmVideoTrackThese public classes lack KDoc. Please add brief docs, including parameter notes about onDispose.
As per coding guidelines
Also applies to: 60-61
66-73: Safer getNative() to avoid CCE and improve error messagesUse a safe cast with a clear error instead of blind casts.
-public fun WebRtcMedia.VideoTrack.getNative(): VideoTrack = - (this as JvmMediaTrack).inner as VideoTrack +public fun WebRtcMedia.VideoTrack.getNative(): VideoTrack { + val jvm = this as? JvmMediaTrack + ?: error("Not a JVM track: ${this::class.qualifiedName}") + return jvm.inner as VideoTrack +} @@ -public fun WebRtcMedia.AudioTrack.getNative(): AudioTrack = - (this as JvmMediaTrack).inner as AudioTrack +public fun WebRtcMedia.AudioTrack.getNative(): AudioTrack { + val jvm = this as? JvmMediaTrack + ?: error("Not a JVM track: ${this::class.qualifiedName}") + return jvm.inner as AudioTrack +}ktor-client/ktor-client-webrtc/api/ktor-client-webrtc.klib.api (3)
830-838: Document close semantics for Capturer (now AutoCloseable)Now that Capturer extends AutoCloseable, please ensure source KDoc clarifies that close() is idempotent and frees native resources, and how it relates to stopCapture().
As per coding guidelines
903-925: Ensure SimulatorVideoCapturer.close() is idempotent and documentedNew close() surfaced; confirm implementations are safe to call multiple times and document behavior in source.
As per coding guidelines
1055-1074: Native accessors and defaultVideoCapturerFactory: add “use with caution” docsNew getNative() accessors and defaultVideoCapturerFactory() are advanced surfaces. Please add clear KDoc about platform specificity, stability, and responsibilities (lifecycle/ownership).
As per coding guidelines
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt (3)
48-53: Guard lateinit inner usage (properties, close())Accessing inner before initialize() throws UninitializedPropertyAccessException. Safer to guard and either return null (properties) or no-op (close).
override val localDescription: WebRtc.SessionDescription? - get() = inner.localDescription?.toKtor() + get() = if (this::inner.isInitialized) inner.localDescription?.toKtor() else null @@ override val remoteDescription: WebRtc.SessionDescription? - get() = inner.remoteDescription?.toKtor() + get() = if (this::inner.isInitialized) inner.remoteDescription?.toKtor() else null @@ override fun close() { super.close() - inner.close() + if (this::inner.isInitialized) { + inner.close() + } }Optionally, consider failing fast in other methods with check(this::inner.isInitialized) for clearer errors. As per coding guidelines
Also applies to: 221-224
89-98: Avoid duplicate DataChannel Open eventssetupEvents() already emits Open on state change. Emitting Open here can produce duplicates if a transition fires after registration. Emit only if already OPEN.
- channel.setupEvents(events) - events.emitDataChannelEvent(event = DataChannelEvent.Open(channel)) + channel.setupEvents(events) + if (channel.state == WebRtc.DataChannel.State.OPEN) { + events.emitDataChannelEvent(DataChannelEvent.Open(channel)) + }
54-57: Unify runInConnectionScope helperThis variant takes a () -> Unit, while JvmWebRtcDataChannel’s takes a suspend block. Consider a single shared helper (suspend CoroutineScope.() -> Unit) for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
gradle/libs.versions.toml(2 hunks)ktor-client/ktor-client-webrtc/androidDevice/test/io/ktor/client/webrtc/utils/Ignore.android.kt(0 hunks)ktor-client/ktor-client-webrtc/androidNative/test/io/ktor/client/webrtc/utils/ConnectionUtils.androidNative.kt(0 hunks)ktor-client/ktor-client-webrtc/androidNative/test/io/ktor/client/webrtc/utils/Ignore.androidNative.kt(0 hunks)ktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api(2 hunks)ktor-client/ktor-client-webrtc/api/ktor-client-webrtc.klib.api(13 hunks)ktor-client/ktor-client-webrtc/build.gradle.kts(2 hunks)ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRtcDataChannelTest.kt(1 hunks)ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRtcEngineTest.kt(0 hunks)ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRtcMediaTest.kt(0 hunks)ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/utils/Ignore.kt(0 hunks)ktor-client/ktor-client-webrtc/desktop/test/io/ktor/client/webrtc/utils/ConnectionUtils.desktop.kt(0 hunks)ktor-client/ktor-client-webrtc/desktop/test/io/ktor/client/webrtc/utils/Ignore.desktop.kt(0 hunks)ktor-client/ktor-client-webrtc/gradle.properties(1 hunks)ktor-client/ktor-client-webrtc/ios/test/io/ktor/client/webrtc/utils/ConnectionUtils.ios.kt(1 hunks)ktor-client/ktor-client-webrtc/ios/test/io/ktor/client/webrtc/utils/Ignore.ios.kt(0 hunks)ktor-client/ktor-client-webrtc/jsAndWasmShared/test/io/ktor/client/webrtc/utils/Ignore.jsAndWasmShared.kt(0 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmSenders.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmUtils.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcDataChannel.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/CameraVideoFactory.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/DefaultAudioFactory.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/JvmMediaDevices.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/JvmMediaTrack.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/MockAudioFactory.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/MockVideoFactory.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ignore.jvm.kt(0 hunks)ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ticker.kt(1 hunks)
💤 Files with no reviewable changes (11)
- ktor-client/ktor-client-webrtc/jsAndWasmShared/test/io/ktor/client/webrtc/utils/Ignore.jsAndWasmShared.kt
- ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRtcMediaTest.kt
- ktor-client/ktor-client-webrtc/ios/test/io/ktor/client/webrtc/utils/Ignore.ios.kt
- ktor-client/ktor-client-webrtc/androidNative/test/io/ktor/client/webrtc/utils/Ignore.androidNative.kt
- ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/utils/Ignore.kt
- ktor-client/ktor-client-webrtc/androidDevice/test/io/ktor/client/webrtc/utils/Ignore.android.kt
- ktor-client/ktor-client-webrtc/desktop/test/io/ktor/client/webrtc/utils/ConnectionUtils.desktop.kt
- ktor-client/ktor-client-webrtc/desktop/test/io/ktor/client/webrtc/utils/Ignore.desktop.kt
- ktor-client/ktor-client-webrtc/androidNative/test/io/ktor/client/webrtc/utils/ConnectionUtils.androidNative.kt
- ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ignore.jvm.kt
- ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRtcEngineTest.kt
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{kt,kts}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{kt,kts}: Follow Kotlin official style guide for all Kotlin source and build scripts
Use star imports for io.ktor.* packages
Max line length is 120 characters
Indent with 4 spaces in Kotlin code
Include a copyright header in new Kotlin files
Files:
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcDataChannel.ktktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRtcDataChannelTest.ktktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ticker.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/CameraVideoFactory.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmUtils.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/JvmMediaDevices.ktktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/MockAudioFactory.ktktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/MockVideoFactory.ktktor-client/ktor-client-webrtc/build.gradle.ktsktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/JvmMediaTrack.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmSenders.ktktor-client/ktor-client-webrtc/ios/test/io/ktor/client/webrtc/utils/ConnectionUtils.ios.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.ktktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/DefaultAudioFactory.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Document all public Kotlin APIs, including parameters, return types, and exceptions
Annotate internal APIs with @internalapi
Follow Kotlin error-handling conventions and use specific Ktor exceptions
Files:
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcDataChannel.ktktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRtcDataChannelTest.ktktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ticker.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/CameraVideoFactory.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmUtils.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/JvmMediaDevices.ktktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/MockAudioFactory.ktktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/MockVideoFactory.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/JvmMediaTrack.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmSenders.ktktor-client/ktor-client-webrtc/ios/test/io/ktor/client/webrtc/utils/ConnectionUtils.ios.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.ktktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/DefaultAudioFactory.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt
**/api/**
📄 CodeRabbit inference engine (CLAUDE.md)
Track all public API changes in the /api/ directories
Files:
ktor-client/ktor-client-webrtc/api/ktor-client-webrtc.klib.apiktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api
🧠 Learnings (1)
📚 Learning: 2025-08-26T06:07:28.352Z
Learnt from: CR
PR: ktorio/ktor#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T06:07:28.352Z
Learning: Project is Kotlin Multiplatform (JVM, JS, Native)
Applied to files:
ktor-client/ktor-client-webrtc/build.gradle.kts
🧬 Code graph analysis (9)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcDataChannel.kt (2)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt (1)
runInConnectionScope(54-57)ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtcDataChannel.kt (2)
stopReceivingMessages(186-188)emitMessage(182-184)
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ticker.kt (1)
ktor-client/ktor-client-plugins/ktor-client-auth/common/src/io/ktor/client/plugins/auth/Auth.kt (1)
atomic(22-24)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/JvmMediaDevices.kt (3)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/CameraVideoFactory.kt (1)
start(16-16)ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/MockVideoFactory.kt (1)
start(33-35)ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ticker.kt (1)
start(51-64)
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/MockAudioFactory.kt (1)
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ticker.kt (2)
stop(70-75)start(51-64)
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/MockVideoFactory.kt (4)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/CameraVideoFactory.kt (1)
stop(18-18)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/JvmMediaDevices.kt (1)
stop(35-35)ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/MockAudioFactory.kt (1)
stop(27-29)ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ticker.kt (1)
stop(70-75)
ktor-client/ktor-client-webrtc/ios/test/io/ktor/client/webrtc/utils/ConnectionUtils.ios.kt (1)
ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtcClient.kt (1)
WebRtcClient(48-53)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.kt (1)
ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtcEngine.kt (1)
createConnectionContext(157-159)
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.kt (1)
ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtcClient.kt (1)
WebRtcClient(48-53)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt (1)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcDataChannel.kt (2)
runInConnectionScope(83-85)setupEvents(87-118)
🔇 Additional comments (9)
ktor-client/ktor-client-webrtc/ios/test/io/ktor/client/webrtc/utils/ConnectionUtils.ios.kt (1)
17-19: No changes needed; pattern is intentional.
Throwing fromCoroutineExceptionHandleris used consistently across all platform test utilities to propagate coroutine exceptions into the test framework.ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/Ticker.kt (3)
1-17: LGTM!The copyright header, package structure, and imports are all correct and follow the Kotlin coding guidelines.
42-44: LGTM!The validation logic is correct and uses an appropriate error message.
46-75: Well-designed start/stop logic with appropriate thread-safety.The implementation correctly uses:
compareAndSetfor idempotent start/stop operations- Coroutine's
isActive(lines 56, 59) to automatically respect cancellation- Timing compensation to maintain consistent tick intervals
Note: There's a brief window between
stop()settingisActiveAtomicto false and the coroutine being cancelled where one additional tick might execute. This is acceptable behavior for a test utility and inherent to the cooperative cancellation model.ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRtcDataChannelTest.kt (1)
215-256: Verify the ignore is necessary and consider tracking removal.While selectively ignoring this single test (rather than the entire class) is the right approach for enabling JVM support, ensure:
- The webrtc-java bug is confirmed and documented
- There's a plan to re-enable this test once the external library is fixed
The transition from class-level ignores to a single method-level ignore demonstrates good progress in JVM WebRTC implementation.
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/utils/ConnectionUtils.jvm.kt (1)
15-26: LGTM for JVM test wiringCorrect engine/config setup and exception propagation for tests.
Also applies to: 29-29
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.kt (1)
38-52: RTCConfiguration build and connection init look good.Policies/servers mapped; candidate prefetch guarded; context wired.
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/JvmMediaDevices.kt (1)
127-135: Unsupported constraints validation: good.Clear, early validation for JVM-only limitations.
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/JvmMediaTrack.kt (1)
32-37: Make close() thread-safe and verify native disposal
- Replace
private var closed = falsewithprivate val closed = AtomicBoolean(false)and inclose()useif (closed.compareAndSet(false, true)) onDispose()to guard against concurrent calls.- Confirm whether
dev.onvoid.webrtc.media.MediaStreamTrackrequires an explicitstop()/dispose call; if so, invoke it insideonDisposeto avoid native-resource leaks.
ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRtcDataChannelTest.kt
Show resolved
Hide resolved
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmSenders.kt
Outdated
Show resolved
Hide resolved
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt
Outdated
Show resolved
Hide resolved
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt
Show resolved
Hide resolved
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcDataChannel.kt
Show resolved
Hide resolved
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.kt
Outdated
Show resolved
Hide resolved
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/CameraVideoFactory.kt
Outdated
Show resolved
Hide resolved
ktor-client/ktor-client-webrtc/jvm/test/io/ktor/client/webrtc/MockVideoFactory.kt
Show resolved
Hide resolved
bjhham
left a comment
bjhham
left a comment
There was a problem hiding this comment.
Looks good to me. You might wanna check the rabbit comments, it seems to have some opinions.
| tomcat = "9.0.108" | ||
| tomcat-jakarta = "10.1.44" | ||
|
|
||
| webrtc-java = "0.14.0" |
There was a problem hiding this comment.
Maybe we should ask the author when they're planning to release 1.0. It would be nice to have something stable.
There was a problem hiding this comment.
@bjhham I've created a discussion about version 1.0 devopvoid/webrtc-java#215
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.kt (1)
33-37: Safe factory resolution looks good; avoids ClassCastException.Using a safe cast to JvmMediaDevices with a clear error message resolves the earlier risk.
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt (2)
193-198: Null-safe sender lookup fix looks good.Using it.track?.id prevents NPE when sender.track is null.
142-145: Prevent Duration-to-Int overflow for maxPacketLifeTime.inWholeMilliseconds can exceed Int range; toInt() will overflow. Validate range before converting.
Apply this diff:
- options.maxPacketLifeTime?.let { maxPacketLifeTime = it.inWholeMilliseconds.toInt() } + options.maxPacketLifeTime?.let { + val ms = it.inWholeMilliseconds + require(ms in 0..Int.MAX_VALUE.toLong()) { + "maxPacketLifeTime must be in 0..${Int.MAX_VALUE} ms, got $ms" + } + maxPacketLifeTime = ms.toInt() + }ktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api (1)
134-150: KDoc for new public JVM APIs (duplicate of earlier ask)Please add KDoc for all new public classes/methods in
jvm/src/io/ktor/client/webrtc/**(params/returns/exceptions). This was requested earlier and still applies to the newly added types listed here.Add a quick check locally:
#!/bin/bash rg -n --type=kt -C0 '^\s*public\s+(class|interface|fun)\s' ktor-client/ktor-client-webrtc/jvm/src \ | while IFS=: read -r file ln _; do prev=$(sed -n "$((ln-1))p" "$file") [[ "$prev" =~ '/\\*\\*' ]] || echo "Missing KDoc: $file:$ln" doneBased on learnings
Also applies to: 791-869
🧹 Nitpick comments (10)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmSenders.kt (1)
75-76: Add explanatory comment for hardcoded degradationPreference
Override always returnsDISABLEDbecause the underlyingRTCRtpSendParametersdoesn’t expose this field. Insert a TODO or comment above the override inJvmRtpParametersto explain this limitation.ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/CameraVideoFactory.kt (2)
10-10: Use star import for io.ktor packages per coding guidelines.The coding guidelines specify using star imports for
io.ktor.*packages. Consider changing the specific import to a star import for consistency.Apply this diff:
-import io.ktor.client.webrtc.WebRtcMedia +import io.ktor.client.webrtc.*As per coding guidelines.
66-68: Consider adding KDoc for public constants.The coding guidelines specify documenting all public Kotlin APIs. While the constant names are self-explanatory, brief KDoc comments would improve API documentation completeness.
Example:
/** Default video capture width in pixels (720p). */ public const val DEFAULT_VIDEO_WIDTH: Int = 1280 /** Default video capture height in pixels (720p). */ public const val DEFAULT_VIDEO_HEIGHT: Int = 720 /** Default video capture frame rate in frames per second. */ public const val DEFAULT_VIDEO_FRAME_RATE: Int = 30As per coding guidelines.
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.kt (1)
9-9: Import style: prefer star imports for io.ktor. packages.*Replace single-class io.ktor import(s) with a star import to match the repo guideline.
As per coding guidelines
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt (2)
89-98: Avoid duplicate DataChannel Open events.Emitting Open immediately can duplicate the subsequent state-change event. Emit based on current state or rely on the observer.
Apply this diff to emit only if already OPEN:
- channel.setupEvents(events) - events.emitDataChannelEvent(event = DataChannelEvent.Open(channel)) + val initialState = channel.state + channel.setupEvents(events) + if (initialState == WebRtc.DataChannel.State.OPEN) { + events.emitDataChannelEvent(DataChannelEvent.Open(channel)) + }Please verify with existing tests that Open is not emitted twice for remotely-created channels.
54-57: Align runInConnectionScope with DataChannel helper for consistency.Consider matching the suspend signature used in JvmWebRtcDataChannel to standardize event dispatch helpers.
ktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api (4)
81-86: Optional: provide stable JVM names for top‑level helpersConsider adding
@file:JvmName("JvmSenders"),@file:JvmName("JvmWebRtcDataChannel"),@file:JvmName("JvmMediaTrack")to avoid*Ktclass names in the public surface for top-level functions. This improves JVM ergonomics and future binary stability.Also applies to: 130-133, 850-854
92-110: Consider visibility of initialize()
initialize(Function1): JvmWebRtcConnection(Line 103) appears to returnthisfor chaining. If not required publicly, make it internal or fold into construction/builders to avoid misordered usage and to keep the public API smaller.
58-71: Avoid leaking backend types in public API
JvmRtpParameters.getRtcp() returns dev.onvoid.webrtc.RTCRtcpParameters, coupling Ktor’s API to a specific implementation—change its signature to return WebRtc.RtcpParameters and provide a Jvm-only accessor to unwrap the native type.
134-150: Hide dev.onvoid.webrtc types from public JVM API
JvmWebRtcEngineConfig.getRtcFactory()/setRtcFactory()exposedev.onvoid.webrtc.PeerConnectionFactory; wrap behind a dedicatedgetNativeRtcFactory()helper or limit to internal APIs, or mark the config experimental to avoid long-term binary-compat risks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
ktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api(2 hunks)ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRtcDataChannelTest.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmSenders.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/CameraVideoFactory.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRtcDataChannelTest.kt
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{kt,kts}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{kt,kts}: Follow Kotlin official style guide for all Kotlin source and build scripts
Use star imports for io.ktor.* packages
Max line length is 120 characters
Indent with 4 spaces in Kotlin code
Include a copyright header in new Kotlin files
Files:
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/CameraVideoFactory.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmSenders.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Document all public Kotlin APIs, including parameters, return types, and exceptions
Annotate internal APIs with @internalapi
Follow Kotlin error-handling conventions and use specific Ktor exceptions
Files:
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/CameraVideoFactory.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmSenders.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt
**/api/**
📄 CodeRabbit inference engine (CLAUDE.md)
Track all public API changes in the /api/ directories
Files:
ktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api
🧠 Learnings (1)
📚 Learning: 2025-08-26T06:07:28.352Z
Learnt from: CR
PR: ktorio/ktor#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T06:07:28.352Z
Learning: Applies to **/*.kt : Document all public Kotlin APIs, including parameters, return types, and exceptions
Applied to files:
ktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api
🧬 Code graph analysis (2)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.kt (1)
ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtcEngine.kt (1)
createConnectionContext(157-159)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt (1)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcDataChannel.kt (2)
runInConnectionScope(83-85)setupEvents(87-118)
🔇 Additional comments (4)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmSenders.kt (1)
79-98: LGTM! getNative() functions correctly typed.The return type mismatch flagged in the previous review has been fixed. Line 96 now correctly declares
RTCRtpSendParametersas the return type, matching the inner field type ofJvmRtpParameters.The unsafe casting in these functions is acceptable given the explicit "Use it with caution" warnings in the documentation.
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/CameraVideoFactory.kt (1)
13-21: LGTM!The VideoTrackCapturer implementation is clean and follows a simple delegation pattern to wrap the external VideoDeviceSource.
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt (1)
216-219: Avoid UninitializedPropertyAccessException in close().Guard inner before closing to handle edge cases gracefully.
Apply this diff:
override fun close() { super.close() - inner.close() + if (this::inner.isInitialized) { + inner.close() + } }Likely an incorrect or invalid review comment.
ktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api (1)
51-57: LGTM for these JVM wrappersSignatures align with the common interfaces; additions look consistent with the rest of the WebRTC surface.
Also applies to: 72-80, 87-91, 111-129
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmSenders.kt
Show resolved
Hide resolved
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmSenders.kt
Show resolved
Hide resolved
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.kt
Outdated
Show resolved
Hide resolved
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/CameraVideoFactory.kt
Show resolved
Hide resolved
zibet27
force-pushed
the
zibet27/ktor-client-webrtc-java
branch
from
cd2a02c to
0a3da3f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/CameraVideoFactory.kt (1)
43-58: LGTM: robust device/capability selection and correct FPS ordering.Flat candidate list, resolution fitness, and descending FPS tie-breaker address earlier pitfalls. Good fallback when no capability meets target FPS.
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmSenders.kt (1)
23-28: Fixes look good (no shadowing; correct return type).replaceTrack no longer shadows; getNative() now returns RTCRtpSendParameters.
Also applies to: 90-95
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.kt (1)
33-37: LGTM: safe factory resolution and null-guard on peer connection.Avoids ClassCastException and NPE on creation; clear error paths.
Also applies to: 49-52
ktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api (2)
81-85: API alignment: RtpParameters.getNative returns RTCRtpSendParameters.Matches implementation; previous mismatch resolved.
134-150: Add KDoc to all public JVM APIs
Add KDoc for every public class, interface, and function under ktor-client/ktor-client-webrtc/jvm/src — include description, @param, @return, and @throws. Missing docs detected in:
- media/JvmMediaTrack.kt
- media/DefaultAudioFactory.kt
- media/JvmMediaDevices.kt
- media/CameraVideoFactory.kt
- JvmWebRtcEngine.kt
- JvmWebRtcDataChannel.kt
- JvmWebRtcConnection.kt
- JvmSenders.kt
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt (1)
137-146: Prevent Duration-to-Int overflow for maxPacketLifeTime.toInt() can overflow for large durations; also allow only non-negative values.
- options.maxPacketLifeTime?.let { maxPacketLifeTime = it.inWholeMilliseconds.toInt() } + options.maxPacketLifeTime?.let { + val ms = it.inWholeMilliseconds + require(ms in 0..Int.MAX_VALUE.toLong()) { + "maxPacketLifeTime must be in 0..${Int.MAX_VALUE} ms, got $ms" + } + maxPacketLifeTime = ms.toInt() + }
🧹 Nitpick comments (6)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/CameraVideoFactory.kt (2)
13-21: Annotate internal class with @internalapi.Mark VideoTrackCapturer as internal API per project guideline.
As per coding guidelines
+import io.ktor.util.* @@ -internal class VideoTrackCapturer : VideoCapturer { +@InternalAPI +internal class VideoTrackCapturer : VideoCapturer {
7-11: Align with import style for io.ktor. packages.*Prefer star import for io.ktor.*.
As per coding guidelines
-import io.ktor.client.webrtc.WebRtcMedia +import io.ktor.client.webrtc.*ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmSenders.kt (1)
23-28: Guard casts with explicit checks to improve error messages.Use require/is to fail fast with a clear message instead of ClassCastException.
override suspend fun replaceTrack(withTrack: WebRtcMedia.Track?) { if (withTrack == null) { return inner.replaceTrack(null) } - inner.replaceTrack((withTrack as JvmMediaTrack).inner) + require(withTrack is JvmMediaTrack) { + "Expected JvmMediaTrack, got ${withTrack::class.simpleName}" + } + inner.replaceTrack(withTrack.inner) } @@ - override suspend fun setParameters(parameters: WebRtc.RtpParameters) { - inner.parameters = (parameters as JvmRtpParameters).inner - } + override suspend fun setParameters(parameters: WebRtc.RtpParameters) { + require(parameters is JvmRtpParameters) { + "Expected JvmRtpParameters, got ${parameters::class.simpleName}" + } + inner.parameters = parameters.inner + }Also applies to: 34-36
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.kt (1)
7-10: Optional: use star import for io.ktor. packages.*As per coding guidelines
-import io.ktor.client.webrtc.media.JvmMediaDevices +import io.ktor.client.webrtc.media.*ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt (2)
193-197: Clarify error message when track not found.Include track id to aid debugging.
- val sender = inner.senders.firstOrNull { it.track?.id == mediaTrack.id } - ?: error("Track is not found.") + val sender = inner.senders.firstOrNull { it.track?.id == mediaTrack.id } + ?: error("Track not found: ${mediaTrack.id}. It may have been replaced or removed.")
20-21: Optional: use star import for io.ktor. packages.*As per coding guidelines
-import io.ktor.client.webrtc.media.JvmMediaTrack +import io.ktor.client.webrtc.media.*
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.devcontainer/Dockerfile(1 hunks)CONTRIBUTING.md(1 hunks)ktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api(2 hunks)ktor-client/ktor-client-webrtc/api/ktor-client-webrtc.api(0 hunks)ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRtcDataChannelTest.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmSenders.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/CameraVideoFactory.kt(1 hunks)
💤 Files with no reviewable changes (1)
- ktor-client/ktor-client-webrtc/api/ktor-client-webrtc.api
🚧 Files skipped from review as they are similar to previous changes (1)
- ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRtcDataChannelTest.kt
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{kt,kts}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{kt,kts}: Follow Kotlin official style guide for all Kotlin source and build scripts
Use star imports for io.ktor.* packages
Max line length is 120 characters
Indent with 4 spaces in Kotlin code
Include a copyright header in new Kotlin files
Files:
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmSenders.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/CameraVideoFactory.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Document all public Kotlin APIs, including parameters, return types, and exceptions
Annotate internal APIs with @internalapi
Follow Kotlin error-handling conventions and use specific Ktor exceptions
Files:
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmSenders.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/media/CameraVideoFactory.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt
**/api/**
📄 CodeRabbit inference engine (CLAUDE.md)
Track all public API changes in the /api/ directories
Files:
ktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api
🧠 Learnings (1)
📚 Learning: 2025-08-26T06:07:28.352Z
Learnt from: CR
PR: ktorio/ktor#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T06:07:28.352Z
Learning: Applies to **/*.kt : Document all public Kotlin APIs, including parameters, return types, and exceptions
Applied to files:
ktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api
🧬 Code graph analysis (2)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.kt (1)
ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtcEngine.kt (1)
createConnectionContext(157-159)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt (1)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcDataChannel.kt (2)
runInConnectionScope(83-85)setupEvents(87-118)
🪛 LanguageTool
CONTRIBUTING.md
[grammar] ~56-~56: There might be a mistake here.
Context: ...d have pulseaudio installed. The actual audio is not played during te...
(QB_NEW_EN)
[grammar] ~57-~57: There might be a mistake here.
Context: ...ctual audio is not played during tests, but WebRTC native library depends on it. Yo...
(QB_NEW_EN)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
CONTRIBUTING.md (1)
56-56: Previous issue resolved: stray closing tag removedThanks for removing the stray
</summary>mentioned earlier; docs render correctly now.ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt (1)
141-144: Fix Duration-to-Int overflow and validate non-negative values
inWholeMilliseconds.toInt()can overflow; also allow only non-negative values. This was previously flagged.- options.id?.let { id = it } - options.maxRetransmits?.let { maxRetransmits = it } - options.maxPacketLifeTime?.let { maxPacketLifeTime = it.inWholeMilliseconds.toInt() } + options.id?.let { id = it } + options.maxRetransmits?.let { + require(it >= 0) { "maxRetransmits must be >= 0, got $it" } + maxRetransmits = it + } + options.maxPacketLifeTime?.let { d -> + val ms = d.inWholeMilliseconds + require(ms in 0..Int.MAX_VALUE.toLong()) { + "maxPacketLifeTime must be in 0..${Int.MAX_VALUE} ms, got $ms" + } + maxPacketLifeTime = ms.toInt() + }
🧹 Nitpick comments (2)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt (2)
192-197: Clarify error message when track not foundImprove diagnosability.
- val sender = inner.senders.firstOrNull { it.track?.id == mediaTrack.id } - ?: error("Track is not found.") + val sender = inner.senders.firstOrNull { it.track?.id == mediaTrack.id } + ?: error("Track not found: ${mediaTrack.id}. It may have been replaced or removed.")
129-150: Avoid variable shadowing and improve readability in createDataChannelRename the local
optionsto avoid shadowing the function parameter.- val options = WebRtcDataChannelOptions().apply(options) + val channelOptions = WebRtcDataChannelOptions().apply(options) @@ - require(options.maxRetransmits == null || options.maxPacketLifeTime == null) { + require(channelOptions.maxRetransmits == null || channelOptions.maxPacketLifeTime == null) { "Both maxRetransmits and maxPacketLifeTime can't be set at the same time." } @@ - ordered = options.ordered - protocol = options.protocol - negotiated = options.negotiated + ordered = channelOptions.ordered + protocol = channelOptions.protocol + negotiated = channelOptions.negotiated @@ - options.id?.let { id = it } - options.maxRetransmits?.let { maxRetransmits = it } - options.maxPacketLifeTime?.let { maxPacketLifeTime = it.inWholeMilliseconds.toInt() } + channelOptions.id?.let { id = it } + channelOptions.maxRetransmits?.let { maxRetransmits = it } + channelOptions.maxPacketLifeTime?.let { maxPacketLifeTime = it.inWholeMilliseconds.toInt() } } val nativeChannel = inner.createDataChannel(label, channelInit) - val receiveOptions = DataChannelReceiveOptions().apply(options.receiveOptions) + val receiveOptions = DataChannelReceiveOptions().apply(channelOptions.receiveOptions)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CONTRIBUTING.md(1 hunks)ktor-client/ktor-client-webrtc/ios/src/io/ktor/client/webrtc/PeerConnection.kt(0 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt(1 hunks)
💤 Files with no reviewable changes (1)
- ktor-client/ktor-client-webrtc/ios/src/io/ktor/client/webrtc/PeerConnection.kt
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{kt,kts}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{kt,kts}: Follow Kotlin official style guide for all Kotlin source and build scripts
Use star imports for io.ktor.* packages
Max line length is 120 characters
Indent with 4 spaces in Kotlin code
Include a copyright header in new Kotlin files
Files:
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Document all public Kotlin APIs, including parameters, return types, and exceptions
Annotate internal APIs with @internalapi
Follow Kotlin error-handling conventions and use specific Ktor exceptions
Files:
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt
🧬 Code graph analysis (1)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt (2)
ktor-client/ktor-client-webrtc/ios/src/io/ktor/client/webrtc/PeerConnection.kt (1)
runInConnectionScope(57-60)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcDataChannel.kt (2)
runInConnectionScope(83-85)setupEvents(87-118)
🪛 LanguageTool
CONTRIBUTING.md
[grammar] ~57-~57: There might be a mistake here.
Context: ...ctual audio is not played during tests, but WebRTC native library depends on it. Yo...
(QB_NEW_EN)
🔇 Additional comments (1)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt (1)
89-97: Good: avoids duplicate DataChannel Open eventsOnly wiring the observer and letting the channel emit OPEN on state change prevents duplicates.
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt
Show resolved
Hide resolved
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
ktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api (1)
51-871: Ensure all public JVM APIs are documented with KDoc.Past review identified 27 public declarations lacking KDoc comments across JVM WebRTC files (JvmSenders.kt, JvmWebRtcEngine.kt, JvmWebRtcConnection.kt, media types, etc.). Public APIs should include
/** ... */documentation with@param,@return, and@throwstags as appropriate.Based on coding guidelines.
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt (1)
136-136: Guard Duration-to-Int conversion against overflow.
inWholeMillisecondsreturnsLong; callingtoInt()can silently overflow if the duration exceeds ~24 days (Int.MAX_VALUE ms).Apply this diff to validate the range:
- options.maxPacketLifeTime?.let { maxPacketLifeTime = it.inWholeMilliseconds.toInt() } + options.maxPacketLifeTime?.let { + val ms = it.inWholeMilliseconds + require(ms in 0..Int.MAX_VALUE) { "maxPacketLifeTime must be ≤ ${Int.MAX_VALUE} ms, got $ms" } + maxPacketLifeTime = ms.toInt() + }
🧹 Nitpick comments (1)
ktor-client/ktor-client-webrtc/build.gradle.kts (1)
19-35: Add KDoc to document the classifier resolution function.The function effectively detects the build machine's platform and architecture, but lacks documentation for maintainability. Consider adding a brief KDoc comment explaining its purpose and the classifier format it returns.
+/** + * Resolves the platform-specific classifier for the webrtc-java native library. + * Detects the host OS and architecture and returns a classifier string (e.g., "macos-aarch64"). + * Throws an error if the OS or architecture is unsupported. + */ fun resolveWebRtcJavaClassifier(): String {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
ktor-client/ktor-client-webrtc/androidNative/test/io/ktor/client/webrtc/utils/Ignore.android.kt(0 hunks)ktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api(2 hunks)ktor-client/ktor-client-webrtc/api/ktor-client-webrtc.klib.api(14 hunks)ktor-client/ktor-client-webrtc/build.gradle.kts(2 hunks)ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRtcEngineTest.kt(0 hunks)ktor-client/ktor-client-webrtc/gradle.properties(1 hunks)ktor-client/ktor-client-webrtc/ios/src/io/ktor/client/webrtc/PeerConnection.kt(0 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt(1 hunks)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.kt(1 hunks)
💤 Files with no reviewable changes (3)
- ktor-client/ktor-client-webrtc/ios/src/io/ktor/client/webrtc/PeerConnection.kt
- ktor-client/ktor-client-webrtc/androidNative/test/io/ktor/client/webrtc/utils/Ignore.android.kt
- ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRtcEngineTest.kt
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{kt,kts}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{kt,kts}: Follow Kotlin official style guide for all Kotlin source and build scripts
Use star imports for io.ktor.* packages
Max line length is 120 characters
Indent with 4 spaces in Kotlin code
Include a copyright header in new Kotlin files
Files:
ktor-client/ktor-client-webrtc/build.gradle.ktsktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.kt
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Document all public Kotlin APIs, including parameters, return types, and exceptions
Annotate internal APIs with @internalapi
Follow Kotlin error-handling conventions and use specific Ktor exceptions
Files:
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.ktktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.kt
**/api/**
📄 CodeRabbit inference engine (CLAUDE.md)
Track all public API changes in the /api/ directories
Files:
ktor-client/ktor-client-webrtc/api/ktor-client-webrtc.klib.apiktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api
🧠 Learnings (2)
📚 Learning: 2025-08-26T06:07:28.352Z
Learnt from: CR
PR: ktorio/ktor#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T06:07:28.352Z
Learning: Project is Kotlin Multiplatform (JVM, JS, Native)
Applied to files:
ktor-client/ktor-client-webrtc/build.gradle.kts
📚 Learning: 2025-08-26T06:07:28.352Z
Learnt from: CR
PR: ktorio/ktor#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T06:07:28.352Z
Learning: Applies to **/*.kt : Document all public Kotlin APIs, including parameters, return types, and exceptions
Applied to files:
ktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api
🧬 Code graph analysis (2)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcConnection.kt (2)
ktor-client/ktor-client-webrtc/ios/src/io/ktor/client/webrtc/PeerConnection.kt (1)
runInConnectionScope(57-60)ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcDataChannel.kt (2)
runInConnectionScope(83-85)setupEvents(87-118)
ktor-client/ktor-client-webrtc/jvm/src/io/ktor/client/webrtc/JvmWebRtcEngine.kt (1)
ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtcEngine.kt (1)
createConnectionContext(157-159)
🔇 Additional comments (2)
ktor-client/ktor-client-webrtc/gradle.properties (1)
7-8: Configuration updates align with target removal objectives.The changes disable POSIX and Android native targets, consistent with the PR goal to remove targets lacking Engine implementations. These minimal configuration updates focus the build on supported platforms (JVM).
Please confirm that the JVM target build remains enabled and unaffected by these changes. If there's a corresponding Gradle build script that interprets these flags, verifying that
target.jvm(or equivalent) is not inadvertently disabled would be helpful.ktor-client/ktor-client-webrtc/build.gradle.kts (1)
60-66: Classifier-based dependency resolution is sound.The logic correctly constructs the platform-specific webrtc-java dependency using the classifier resolver. The workaround comment explains why manual addition is necessary rather than relying on Gradle's automatic classifier resolution, which is helpful for future maintainers. The dependency notation format (using colon separator) is correct for Maven coordinates with classifiers.
Verify that the classifier format matches webrtc-java's published artifact naming convention (e.g., ensure the function output aligns with available artifacts on Maven Central or the webrtc-java project's release artifacts).
zibet27
force-pushed
the
zibet27/ktor-client-webrtc-java
branch
from
bf07dcf to
519f9c8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
Dockerfile (1)
8-17: Evaluate whether all system dependencies are necessary.The dependencies layer installs
curl,wget,unzip, andgit. Verify that these are all needed for the WebRTC build:
curlandwgetmay both be unnecessary if only one is used.unzipmay not be needed if Gradle handles archive extraction.gitmay only be necessary if the build clones repositories.If any are unused, remove them to reduce image size and attack surface.
ktor-client/ktor-client-webrtc/api/ktor-client-webrtc.klib.api (1)
1055-1076: Interop pattern follows Kotlin Multiplatform best practices.The
getNative()extension functions (lines 1055-1074) provide necessary escape hatches for platform-specific interop. This is a well-established pattern in Kotlin Multiplatform libraries when users need direct access to underlying native types.The
defaultVideoCapturerFactory()function (line 1076) provides a convenient default implementation, which is good API design.Note: These functions expose internal implementation details, which slightly couples the public API to the underlying webrtc-java library. Consider documenting that these are advanced/interop APIs and that the returned native types may change if the underlying WebRTC implementation is swapped.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Dockerfile(1 hunks)ktor-client/ktor-client-webrtc/androidNative/test/io/ktor/client/webrtc/utils/Ignore.android.kt(0 hunks)ktor-client/ktor-client-webrtc/api/ktor-client-webrtc.klib.api(14 hunks)ktor-client/ktor-client-webrtc/gradle.properties(1 hunks)
💤 Files with no reviewable changes (1)
- ktor-client/ktor-client-webrtc/androidNative/test/io/ktor/client/webrtc/utils/Ignore.android.kt
🧰 Additional context used
📓 Path-based instructions (1)
**/api/**
📄 CodeRabbit inference engine (CLAUDE.md)
Track all public API changes in the /api/ directories
Files:
ktor-client/ktor-client-webrtc/api/ktor-client-webrtc.klib.api
🔇 Additional comments (4)
ktor-client/ktor-client-webrtc/gradle.properties (1)
7-8: Inconsistency between PR objectives and actual changes.The PR objectives state "Removed targets that lacked an Engine implementation (desktop, androidNative)", but the actual changes show the opposite:
target.androidNative=true(enabled) andtarget.desktop=false(disabled).From a technical standpoint, this configuration makes sense for a JVM-based WebRTC implementation (webrtc-java is a JVM library that Android can use via Java interop, but desktop may lack the necessary engine). However, the PR description should be clarified to accurately reflect that androidNative is being enabled, not removed.
Confirm that:
- The desktop target was intentionally disabled because it lacks a WebRTC engine implementation
- The androidNative target is now enabled because the JVM WebRTC implementation works on Android
- The PR objectives description should be updated to reflect these actual changes (enable androidNative, disable desktop) rather than claiming both targets were removed
ktor-client/ktor-client-webrtc/api/ktor-client-webrtc.klib.api (3)
831-837: LGTM: Capturer interface addition for iOS platform.The
Capturerinterface provides a clean abstraction for video capture with start/stop lifecycle and AutoCloseable support. ThestartCapture()method at line 835 is appropriately part of the interface contract alongsidestopCapture().
840-1052: LGTM: Comprehensive iOS WebRTC implementation.The iOS-specific API surface is well-structured and follows consistent patterns:
- Proper use of platform-specific prefixes (
Ios*naming)- Clean separation between video capture implementations (
CameraVideoCapturer,SimulatorVideoCapturer)- Appropriate factory interfaces (
VideoCapturerFactory)- Complete implementation of core WebRTC types (Connection, DataChannel, Engine, etc.)
- Platform-specific configuration type (
IosWebRtcEngineConfig)The addition of these iOS-specific types significantly expands the public API surface and provides the necessary foundation for JVM WebRTC support.
2-3: The review comment is incorrect—lines 2-3 appear unchanged from HEAD.Git history shows the current state of lines 2-3 is identical to the previous commit, with no change from the
applealias or removal of androidNative targets. Additionally, the gradle.properties file confirmstarget.androidNative=trueand the module includes the./ktor-client/ktor-client-webrtc/androidNativesource directory, indicating androidNative targets are actively supported in this module, contradicting the stated PR objective of removing them.The concerns about breaking changes and target removal do not align with the actual diff or codebase state.
Likely an incorrect or invalid review comment.
zibet27
force-pushed
the
zibet27/ktor-client-webrtc-java
branch
from
519f9c8 to
e4fd653
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
ktor-client/ktor-client-webrtc/api/ktor-client-webrtc.klib.api (4)
839-843: Factory signature leaks native type; consider decoupling.
VideoCapturerFactory.create(..., WebRTC/RTCVideoCapturerDelegateProtocol)exposes a platform delegate in the public API, tightening coupling to the Apple WebRTC SDK. Prefer a Ktor-owned delegate wrapper (e.g.,VideoCapturerCallback) to preserve room for swapping native deps later.
875-877: DEFAULT_FPS type inconsistency between capturers.
CameraVideoCapturer.Companion.DEFAULT_FPSisDouble;SimulatorVideoCapturer.Companion.DEFAULT_FPSisInt. Unify the type (Int is typical for FPS) to avoid cross-platform/naming confusion in consumers.Also applies to: 915-917
1055-1074: New getNative()/defaultVideoCapturerFactory(): mark stability and document ownership.The iOS
getNative()extensions anddefaultVideoCapturerFactory()are additive, but:
- Document lifetime/ownership and threading of returned native handles.
- Consider an opt‑in annotation (e.g.,
@DelicateWebRtcNativeApi) to signal advanced/unstable surface and allow future evolution.Also applies to: 1076-1076
979-981: Publicinitialize(...)on IosWebRtcConnection: restrict or rename.
initialize(Function1<RTCPeerConnectionDelegateProtocol, RTCPeerConnection?>)looks like internal wiring. If not meant for general consumption, mark internal or annotate as experimental; otherwise consider a more explicit name and docs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
ktor-client/ktor-client-webrtc/androidNative/test/io/ktor/client/webrtc/utils/Ignore.android.kt(0 hunks)ktor-client/ktor-client-webrtc/api/ktor-client-webrtc.klib.api(14 hunks)ktor-client/ktor-client-webrtc/gradle.properties(1 hunks)
💤 Files with no reviewable changes (1)
- ktor-client/ktor-client-webrtc/androidNative/test/io/ktor/client/webrtc/utils/Ignore.android.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- ktor-client/ktor-client-webrtc/gradle.properties
🧰 Additional context used
📓 Path-based instructions (1)
**/api/**
📄 CodeRabbit inference engine (CLAUDE.md)
Track all public API changes in the /api/ directories
Files:
ktor-client/ktor-client-webrtc/api/ktor-client-webrtc.klib.api
🔇 Additional comments (1)
ktor-client/ktor-client-webrtc/api/ktor-client-webrtc.klib.api (1)
830-837: ****The review comment's concern about breaking BC by adding
startCapture()to theCapturerinterface is unfounded. The interface already existed in the previous version with the same abstract methods (startCapture(),stopCapture(),isCapturing). The source code atktor-client/ktor-client-webrtc/ios/src/io/ktor/client/webrtc/media/MediaDevices.kt(lines 34–37) shows no changes to the interface definition.The actual API dump change is limited to the targets header: expanding from iOS-only (
[iosArm64, iosSimulatorArm64, iosX64, js, wasmJs]) to include Android native variants. Existing implementers likeCameraVideoCapturerandSimulatorVideoCaptureralready satisfy the interface contract—no BC breaks are introduced.Likely an incorrect or invalid review comment.
Subsystem
WebRTC Client.
Motivation
KTOR-8953 WebRTC client on JVM
Solution
The solution is based on webrtc-java.
One common test(
testDataChannelBufferedAmountLowEvent) is still ignored because of a bug in a library.Also, some targets that didn't have an Engine implementation were removed (desktop, androidNative).