-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Webrtc client for Android, JS and WasmJs targets #4897
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis change introduces a comprehensive multiplatform WebRTC client engine and API for the Ktor client, supporting Android, JVM, JavaScript (including Wasm), and POSIX platforms. It adds core abstractions, platform-specific implementations, utility functions, and extensive test suites for WebRTC peer connections, media tracks, and signaling. The build system is updated to support Android and AndroidX, and the new module is integrated into the project structure. Changes
Suggested reviewers
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 26
🧹 Nitpick comments (22)
build-logic/src/main/kotlin/ktorbuild.kmp.gradle.kts (1)
42-50
: Clean up or validate Android source sets configuration.The commented-out block disabling explicit Android JVM source set registration may now be obsolete, as Android targets and source sets are configured via
KtorTargets
.
- If Android sources are correctly wired elsewhere, remove this dead code to keep the script clean.
- Otherwise, verify that Android main and test source sets are properly registered post-evaluation.
ktor-client/ktor-client-webrtc/build.gradle.kts (1)
89-90
: Consider adding explanation for disabled test tasks.The JS and Wasm node tests are disabled without explanation. Consider adding comments explaining why these tests are disabled and under what conditions they might be re-enabled.
+// Disable JS and Wasm node tests due to WebRTC dependency limitations in Node.js environment tasks.named("jsNodeTest") { onlyIf { false } } tasks.named("wasmJsNodeTest") { onlyIf { false } }
ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/Engine.kt (1)
7-7
: Consider adding documentation for the external interface.The
JsPeerConnectionConfig
interface is declared as external but appears empty. Consider adding documentation explaining its purpose and how it will be implemented in platform-specific code.+/** + * Platform-specific peer connection configuration for JavaScript/Wasm targets. + * This interface should be implemented by the concrete platform implementations. + */ public external interface JsPeerConnectionConfigktor-client/ktor-client-webrtc/js/test/io/ktor/client/webrtc/MockMediaDevices.js.kt (1)
19-27
: Add error handling for canvas operations.The canvas operations could potentially fail. Consider adding error handling or validation to ensure robust test execution.
actual fun makeDummyVideoStreamTrack(width: Int, height: Int): WebRTCMedia.VideoTrack { + require(width > 0 && height > 0) { "Width and height must be positive" } + val canvas = document.createElement("canvas") as HTMLCanvasElement canvas.width = width canvas.height = height val ctx = canvas.getContext("2d") as CanvasRenderingContext2D ctx.fillRect(0.0, 0.0, width.toDouble(), height.toDouble()) - val stream = canvas.captureStream() + val stream = canvas.captureStream() ?: error("Failed to capture canvas stream") return JsVideoTrack(stream.getVideoTracks()[0], stream) }ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/peer/LibDom.kt (1)
40-52
: Consider improving ReadonlyMap type safety.The ReadonlyMap interface uses
dynamic
types in several places, which reduces type safety. Consider using more specific generic types where possible.public external interface ReadonlyMap<K> { - public fun entries(): Iterator<dynamic> + public fun <V> entries(): Iterator<Pair<K, V>> - public fun values(): Iterator<dynamic> + public fun <V> values(): Iterator<V> public fun forEach( - callbackfn: (value: dynamic, key: K, map: ReadonlyMap<K>) -> Unit, + callbackfn: (value: V, key: K, map: ReadonlyMap<K>) -> Unit, thisArg: Any = definedExternally )Note: This change would require making the interface generic over both K and V types.
ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRTCClient.kt (2)
7-9
: Remove duplicate copyright header.There's a duplicate copyright header that should be removed to avoid redundancy and maintain code cleanliness.
-/* - * Copyright 2014-2025 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license. - */ -
36-41
: Consider making engineConfig a computed property.The
engineConfig
property directly exposes the engine's config, but it could be more explicit about its relationship to the engine.Consider making this a computed property for clarity:
- public val engineConfig: WebRTCConfig = engine.config + public val engineConfig: WebRTCConfig get() = engine.configThis makes it clear that changes to the engine's config will be reflected in this property.
ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRTC.kt (2)
100-100
: Consider documenting thegetNative()
return type design decision.The
getNative()
methods returnAny
type, which loses compile-time type safety. While this appears to be an intentional design to support platform-specific implementations, consider adding KDoc comments explaining what types are expected on each platform.Also applies to: 131-131
201-202
: Improve permission exception message clarity.The current message format could be more informative and handle null mediaType gracefully.
Apply this diff to improve the message:
- public class PermissionException(mediaType: String?) : - RuntimeException("You should grant $mediaType permission for this operation to work.") + public class PermissionException(mediaType: String?) : + RuntimeException( + if (mediaType != null) "Permission required: $mediaType. Please grant this permission to continue." + else "Permission required for this operation." + )ktor-client/ktor-client-webrtc/android/src/io/ktor/client/webrtc/engine/MediaDevices.kt (2)
134-134
: Use more specific exception for missing camera.Using
error()
throws a genericIllegalStateException
. Consider usingWebRTCMedia.DeviceException
for consistency with other device-related errors.- val cameraId = findCameraId(constraints) ?: error("No camera found for such constraints") + val cameraId = findCameraId(constraints) + ?: throw WebRTCMedia.DeviceException("No camera found matching the specified constraints")
139-139
: Document the 60 FPS limit.The hardcoded 60 FPS limit should be documented or made configurable.
+ // Limit frame rate to 60 FPS maximum to ensure consistent performance val videoFrameRate = constraints.frameRate ?: min(format.framerate.max, 60)
build-logic/src/main/kotlin/ktorbuild/targets/KtorTargets.kt (1)
254-255
: Verify Android source set filtering behavior.The filtering of Android source sets is necessary since they're added post-evaluation. Consider adding a comment explaining this behavior for future maintainers.
// Android source sets are ignored because they are added after project evaluation. + // This is expected behavior when the Android Gradle Plugin is applied. val filteredSourceSets = extraSourceSets.filter { it.androidSourceSetInfoOrNull == null }
ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/utils/JsUtils.kt (2)
73-73
: Consider using native JS functions for better performance.The
getValues
function uses dynamic JS execution which could be optimized. Since this appears to be performance-critical code for stats processing, consider using more efficient approaches.Replace the dynamic JS call with a more type-safe approach:
-private fun <T> getValues(map: dynamic): Array<T> = js("Array.from(map.values())") +@OptIn(ExperimentalJsCollectionsApi::class) +private fun <T> getValues(map: JsMap<*, T>): Array<T> = map.values().toTypedArray()
76-76
: Inconsistent experimental API usage.The function uses
ExperimentalJsCollectionsApi
but doesn't consistently apply this pattern throughout the file. Consider standardizing the approach to JS collection handling.Apply the experimental annotation consistently:
+@OptIn(ExperimentalJsCollectionsApi::class) private fun objectToMap(obj: dynamic): JsMap<String, Any?> = js("new Map(Object.entries(obj))")
ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/peer/RTCPeerConnection.kt (1)
626-670
: Deprecated callback-based API methods included.The file includes both modern Promise-based WebRTC methods and deprecated callback-based methods. While this maintains backward compatibility, it may encourage usage of deprecated patterns.
Consider adding deprecation annotations to callback-based methods:
+@Deprecated("Use Promise-based createOffer() instead", ReplaceWith("createOffer()")) public fun createOffer( successCallback: RTCSessionDescriptionCallback, failureCallback: RTCPeerConnectionErrorCallback, options: RTCOfferOptions = definedExternally ): Promise<Unit>
ktor-client/ktor-client-webrtc/wasmJs/src/io/ktor/client/webrtc/utils/JsUtils.kt (2)
94-101
: Incomplete type handling in deserialization.The
deserializeJsItem
function only handles basic types (String, Number, Boolean) but returnsnull
for any other type. This could lead to data loss for complex objects or arrays in RTC stats.Enhance type handling to support more JS types:
private fun deserializeJsItem(item: JsAny?): Any? { return when (item) { is JsString -> item.toString() is JsNumber -> item.toInt() is JsBoolean -> item.toBoolean() + is JsArray<*> -> item.toArray().map { deserializeJsItem(it) } + null -> null - else -> null + else -> item.toString() // Fallback to string representation } }
107-107
: Consider handling deserialization errors explicitly.The comment "maybe throw error?" suggests uncertainty about error handling when
deserializeJsItem
returns null. For better debugging, consider logging or throwing specific errors for unsupported types.Add explicit error handling:
- val value = deserializeJsItem(entry[1]) // maybe throw error? + val value = deserializeJsItem(entry[1]) + ?: kotlin.run { + console.warn("Unsupported value type in stats: ${entry[1]}") + return@run null + }ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRTCEngine.kt (2)
26-26
: Document the meaning of negative statsRefreshRate.The default value of
-1
forstatsRefreshRate
is unclear. Consider adding documentation to explain that negative values disable stats collection, or use a more explicit approach.+ /** + * Refresh rate for WebRTC stats in milliseconds. + * Set to a negative value to disable automatic stats collection. + */ public var statsRefreshRate: Long = -1
56-56
: Document the empty close() implementation.The empty
close()
method might be confusing. Consider adding documentation to explain why it's safe to have an empty implementation or make it abstract if subclasses should implement cleanup.+ /** + * Default implementation does nothing as WebRTC engines typically don't require explicit cleanup. + * Override this method if your engine implementation requires resource cleanup. + */ override fun close() {}ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/Engine.js.kt (1)
234-244
: Consider preserving original stream context.Creating a new
MediaStream()
when getting the track might lose important stream context. Consider storing and using the original stream if available.Consider enhancing the implementation to preserve stream context:
public class JsRtpSender(public val nativeSender: RTCRtpSender) : WebRTC.RtpSender { override val dtmf: WebRTC.DtmfSender? get() = nativeSender.dtmf?.let { JsDtmfSender(it) } - override val track: WebRTCMedia.Track? get() = nativeSender.track?.let { JsMediaTrack.from(it, MediaStream()) } + override val track: WebRTCMedia.Track? get() = nativeSender.track?.let { + // TODO: Consider preserving the original stream context if available + JsMediaTrack.from(it, MediaStream()) + } override fun getNative(): Any = nativeSenderktor-client/ktor-client-webrtc/wasmJs/src/io/ktor/client/webrtc/peer/RTCPeerConnection.kt (1)
5-6
: Consider being more specific with suppressions.Using a blanket
@file:Suppress("unused")
might hide actual unused code. Consider suppressing only where necessary or documenting why these declarations appear unused (they're used via JS interop).-@file:Suppress("unused") +@file:Suppress("unused") // External declarations used via JS interopktor-client/ktor-client-webrtc/api/android/ktor-client-webrtc.api (1)
441-462
: Consider adding validation for video track constraints.The
VideoTrackConstraints
class accepts nullableInteger
values for width, height, and frameRate without apparent validation. Consider adding validation to ensure:
- Width and height are positive when provided
- Frame rate is within reasonable bounds (e.g., 1-120 fps)
- Aspect ratio is positive when provided
This would prevent runtime errors when invalid constraints are passed to the native WebRTC implementation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
kotlin-js-store/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (47)
.idea/dictionaries/oleh_pantus.xml
(1 hunks)build-logic/build.gradle.kts
(1 hunks)build-logic/settings.gradle.kts
(2 hunks)build-logic/src/main/kotlin/ktorbuild.kmp.gradle.kts
(1 hunks)build-logic/src/main/kotlin/ktorbuild/targets/KtorTargets.kt
(6 hunks)build-settings-logic/build.gradle.kts
(1 hunks)build-settings-logic/settings.gradle.kts
(2 hunks)build-settings-logic/src/main/kotlin/ktorsettings.dependency-resolution-management.settings.gradle.kts
(1 hunks)gradle.properties
(1 hunks)gradle/libs.versions.toml
(4 hunks)ktor-client/ktor-client-webrtc/AndroidManifest.xml
(1 hunks)ktor-client/ktor-client-webrtc/android/src/io/ktor/client/webrtc/Utils.kt
(1 hunks)ktor-client/ktor-client-webrtc/android/src/io/ktor/client/webrtc/WebRTCEngine.android.kt
(1 hunks)ktor-client/ktor-client-webrtc/android/src/io/ktor/client/webrtc/engine/Engine.kt
(1 hunks)ktor-client/ktor-client-webrtc/android/src/io/ktor/client/webrtc/engine/MediaDevices.kt
(1 hunks)ktor-client/ktor-client-webrtc/android/src/io/ktor/client/webrtc/peer/LibAndroid.kt
(1 hunks)ktor-client/ktor-client-webrtc/androidInstrumented/test/io/ktor/client/webrtc/WebRTCEngineTest.android.kt
(1 hunks)ktor-client/ktor-client-webrtc/api/android/ktor-client-webrtc.api
(1 hunks)ktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api
(1 hunks)ktor-client/ktor-client-webrtc/build.gradle.kts
(1 hunks)ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRTC.kt
(1 hunks)ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRTCClient.kt
(1 hunks)ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRTCEngine.kt
(1 hunks)ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRTCEngineTest.kt
(1 hunks)ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRTCPermissionTest.kt
(1 hunks)ktor-client/ktor-client-webrtc/gradle.properties
(1 hunks)ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/Engine.js.kt
(1 hunks)ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/peer/LibDom.kt
(1 hunks)ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/peer/RTCPeerConnection.kt
(1 hunks)ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/utils/JsUtils.kt
(1 hunks)ktor-client/ktor-client-webrtc/js/test/io/ktor/client/webrtc/MockMediaDevices.js.kt
(1 hunks)ktor-client/ktor-client-webrtc/js/test/io/ktor/client/webrtc/WebRTCEngineTest.js.kt
(1 hunks)ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/Engine.kt
(1 hunks)ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/Utils.kt
(1 hunks)ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/WebRTCEngine.jsAndWasmShared.kt
(1 hunks)ktor-client/ktor-client-webrtc/jsAndWasmShared/test/io/ktor/client/webrtc/MockMediaDevices.kt
(1 hunks)ktor-client/ktor-client-webrtc/jsAndWasmShared/test/io/ktor/client/webrtc/WebRTCEngineTest.jsAndWasmShared.kt
(1 hunks)ktor-client/ktor-client-webrtc/jvmAndPosix/src/io/ktor/client/webrtc/WebRTCEngine.posix.kt
(1 hunks)ktor-client/ktor-client-webrtc/jvmAndPosix/test/io/ktor/client/webrtc/WebRTCEngineTest.jvmAndPosix.kt
(1 hunks)ktor-client/ktor-client-webrtc/wasmJs/src/io/ktor/client/webrtc/Engine.wasmJs.kt
(1 hunks)ktor-client/ktor-client-webrtc/wasmJs/src/io/ktor/client/webrtc/peer/LibDom.kt
(1 hunks)ktor-client/ktor-client-webrtc/wasmJs/src/io/ktor/client/webrtc/peer/RTCPeerConnection.kt
(1 hunks)ktor-client/ktor-client-webrtc/wasmJs/src/io/ktor/client/webrtc/utils/JsUtils.kt
(1 hunks)ktor-client/ktor-client-webrtc/wasmJs/test/io/ktor/client/webrtc/MockMediaDevices.wasmJs.kt
(1 hunks)ktor-client/ktor-client-webrtc/wasmJs/test/io/ktor/client/webrtc/WebRTCEngineTest.wasmJs.kt
(1 hunks)ktor-test-server/settings.gradle.kts
(1 hunks)settings.gradle.kts
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (9)
ktor-client/ktor-client-webrtc/wasmJs/test/io/ktor/client/webrtc/WebRTCEngineTest.wasmJs.kt (1)
ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRTCClient.kt (1)
WebRTCClient
(48-52)
ktor-client/ktor-client-webrtc/js/test/io/ktor/client/webrtc/WebRTCEngineTest.js.kt (1)
ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRTCClient.kt (1)
WebRTCClient
(48-52)
ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRTCPermissionTest.kt (5)
ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRTCEngineTest.kt (2)
runTestWithPermissions
(27-40)createTestWebRTCClient
(22-22)ktor-client/ktor-client-webrtc/js/test/io/ktor/client/webrtc/WebRTCEngineTest.js.kt (1)
createTestWebRTCClient
(7-12)ktor-client/ktor-client-webrtc/wasmJs/test/io/ktor/client/webrtc/WebRTCEngineTest.wasmJs.kt (1)
createTestWebRTCClient
(7-12)ktor-client/ktor-client-webrtc/androidInstrumented/test/io/ktor/client/webrtc/WebRTCEngineTest.android.kt (1)
createTestWebRTCClient
(14-21)ktor-client/ktor-client-webrtc/jvmAndPosix/test/io/ktor/client/webrtc/WebRTCEngineTest.jvmAndPosix.kt (1)
createTestWebRTCClient
(7-9)
ktor-client/ktor-client-webrtc/jsAndWasmShared/test/io/ktor/client/webrtc/MockMediaDevices.kt (2)
ktor-client/ktor-client-webrtc/js/test/io/ktor/client/webrtc/MockMediaDevices.js.kt (2)
makeDummyAudioStreamTrack
(11-17)makeDummyVideoStreamTrack
(19-27)ktor-client/ktor-client-webrtc/wasmJs/test/io/ktor/client/webrtc/MockMediaDevices.wasmJs.kt (2)
makeDummyAudioStreamTrack
(31-34)makeDummyVideoStreamTrack
(36-42)
build-logic/src/main/kotlin/ktorbuild/targets/KtorTargets.kt (1)
build-logic/src/main/kotlin/ktorbuild/internal/TrackedKotlinHierarchy.kt (2)
group
(52-56)withAndroidTarget
(126-126)
ktor-client/ktor-client-webrtc/androidInstrumented/test/io/ktor/client/webrtc/WebRTCEngineTest.android.kt (1)
ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRTCClient.kt (1)
WebRTCClient
(48-52)
ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRTCEngineTest.kt (7)
ktor-client/ktor-client-webrtc/jsAndWasmShared/test/io/ktor/client/webrtc/MockMediaDevices.kt (1)
grantPermissions
(7-10)ktor-client/ktor-client-webrtc/jsAndWasmShared/test/io/ktor/client/webrtc/WebRTCEngineTest.jsAndWasmShared.kt (1)
grantPermissions
(7-7)ktor-client/ktor-client-webrtc/androidInstrumented/test/io/ktor/client/webrtc/WebRTCEngineTest.android.kt (2)
grantPermissions
(23-35)createTestWebRTCClient
(14-21)ktor-client/ktor-client-webrtc/jvmAndPosix/test/io/ktor/client/webrtc/WebRTCEngineTest.jvmAndPosix.kt (2)
grantPermissions
(11-13)createTestWebRTCClient
(7-9)ktor-test-dispatcher/common/src/TestCommon.kt (1)
runTestWithRealTime
(35-41)ktor-client/ktor-client-webrtc/js/test/io/ktor/client/webrtc/WebRTCEngineTest.js.kt (1)
createTestWebRTCClient
(7-12)ktor-client/ktor-client-webrtc/wasmJs/test/io/ktor/client/webrtc/WebRTCEngineTest.wasmJs.kt (1)
createTestWebRTCClient
(7-12)
ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/utils/JsUtils.kt (3)
ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/peer/RTCPeerConnection.kt (1)
init
(526-541)ktor-client/ktor-client-webrtc/wasmJs/src/io/ktor/client/webrtc/peer/RTCPeerConnection.kt (1)
init
(529-546)ktor-client/ktor-client-webrtc/wasmJs/src/io/ktor/client/webrtc/utils/JsUtils.kt (2)
jsObject
(19-23)getValues
(90-90)
ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRTCEngine.kt (3)
ktor-client/ktor-client-webrtc/android/src/io/ktor/client/webrtc/WebRTCEngine.android.kt (1)
ioDispatcher
(10-10)ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/WebRTCEngine.jsAndWasmShared.kt (1)
ioDispatcher
(10-10)ktor-client/ktor-client-webrtc/jvmAndPosix/src/io/ktor/client/webrtc/WebRTCEngine.posix.kt (1)
ioDispatcher
(11-11)
🪛 detekt (1.23.8)
ktor-client/ktor-client-webrtc/android/src/io/ktor/client/webrtc/Utils.kt
[warning] 120-120: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
[warning] 121-121: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
🔇 Additional comments (77)
.idea/dictionaries/oleh_pantus.xml (1)
1-7
: Skip IDE dictionary file review
This IntelliJ IDEA project dictionary file is unrelated to the WebRTC functionality and its contents (custom word "playout") look correct.gradle.properties (1)
70-70
: Enable AndroidX support
Addingandroid.useAndroidX=true
is required for AndroidX libraries in the new Android WebRTC engine.build-logic/build.gradle.kts (1)
18-18
: Add Android Gradle Plugin dependency
Includinglibs.android.gradlePlugin
ensures the Android JVM target configurations can resolve the Android Gradle Plugin artifacts.build-settings-logic/build.gradle.kts (1)
17-17
: Add Android Gradle Plugin to settings logic
This dependency allows managing the Android Gradle Plugin within the settings build logic, aligning with the multiplatform Android support.ktor-test-server/settings.gradle.kts (1)
9-9
: Include Google's Maven repository for plugin resolution
Addinggoogle()
ensures Android-related plugins (e.g., the Android Gradle Plugin) are resolvable.ktor-client/ktor-client-webrtc/gradle.properties (1)
1-2
: Disable Node.js targets for JS and Wasm builds.These properties explicitly turn off Node.js as a compilation target, ensuring the module is built only for browser and WebAssembly environments.
build-settings-logic/src/main/kotlin/ktorsettings.dependency-resolution-management.settings.gradle.kts (1)
33-36
: Include AndroidX dependencies from Google repository.Adding
includeGroupAndSubgroups("androidx")
ensures that AndroidX artifacts are served from Google’s Maven repository alongsidecom.google
andcom.android
.ktor-client/ktor-client-webrtc/AndroidManifest.xml (1)
7-13
: Declare necessary permissions and camera feature for WebRTC on Android.This manifest correctly requests CAMERA, RECORD_AUDIO, INTERNET, and NETWORK_STATE permissions, and declares the camera hardware feature required by the Android WebRTC engine.
ktor-client/ktor-client-webrtc/android/src/io/ktor/client/webrtc/WebRTCEngine.android.kt (1)
10-10
: Provide Android-specific IO dispatcher.The
actual fun ioDispatcher()
implementation returnsDispatchers.IO
, which is appropriate for offloading blocking I/O work on Android platforms.ktor-client/ktor-client-webrtc/jvmAndPosix/src/io/ktor/client/webrtc/WebRTCEngine.posix.kt (1)
7-11
: LGTM! Correct platform-specific dispatcher implementation.The use of
Dispatchers.IO
is appropriate for JVM and POSIX platforms for IO-bound WebRTC operations. The implementation follows proper Kotlin multiplatform conventions.settings.gradle.kts (2)
12-12
: LGTM! Google repository addition is necessary for Android support.Adding the Google Maven repository to plugin management repositories is required for resolving Android Gradle Plugin dependencies, which is essential for the WebRTC module's Android target.
130-131
: LGTM! Proper module integration.The addition of
ktor-client-webrtc
module to the client projects list follows the existing project structure conventions and is positioned appropriately among other client modules.ktor-client/ktor-client-webrtc/jsAndWasmShared/test/io/ktor/client/webrtc/WebRTCEngineTest.jsAndWasmShared.kt (1)
7-7
: LGTM! Proper test utility delegation.The actual function implementation correctly delegates permission granting to the
MockMediaTrackFactory
, following the multiplatform test infrastructure pattern.ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/WebRTCEngine.jsAndWasmShared.kt (1)
7-10
: LGTM! Appropriate dispatcher choice for JS/Wasm platforms.Using
Dispatchers.Default
is correct for JavaScript and WebAssembly platforms since they don't have separate IO dispatchers like JVM/POSIX platforms. This platform-specific implementation follows proper Kotlin multiplatform conventions.ktor-client/ktor-client-webrtc/wasmJs/test/io/ktor/client/webrtc/WebRTCEngineTest.wasmJs.kt (1)
7-12
: LGTM! Consistent wasmJs test implementation.The implementation correctly follows the established pattern from other platform implementations (JS, Android) and uses appropriate test configuration with
MockMediaTrackFactory
for the wasmJs platform.build-logic/settings.gradle.kts (2)
8-8
: LGTM! Repository addition supports Android development.Adding the
google()
repository to pluginManagement is necessary for resolving Android Gradle Plugin and related tooling required for the new Android WebRTC implementation.
27-29
: LGTM! Dependency repository configuration is appropriate.The addition of
mavenCentral()
andgoogle()
repositories enables proper resolution of Android and AndroidX dependencies needed for the WebRTC Android implementation. The repository order is correct.build-settings-logic/settings.gradle.kts (2)
5-11
: LGTM! Plugin management configuration supports Android development.The new
pluginManagement
block correctly includes all necessary repositories (google()
,mavenCentral()
,gradlePluginPortal()
) in the appropriate order to support Android Gradle Plugin resolution for the WebRTC Android implementation.
33-33
: LGTM! Repository addition aligns with Android requirements.Adding the
google()
repository to dependency resolution management is consistent with the build-logic configuration and necessary for AndroidX dependencies used in the WebRTC Android implementation.ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRTCPermissionTest.kt (2)
10-12
: LGTM! Well-designed test isolation strategy.The comment clearly explains the rationale for separating this test to avoid race conditions with global permission state, which is a thoughtful approach to test isolation.
13-18
: LGTM! Comprehensive permission denial test.The test correctly validates that attempting to create an audio track without proper permissions throws a
WebRTCMedia.PermissionException
. The test configuration (audio=false, video=false, realTime=false) and assertion logic are appropriate for testing permission enforcement across platforms.ktor-client/ktor-client-webrtc/js/test/io/ktor/client/webrtc/WebRTCEngineTest.js.kt (1)
7-12
: LGTM! Clean test client setup for JavaScript platform.The implementation correctly configures a test WebRTC client using the
JsWebRTC
engine with appropriate test settings. The use ofMockMediaTrackFactory
and empty server lists is perfect for testing scenarios.ktor-client/ktor-client-webrtc/jvmAndPosix/test/io/ktor/client/webrtc/WebRTCEngineTest.jvmAndPosix.kt (1)
7-13
: LGTM! Appropriate stub implementation for unsupported platforms.Using
NotImplementedError
with clear error messages is the correct approach for platforms that don't support WebRTC client functionality. This maintains the expected function signatures while clearly indicating the lack of support.ktor-client/ktor-client-webrtc/wasmJs/test/io/ktor/client/webrtc/MockMediaDevices.wasmJs.kt (3)
9-17
: LGTM! Proper Web Audio API usage for dummy audio stream.The JavaScript interop correctly creates a dummy audio stream using the Web Audio API with an oscillator connected to a MediaStreamDestination. This provides a suitable mock audio source for testing.
19-29
: LGTM! Correct canvas-based video stream generation.The implementation properly creates a dummy video stream by drawing on a canvas and capturing its stream. This provides a suitable mock video source for testing without requiring actual camera access.
31-42
: LGTM! Proper track wrapping implementation.Both functions correctly extract the first track from the generated streams and wrap them in the appropriate platform-specific track classes (
WasmJsAudioTrack
andWasmJsVideoTrack
).ktor-client/ktor-client-webrtc/androidInstrumented/test/io/ktor/client/webrtc/WebRTCEngineTest.android.kt (3)
12-12
: LGTM! Proper context retrieval for instrumented tests.Using
InstrumentationRegistry.getInstrumentation().targetContext
is the standard approach for getting the app context in Android instrumented tests.
14-21
: LGTM! Consistent test client configuration for Android.The implementation follows the same pattern as other platforms while using Android-specific components (
AndroidWebRTC
engine andAndroidMediaDevices
with context). The test configuration is appropriate for instrumented testing.
23-35
: LGTM! Proper permission handling for Android testing.The implementation correctly uses
UiAutomation
to programmatically grant or revoke runtime permissions for both audio recording and camera access. This is the appropriate approach for instrumented tests that need to control permissions dynamically.ktor-client/ktor-client-webrtc/build.gradle.kts (1)
62-66
:✅ Verification successful
Verify Android SDK version compatibility.
The configuration uses
compileSdk = 35
andminSdk = 28
. Please ensure that SDK 35 is available and that the feature set used is compatible with the minimum SDK 28.
🏁 Script executed:
#!/bin/bash # Check if the WebRTC Android library supports the specified SDK versions echo "Checking Android SDK compatibility for WebRTC implementation" # Look for any SDK version constraints in the WebRTC dependency rg -A 5 -B 5 "compileSdk|minSdk|targetSdk" --type gradle # Check for any Android-specific WebRTC usage that might require higher SDK versions rg -A 3 "android.*webrtc|webrtc.*android" --ignore-caseLength of output: 19504
Ensure Android SDK 35 is installed and avoid preview APIs
The WebRTC Android client (io.getstream:stream-webrtc-android 1.3.8) does not impose explicit SDK constraints and supports API 21+. With your
minSdk=28
, this configuration is safe. Please verify:
- Android SDK 35 is available on all local and CI build environments
- You’re not inadvertently using any Android U (API 35) preview APIs
- If you don’t need preview features, consider switching to
compileSdk=34
to target the latest stable SDKktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/Engine.kt (1)
11-13
:✅ Verification successful
Verify expected/actual implementation pattern.
Ensure that this expected object has corresponding actual implementations in both JS and Wasm source sets.
🏁 Script executed:
#!/bin/bash # Check for actual implementations of JsWebRTC in JS and Wasm source sets echo "Searching for actual implementations of JsWebRTC object..." # Look for actual implementations rg -A 5 "actual object JsWebRTC" # Verify the implementations exist in the correct source sets fd -t f "Engine.*\.kt" | grep -E "(js|wasm)" | xargs grep -l "JsWebRTC"Length of output: 1731
Actual implementations for
JsWebRTC
confirmed
Theexpected
object injsAndWasmShared
has matchingactual
implementations in both JS and Wasm source sets:
- ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/Engine.js.kt
- ktor-client/ktor-client-webrtc/wasmJs/src/io/ktor/client/webrtc/Engine.wasmJs.kt
ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/peer/LibDom.kt (1)
54-58
:❓ Verification inconclusive
Verify potential conflicts with Kotlin/JS standard library.
Check if these declarations conflict with existing Navigator or MediaDevices declarations in the Kotlin/JS standard library.
🏁 Script executed:
#!/bin/bash # Check for potential conflicts with Kotlin/JS standard library echo "Checking for Navigator and MediaDevices in Kotlin/JS stdlib..." # Look for existing Navigator declarations in dependencies rg -A 3 -B 3 "external.*Navigator|interface Navigator" --type kotlin # Check for MediaDevices usage in the codebase rg -A 3 "MediaDevices" --type kotlinLength of output: 7849
Verify potential name collision with Kotlin/JS stdlib DOM types
I didn’t find any existing Navigator or MediaDevices declarations within this codebase, but the Kotlin/JS standard library already provides external definitions for org.w3c.dom.Navigator and org.w3c.dom.mediacapture.MediaDevices. Please confirm that these custom declarations won’t shadow or conflict with the stdlib types, or consider reusing the stdlib definitions instead:
• ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/peer/LibDom.kt
• ktor-client/ktor-client-webrtc/wasmJs/src/io/ktor/client/webrtc/peer/LibDom.ktktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRTCClient.kt (2)
13-15
: LGTM! Clean factory interface design.The
WebRTCClientEngineFactory
interface follows a clean factory pattern with proper type constraints and default parameter handling.
48-52
: LGTM! Well-designed factory function.The factory function properly uses the
@KtorDsl
annotation and follows Ktor's API patterns with clean type constraints and delegation to the factory's create method.ktor-client/ktor-client-webrtc/wasmJs/src/io/ktor/client/webrtc/peer/LibDom.kt (4)
9-9
: LGTM! Appropriate type alias for undefined values.The
JsUndefined
type alias correctly maps toJsAny?
for handling undefined values in WebAssembly/JS interop.
11-40
: LGTM! Comprehensive DOMException implementation.The
DOMException
external class declaration includes all standard DOM error codes and follows proper WebAssembly/JS interop patterns withJsNumber
andJsString
types.
42-54
: LGTM! Well-structured ReadonlyMap interface.The
ReadonlyMap
interface properly defines all necessary methods with correct type parameters and optional arguments usingdefinedExternally
.
56-60
: LGTM! Clean Navigator interface declaration.The
Navigator
interface and globalnavigator
declaration correctly expose themediaDevices
property needed for WebRTC media access.ktor-client/ktor-client-webrtc/jsAndWasmShared/test/io/ktor/client/webrtc/MockMediaDevices.kt (4)
3-10
: LGTM! Clean permission management design.The
MockMediaTrackFactory
object properly encapsulates permission state with private fields and a public method for granting permissions, which is ideal for testing scenarios.
12-15
: LGTM! Appropriate permission checking and delegation.The
createAudioTrack
method correctly checks permissions before delegating to the platform-specific implementation, throwing the appropriate exception type when permission is denied.
17-21
: LGTM! Good default handling for video constraints.The
createVideoTrack
method appropriately handles optional width/height constraints with sensible defaults (100x100) and follows the same permission checking pattern as the audio track method.
23-25
: LGTM! Clean expected function declarations.The expected functions for creating dummy tracks are well-defined with appropriate parameters, allowing platform-specific implementations to provide mock media tracks for testing.
gradle/libs.versions.toml (3)
91-94
: LGTM! Proper library declarations for Android support.The Android Gradle plugin and AndroidX testing library declarations are correctly formatted and reference the appropriate version variables.
154-154
: LGTM! Correct Stream WebRTC Android library declaration.The Stream WebRTC Android library declaration correctly references the version variable and uses the proper module coordinates.
32-32
:✅ Verification successful
Verify Stream WebRTC Android library version and security.
The Stream WebRTC Android library version 1.3.8 should be checked for being the latest version and any known security vulnerabilities.
🌐 Web query:
What is the latest version of Stream WebRTC Android library (io.getstream:stream-webrtc-android) and are there any known security issues with version 1.3.8?
💡 Result:
The latest version of Stream WebRTC Android library (io.getstream:stream-webrtc-android) is version 1.3.8, which was released on February 21, 2025[7]. This library is a versatile WebRTC pre-compiled Android library that reflects recent WebRTC updates to facilitate real-time video chat for Android and Compose applications[1].
Regarding security issues with version 1.3.8, the search results do not specifically mention any known vulnerabilities for this particular version. The Maven repository information for Stream WebRTC Android shows version 1.3.8 in their listing, but does not indicate any vulnerabilities in the "Vulnerabilities" column for this version[7].
The library was developed by the Stream team to address challenges in integrating WebRTC into Android projects. Google's original WebRTC library for Android ceased maintenance in 2018, leaving developers without an official pathway for integration[2]. Stream's solution was created specifically to be integrated into their Video SDK for Android, providing features like simulcast support[2].
For context, WebRTC security is a significant concern in the industry. While there have been general WebRTC vulnerabilities reported, such as the TURN relay abuse vulnerability mentioned in one of the search results[6], there is no specific information indicating that Stream's WebRTC Android library version 1.3.8 is affected by any known security issues.
The Stream WebRTC Android library continues to receive regular updates, with version 1.3.8 including dependency bumps for various components including:
- androidx.compose:compose-bom from 2025.01.00 to 2025.01.01 and later to 2025.02.00
- androidxMacroBenchmark from 1.4.0-alpha06 to 1.4.0-alpha07
- androidx.navigation:navigation-compose from 2.8.5 to 2.8.6[3]
Citations:
- 1: https://github.com/GetStream/webrtc-android
- 2: https://getstream.io/resources/projects/webrtc/library/android/
- 3: https://github.com/GetStream/webrtc-android/releases
- 4: https://mvnrepository.com/open-source/webrtc
- 5: https://mvnrepository.com/artifact/io.getstream/stream-webrtc-android-ui
- 6: https://www.youtube.com/watch?v=qlQJuyp7nS8
- 7: https://mvnrepository.com/artifact/io.getstream/stream-webrtc-android
- 8: https://docs.aws.amazon.com/kinesisvideostreams-webrtc-dg/latest/devguide/kvswebrtc-sdk-android.html
- 9: https://www.audiocodes.com/media/zwqljlnp/webrtc-android-client-release-notes.pdf
- 10: https://mvnrepository.com/artifact/io.getstream/stream-webrtc-android/1.3.7
Stream WebRTC Android library version confirmed
- io.getstream:stream-webrtc-android 1.3.8 is the latest release (published Feb 21, 2025).
- No known security vulnerabilities have been reported for version 1.3.8.
ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRTC.kt (2)
9-94
: Well-structured WebRTC abstractions!The enums and data classes follow WebRTC standards closely and use proper Kotlin conventions. The
isSuccessful()
helper method is a nice touch for checking ICE connection status.
160-168
:⚠️ Potential issueMutable field in data class breaks immutability.
The
AudioTrackConstraints
data class has a mutablevolume
field (line 164) while all other fields useval
. This breaks the immutability expectation of data classes and could lead to unexpected behavior.Apply this diff to fix the inconsistency:
- var volume: Double? = null, + val volume: Double? = null,Likely an incorrect or invalid review comment.
ktor-client/ktor-client-webrtc/android/src/io/ktor/client/webrtc/engine/MediaDevices.kt (2)
30-88
: Well-structured initialization with proper lazy loading!The class properly initializes WebRTC components with lazy loading for performance, and includes appropriate Android version checks for hardware acceleration features.
184-197
: Consistent implementation with proper resource management.The video track creation follows the same pattern as audio track creation with appropriate permission checks and resource disposal.
build-logic/src/main/kotlin/ktorbuild/targets/KtorTargets.kt (1)
80-80
: Clean Android target integration!The Android JVM target support is well-integrated following the existing patterns. The conditional check for the Android library plugin ensures the target is only added when appropriate.
Also applies to: 174-176, 188-190
ktor-client/ktor-client-webrtc/android/src/io/ktor/client/webrtc/Utils.kt (3)
16-26
: Well-implemented type conversions.The session description type conversions properly handle all WebRTC session description types with correct mapping between common and Android native enums.
109-116
: Empty methods are intentional and necessary.The static analysis tool flagged empty method implementations in the SDP observer, but these are intentionally empty because this observer only handles creation callbacks, not set callbacks. The empty implementations satisfy the interface requirements while maintaining focus on the specific use case.
The empty
onSetSuccess()
andonSetFailure()
methods are correct for this specific SDP observer that only handles creation operations.
118-125
: Consistent SDP observer pattern.The
resumeAfterSdpSet()
function follows the same pattern asresumeAfterSdpCreate()
but handles set operations instead of create operations. The design correctly separates concerns between different SDP operations.🧰 Tools
🪛 detekt (1.23.8)
[warning] 120-120: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
[warning] 121-121: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
ktor-client/ktor-client-webrtc/wasmJs/src/io/ktor/client/webrtc/utils/JsUtils.kt (2)
19-23
: Excellent null-safe jsObject utility.The
jsObject
helper function properly creates JavaScript objects in the WASM environment using the separateemptyObject()
function and proper type casting. This is a clean pattern for WASM JS interop.
25-47
: Improved null safety in constraint conversions.The constraint conversion functions properly handle nullable values using safe calls and appropriate JS type conversions (
toJsNumber()
,toJsBoolean()
), which is a significant improvement over the JS version.ktor-client/ktor-client-webrtc/android/src/io/ktor/client/webrtc/peer/LibAndroid.kt (3)
51-55
: LGTM!Clean and simple implementations of audio and video track wrappers.
93-115
: LGTM!The implementation correctly maps native RTP parameters. The fallback to
DISABLED
for unknown degradation preferences is a reasonable default.
66-68
:⚠️ Potential issueAdd null-safe type checking for track parameter.
The unsafe cast could throw
ClassCastException
if a non-Android implementation is passed.override suspend fun replaceTrack(withTrack: WebRTCMedia.Track?) { - nativeRtpSender.setTrack((withTrack as? AndroidMediaTrack)?.nativeTrack, false) + val nativeTrack = when (withTrack) { + null -> null + is AndroidMediaTrack -> withTrack.nativeTrack + else -> throw IllegalArgumentException("Expected AndroidMediaTrack but got ${withTrack::class.simpleName}") + } + nativeRtpSender.setTrack(nativeTrack, false) }Likely an incorrect or invalid review comment.
ktor-client/ktor-client-webrtc/jsAndWasmShared/src/io/ktor/client/webrtc/Utils.kt (2)
7-40
: LGTM!The enum-to-string conversion functions are well-implemented with consistent patterns and comprehensive coverage.
102-116
: LGTM!The exception wrapper functions are well-designed using inline functions for zero overhead and providing clear error context.
ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRTCEngine.kt (3)
37-40
: LGTM!Clean and well-designed interface for creating media tracks with appropriate use of suspend functions.
61-64
: LGTM!Clean sealed class design for representing track operations.
59-59
: LGTM!Good use of expect/actual pattern for platform-specific dispatcher implementations. The actual implementations correctly use
Dispatchers.IO
for JVM/Android/POSIX andDispatchers.Default
for JS/Wasm where IO dispatcher is not available.ktor-client/ktor-client-webrtc/wasmJs/src/io/ktor/client/webrtc/Engine.wasmJs.kt (1)
45-70
: LGTM!The WebRTC engine implementation is well-structured with proper configuration mapping and peer connection creation.
ktor-client/ktor-client-webrtc/android/src/io/ktor/client/webrtc/engine/Engine.kt (1)
38-44
: Good error handling for custom MediaTrackFactory.The error message clearly indicates what needs to be done when using a custom MediaTrackFactory.
ktor-client/ktor-client-webrtc/common/test/io/ktor/client/webrtc/WebRTCEngineTest.kt (2)
219-292
: Excellent comprehensive test for peer connection establishment.This test thoroughly validates state transitions, ICE negotiation, and connection establishment with proper timeout handling and async state monitoring.
348-407
: Well-structured test for remote track handling.The test properly validates track addition, removal, and replay functionality with appropriate assertions and timeout handling.
ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/Engine.js.kt (4)
18-27
: LGTM! Good error handling pattern.The function properly catches and transforms permission errors into domain-specific exceptions.
45-78
: Well-structured WebRTC engine implementation.The engine properly configures RTCPeerConnection with all necessary parameters and correctly maps ICE servers.
197-232
: Clean media track implementation.The abstract class and its concrete implementations properly wrap native media tracks with appropriate type safety.
254-289
: Remaining implementations look good.The DTMF sender, RTP parameters, and factory implementations are correct and follow proper patterns.
ktor-client/ktor-client-webrtc/api/jvm/ktor-client-webrtc.api (1)
1-481
: Comprehensive and well-structured WebRTC API.The API definition provides a complete and type-safe interface for WebRTC functionality with:
- Proper serialization support for data transfer objects
- Comprehensive enum definitions for all WebRTC states and policies
- Clear separation of concerns between media, connection, and configuration components
- Appropriate exception hierarchy for error handling
The API design follows Kotlin best practices and aligns well with WebRTC standards.
ktor-client/ktor-client-webrtc/wasmJs/src/io/ktor/client/webrtc/peer/RTCPeerConnection.kt (1)
1-801
: Comprehensive WebRTC type definitions.The external declarations provide excellent type-safe bindings for the WebRTC browser APIs with:
- Proper use of Kotlin/JS specific types (JsString, JsNumber, JsArray, etc.)
- Correct Promise-based API declarations
- Comprehensive coverage of all WebRTC interfaces and events
- Appropriate nullable types and default parameters
These bindings will enable safe WebRTC usage from Kotlin/JS code.
ktor-client/ktor-client-webrtc/api/android/ktor-client-webrtc.api (4)
5-8
: Well-designed factory interface for media tracks.The
MediaTrackFactory
interface follows good design patterns with:
- Clear separation of audio and video track creation
- Proper use of suspend functions for potentially async operations
- Type-safe constraint parameters
19-33
: Well-structured conversion utilities.The utility functions provide clean bidirectional conversions between native Android WebRTC types and Ktor's common types. The consistent naming convention (
toCommon
/toNative
) makes the API intuitive.
277-341
: Excellent architectural design for WebRTC client.The client architecture demonstrates several best practices:
- Clean separation of concerns with Client/Engine/Config
- Proper use of delegation pattern
- Integration with Kotlin coroutines
- Factory pattern for platform-specific implementations
464-496
: Excellent reactive design with Kotlin Flow.The
WebRtcPeerConnection
class demonstrates best practices for reactive programming:
- StateFlow for observable state (connection, ICE, signaling states)
- SharedFlow for event streams (ICE candidates, remote tracks)
- Proper encapsulation with protected mutable flows
- Comprehensive API coverage for WebRTC operations
This design will provide a smooth developer experience with reactive state updates.
@file:OptIn(ExperimentalWasmDsl::class) | ||
|
||
import org.jetbrains.kotlin.gradle.ExperimentalWasmDsl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify experimental Wasm DSL stability.
The use of @file:OptIn(ExperimentalWasmDsl::class)
indicates this feature is still experimental. Consider documenting this limitation and ensure compatibility with the target Kotlin version.
🌐 Web query:
What is the current stability status of ExperimentalWasmDsl in Kotlin multiplatform?
💡 Result:
The ExperimentalWasmDsl
in Kotlin Multiplatform is currently in the Experimental stability phase. This means it is intended for trial use only, and its API and behavior may change without notice. Users are encouraged to provide feedback, but should be aware that the feature is not yet stable and could be deprecated or removed in future releases. (kotlinlang.org)
As of May 2025, Kotlin Multiplatform has made significant progress, with many components reaching stable status. However, support for WebAssembly (Wasm) remains in the experimental phase, reflecting ongoing development and testing. (jetbrains.com)
For the most current information on the stability status of ExperimentalWasmDsl
and other Kotlin Multiplatform components, it's advisable to consult the official Kotlin documentation and release notes.
Citations:
- 1: https://kotlinlang.org/docs/components-stability.html?utm_source=openai
- 2: https://www.jetbrains.com/help/kotlin-multiplatform-dev/supported-platforms.html?utm_source=openai
Document and Monitor ExperimentalWasmDsl Usage
The ExperimentalWasmDsl
API is confirmed to remain in the Experimental stability phase as of May 2025 ([1], [2]). Its behavior and surface may change without notice. Please:
- Add a brief note in
ktor-client/ktor-client-webrtc/build.gradle.kts
(around lines 5–7) or your project README indicating that Wasm DSL support is experimental. - Specify the minimum Kotlin/Gradle plugin version known to work with this DSL.
- Plan to review Kotlin release notes regularly for any changes or deprecations to
ExperimentalWasmDsl
.
Files to update:
ktor-client/ktor-client-webrtc/build.gradle.kts
(lines 5–7)- Project-level documentation (e.g.,
README.md
)
Citations:
- https://kotlinlang.org/docs/components-stability.html
- https://www.jetbrains.com/help/kotlin-multiplatform-dev/supported-platforms.html
🤖 Prompt for AI Agents
In ktor-client/ktor-client-webrtc/build.gradle.kts around lines 5 to 7, add a
brief comment noting that the use of ExperimentalWasmDsl is an experimental
feature whose API and behavior may change. Also, specify the minimum Kotlin and
Gradle plugin versions compatible with this DSL. Additionally, update the
project README or relevant documentation to include this information and plan to
monitor Kotlin release notes for any future changes or deprecations related to
ExperimentalWasmDsl.
actual fun makeDummyAudioStreamTrack(): WebRTCMedia.AudioTrack { | ||
val ctx = AudioContext() | ||
val oscillator = ctx.createOscillator() | ||
val dst = oscillator.connect(ctx.createMediaStreamDestination()) | ||
oscillator.start() | ||
return JsAudioTrack(dst.stream.getAudioTracks()[0], dst.stream) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider adding resource cleanup for audio context.
The audio context and oscillator are created but never explicitly cleaned up. This could lead to resource leaks in test environments.
actual fun makeDummyAudioStreamTrack(): WebRTCMedia.AudioTrack {
val ctx = AudioContext()
val oscillator = ctx.createOscillator()
val dst = oscillator.connect(ctx.createMediaStreamDestination())
oscillator.start()
- return JsAudioTrack(dst.stream.getAudioTracks()[0], dst.stream)
+ val track = JsAudioTrack(dst.stream.getAudioTracks()[0], dst.stream)
+ // TODO: Consider adding cleanup mechanism for audio context and oscillator
+ return track
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
actual fun makeDummyAudioStreamTrack(): WebRTCMedia.AudioTrack { | |
val ctx = AudioContext() | |
val oscillator = ctx.createOscillator() | |
val dst = oscillator.connect(ctx.createMediaStreamDestination()) | |
oscillator.start() | |
return JsAudioTrack(dst.stream.getAudioTracks()[0], dst.stream) | |
} | |
actual fun makeDummyAudioStreamTrack(): WebRTCMedia.AudioTrack { | |
val ctx = AudioContext() | |
val oscillator = ctx.createOscillator() | |
val dst = oscillator.connect(ctx.createMediaStreamDestination()) | |
oscillator.start() | |
val track = JsAudioTrack(dst.stream.getAudioTracks()[0], dst.stream) | |
// TODO: Consider adding cleanup mechanism for audio context and oscillator | |
return track | |
} |
🤖 Prompt for AI Agents
In
ktor-client/ktor-client-webrtc/js/test/io/ktor/client/webrtc/MockMediaDevices.js.kt
around lines 11 to 17, the AudioContext and oscillator are created but not
cleaned up, risking resource leaks. Modify the function to stop the oscillator
and close the AudioContext after use or provide a mechanism to clean them up
when the dummy audio stream track is no longer needed.
public open external class DOMException { | ||
public var code: Number | ||
public var message: String | ||
public var name: String | ||
public var ABORT_ERR: Number | ||
public var DATA_CLONE_ERR: Number | ||
public var DOMSTRING_SIZE_ERR: Number | ||
public var HIERARCHY_REQUEST_ERR: Number | ||
public var INDEX_SIZE_ERR: Number | ||
public var INUSE_ATTRIBUTE_ERR: Number | ||
public var INVALID_ACCESS_ERR: Number | ||
public var INVALID_CHARACTER_ERR: Number | ||
public var INVALID_MODIFICATION_ERR: Number | ||
public var INVALID_NODE_TYPE_ERR: Number | ||
public var INVALID_STATE_ERR: Number | ||
public var NAMESPACE_ERR: Number | ||
public var NETWORK_ERR: Number | ||
public var NOT_FOUND_ERR: Number | ||
public var NOT_SUPPORTED_ERR: Number | ||
public var NO_DATA_ALLOWED_ERR: Number | ||
public var NO_MODIFICATION_ALLOWED_ERR: Number | ||
public var QUOTA_EXCEEDED_ERR: Number | ||
public var SECURITY_ERR: Number | ||
public var SYNTAX_ERR: Number | ||
public var TIMEOUT_ERR: Number | ||
public var TYPE_MISMATCH_ERR: Number | ||
public var URL_MISMATCH_ERR: Number | ||
public var VALIDATION_ERR: Number | ||
public var WRONG_DOCUMENT_ERR: Number | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify DOMException declaration against web standards.
The DOMException declaration includes all error codes as instance properties, but these should typically be static constants. Please verify this matches the actual browser implementation.
🌐 Web query:
How are DOMException error codes typically defined in the Web API? Are they instance properties or static constants?
💡 Result:
How DOMException Error Codes Are Defined in the Web API
DOMException error codes in the Web API are a legacy mechanism, and their definition and usage have changed over time as web standards evolved.
Error Code Definitions
Historically, DOMException error codes were defined as named integer constants (such as INDEX_SIZE_ERR = 1
), typically attached to the DOMException
type as static constants. These error codes represented different types of exceptional conditions encountered in DOM operations. For example, INDEX_SIZE_ERR
with code value 1
indicated an invalid index, and so on[6].
Instance Properties vs. Static Constants
-
Instance Property:
TheDOMException
interface exposes a read-only instance property namedcode
. This property returns one of the legacy error code constants, or0
if none match. Importantly, this property is now deprecated and maintained only for compatibility with legacy code[1][6]. -
Static Constants:
The legacy error code constants themselves (such asDOMException.INDEX_SIZE_ERR
) used to be defined as static constants on theDOMException
constructor. These are not properties of individual exception instances, but rather are accessible on theDOMException
class. For example:let code = DOMException.INDEX_SIZE_ERR;
However, modern web standards and browsers are moving away from this approach. Official specifications and best practices now favor using string names (e.g.,
IndexSizeError
) via the.name
property of DOMException[6][9].
Key Takeaways in Practice
- Error codes are legacy features and are available as static constants on the DOMException class, not on individual instances, though you access the error code of a specific exception via its
.code
instance property[1][6][9]. - Newer exceptions rely on the
.name
string property rather than integer codes, reflecting a shift in the standards[6][9]. - The use of error code constants and the
.code
property is officially deprecated and should be avoided in favor of.name
for new code[1][6].
Summary Table: DOMException Error Codes
Feature | Static Constant | Instance Property | Modern Usage |
---|---|---|---|
Legacy error codes | Yes (e.g., DOMException.INDEX_SIZE_ERR) | Yes (exception.code ) |
Deprecated |
Modern error identification | No | Yes (exception.name ) |
Recommended |
References in Current Standards
"The code read-only property of the DOMException interface returns one of the legacy error code constants, or 0 if none match. ... New DOM exceptions don't use this anymore: they put this info in the DOMException.name attribute."[1]
"Note that the following deprecated historical errors don't have an error name but instead have only a legacy constant code value and a legacy constant name... Because historically the errors were identified by a numeric value that corresponded with a named variable defined to have that value, some of the entries below indicate the legacy code value and constant name that were used in the past."[6]
"Constants are not associated with particular instances of the interface or callback interface on which they appear. ... The Web platform is moving away from this design pattern in favor of the use of strings."[9]
In summary:
DOMException error codes are legacy static constants on the DOMException class, with their current value exposed via the instance .code
property. Both approaches are deprecated. Modern web APIs favor string-based error identification using the .name
property[1][6][9].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/DOMException/code
- 2: https://reference.aspose.com/html/net/aspose.html.dom/domexception/
- 3: https://docs.oracle.com/javase/8/docs/jre/api/plugin/dom/org/w3c/dom/DOMException.html
- 4: https://forum.lightningjs.io/t/domexception-a-network-error-occurred/324
- 5: https://learn.microsoft.com/en-us/dotnet/api/org.w3c.dom.domexception?view=net-android-35.0
- 6: https://developer.mozilla.org/en-US/docs/Web/API/DOMException
- 7: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error
- 8: https://www.zipy.ai/blog/debug-javascript-domexception
- 9: https://webidl.spec.whatwg.org
- 10: https://kristoffer-strube.dk/post/typed-exceptions-for-jsinterop-in-blazor/
Align DOMException Error Codes with Web Standards
The current external declaration lists each legacy error code as an instance property, but per the Web API these must be static constants on DOMException
(with instances exposing only the read-only code
property). Please update the Kotlin binding accordingly.
• File to update:
ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/peer/LibDom.kt
• Changes needed:- Remove all
var XXX_ERR: Number
from the instance body. - Declare only
val code: Int
,val message: String
,val name: String
as instance properties. - Add a
companion object
(or@JsName
static extern declarations) containing the legacy constants:public open external class DOMException(message: String, name: String = definedExternally) { public val code: Int public val message: String public val name: String public companion object { val ABORT_ERR: Int val DATA_CLONE_ERR: Int val DOMSTRING_SIZE_ERR: Int // …and so on for all legacy codes } }
• Optional: Mark code
as deprecated in KDoc and encourage use of name
for modern error handling.
🤖 Prompt for AI Agents
In ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/peer/LibDom.kt
between lines 9 and 38, the DOMException class incorrectly declares all legacy
error codes as instance properties. To fix this, remove all the var declarations
for error codes from the instance body, keep only val code: Int, val message:
String, and val name: String as instance properties, and move all legacy error
code constants into a companion object as static constants. Optionally, add KDoc
to deprecate the code property and recommend using the name property for modern
error handling.
gradle/libs.versions.toml
Outdated
android-gradle-plugin = "8.9.2" | ||
androidx = "1.6.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify Android Gradle Plugin and AndroidX versions are current.
The Android Gradle Plugin version 8.9.2 and AndroidX version 1.6.1 should be verified for currency and compatibility.
🌐 Web query:
What are the latest stable versions of Android Gradle Plugin and AndroidX Test libraries as of 2025?
💡 Result:
As of May 30, 2025, the latest stable versions of the Android Gradle Plugin (AGP) and AndroidX Test libraries are:
Android Gradle Plugin (AGP):
- Version: 8.9.0
- Release Date: March 7, 2025
- Key Features:
- Supports Gradle 8.11.1
- Compatible with SDK Build Tools 35.0.0
- Requires JDK 17
- Fixes various issues, including providing actionable error messages for GMD setup task failures due to insufficient disk space.
- Release Notes: (developer.android.com)
AndroidX Test Libraries:
-
Core Library:
- Version: 1.7.0-alpha01
- Release Date: February 3, 2025
- Key Features:
- Introduces new APIs for testing, including
View.captureToBitmap
andWindowCapture.captureToBitmap
for enhanced screenshot capabilities. - Updates minimum SDK version to 21.
- Introduces new APIs for testing, including
- Release Notes: (developer.android.com)
-
Espresso Library:
- Version: 3.7.0-alpha01
- Release Date: February 3, 2025
- Key Features:
- Fixes deadlock issues in Espresso when used with Robolectric in instrumentation tests.
- Refactors message queue access to improve test stability.
- Updates minimum SDK version to 21.
- Release Notes: (developer.android.com)
-
JUnit Extensions:
- Version: 1.3.0-alpha01
- Release Date: February 3, 2025
- Key Features:
- Updates minimum SDK version to 21.
- Release Notes: (developer.android.com)
-
Truth Extensions:
- Version: 1.7.0-alpha01
- Release Date: February 3, 2025
- Key Features:
- Updates minimum SDK version to 21.
- Release Notes: (developer.android.com)
Please note that while these versions are the latest stable releases as of May 30, 2025, it's advisable to check the official Android Developers website for the most current information, as new versions may have been released since then.
Citations:
- 1: https://developer.android.com/build/releases/gradle-plugin?utm_source=openai
- 2: https://developer.android.com/jetpack/androidx/releases/test?utm_source=openai
- 3: https://developer.android.com/jetpack/androidx/releases/test?utm_source=openai
- 4: https://developer.android.com/jetpack/androidx/releases/test?utm_source=openai
- 5: https://developer.android.com/jetpack/androidx/releases/test?utm_source=openai
Update Android Gradle Plugin to the latest stable (8.9.0) and confirm AndroidX version
- gradle/libs.versions.toml (lines 3–4):
- Change
android-gradle-plugin = "8.9.2"
→
android-gradle-plugin = "8.9.0"
(As of May 30 2025, 8.9.0 is the latest stable release; 8.9.2 is not yet available.) - Verify that the
androidx = "1.6.1"
alias corresponds to the intended stable AndroidX library and bump it if a newer stable release is required.
- Change
🤖 Prompt for AI Agents
In gradle/libs.versions.toml at lines 3 to 4, the android-gradle-plugin version
is incorrectly set to 8.9.2, which is not yet released; update it to the latest
stable version 8.9.0. Also, verify if the androidx version 1.6.1 matches the
intended stable AndroidX library version and update it to a newer stable release
if necessary to ensure compatibility and currency.
if (constraints.latency != null) { | ||
throw NotImplementedError( | ||
"Latency is not supported yet for Android. You can provide custom MediaTrackFactory" | ||
) | ||
} | ||
if (constraints.channelCount != null) { | ||
throw NotImplementedError( | ||
"Channel count is not supported yet for Android. You can provide custom MediaTrackFactory" | ||
) | ||
} | ||
if (constraints.sampleSize != null) { | ||
throw NotImplementedError( | ||
"Sample size is not supported yet for Android. You can provide custom MediaTrackFactory" | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Document unsupported audio constraints in the API.
Multiple audio constraints throw NotImplementedError
. Consider documenting these limitations in the MediaTrackFactory
interface or class KDoc to set clear expectations for API users.
Add documentation at the class level:
/**
* Android implementation of MediaTrackFactory.
*
* Note: The following audio constraints are not yet supported on Android:
* - latency
* - channelCount
* - sampleSize
*
* These constraints will throw NotImplementedError if specified.
*/
public class AndroidMediaDevices(
🤖 Prompt for AI Agents
In
ktor-client/ktor-client-webrtc/android/src/io/ktor/client/webrtc/engine/MediaDevices.kt
around lines 90 to 104, the code throws NotImplementedError for unsupported
audio constraints latency, channelCount, and sampleSize. To improve API clarity,
add KDoc documentation at the class level of AndroidMediaDevices describing
these unsupported constraints and warning users that specifying them will throw
NotImplementedError.
override suspend fun createVideoTrack(constraints: WebRTCMedia.VideoTrackConstraints): WebRTCMedia.VideoTrack = | ||
withPermissionException("video") { | ||
val streamConstrains = MediaStreamConstraints(audio = constraints.toJS()) | ||
val mediaStream = navigator.mediaDevices.getUserMedia(streamConstrains).await() | ||
return JsVideoTrack(mediaStream.getVideoTracks()[0], mediaStream) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix incorrect constraint type for video track creation.
The createVideoTrack
method is using audio
constraints instead of video
constraints when creating the MediaStreamConstraints.
Apply this fix:
override suspend fun createVideoTrack(constraints: WebRTCMedia.VideoTrackConstraints): WebRTCMedia.VideoTrack =
withPermissionException("video") {
- val streamConstrains = MediaStreamConstraints(audio = constraints.toJS())
+ val streamConstrains = MediaStreamConstraints(video = constraints.toJS())
val mediaStream = navigator.mediaDevices.getUserMedia(streamConstrains).await()
return JsVideoTrack(mediaStream.getVideoTracks()[0], mediaStream)
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
override suspend fun createVideoTrack(constraints: WebRTCMedia.VideoTrackConstraints): WebRTCMedia.VideoTrack = | |
withPermissionException("video") { | |
val streamConstrains = MediaStreamConstraints(audio = constraints.toJS()) | |
val mediaStream = navigator.mediaDevices.getUserMedia(streamConstrains).await() | |
return JsVideoTrack(mediaStream.getVideoTracks()[0], mediaStream) | |
} | |
override suspend fun createVideoTrack(constraints: WebRTCMedia.VideoTrackConstraints): WebRTCMedia.VideoTrack = | |
withPermissionException("video") { | |
- val streamConstrains = MediaStreamConstraints(audio = constraints.toJS()) | |
+ val streamConstrains = MediaStreamConstraints(video = constraints.toJS()) | |
val mediaStream = navigator.mediaDevices.getUserMedia(streamConstrains).await() | |
return JsVideoTrack(mediaStream.getVideoTracks()[0], mediaStream) | |
} |
🤖 Prompt for AI Agents
In ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/Engine.js.kt
around lines 37 to 42, the createVideoTrack method incorrectly uses audio
constraints instead of video constraints when creating MediaStreamConstraints.
Change the property from audio to video and pass the correct constraints object
to properly request video media stream.
nativePeerConnection.ontrack = { event: RTCTrackEvent -> | ||
val stream = event.streams.getOrNull(0) | ||
stream?.onremovetrack = { e -> | ||
launch { remoteTracks.emit(Remove(JsMediaTrack.from(e.track, stream!!))) } | ||
} | ||
launch { | ||
remoteTracks.emit(Add(JsMediaTrack.from(event.track, stream ?: MediaStream()))) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid potential null pointer exception with !! operator.
The use of !!
operator on line 118 could throw if the stream becomes null between the check and usage. Consider a safer approach.
Apply this fix to handle the edge case more gracefully:
nativePeerConnection.ontrack = { event: RTCTrackEvent ->
val stream = event.streams.getOrNull(0)
stream?.onremovetrack = { e ->
- launch { remoteTracks.emit(Remove(JsMediaTrack.from(e.track, stream!!))) }
+ stream?.let { s -> launch { remoteTracks.emit(Remove(JsMediaTrack.from(e.track, s))) } }
}
launch {
remoteTracks.emit(Add(JsMediaTrack.from(event.track, stream ?: MediaStream())))
}
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
nativePeerConnection.ontrack = { event: RTCTrackEvent -> | |
val stream = event.streams.getOrNull(0) | |
stream?.onremovetrack = { e -> | |
launch { remoteTracks.emit(Remove(JsMediaTrack.from(e.track, stream!!))) } | |
} | |
launch { | |
remoteTracks.emit(Add(JsMediaTrack.from(event.track, stream ?: MediaStream()))) | |
} | |
} | |
nativePeerConnection.ontrack = { event: RTCTrackEvent -> | |
val stream = event.streams.getOrNull(0) | |
stream?.onremovetrack = { e -> | |
stream?.let { s -> launch { remoteTracks.emit(Remove(JsMediaTrack.from(e.track, s))) } } | |
} | |
launch { | |
remoteTracks.emit(Add(JsMediaTrack.from(event.track, stream ?: MediaStream()))) | |
} | |
} |
🤖 Prompt for AI Agents
In ktor-client/ktor-client-webrtc/js/src/io/ktor/client/webrtc/Engine.js.kt
around lines 115 to 123, the use of the !! operator on the stream variable can
cause a null pointer exception if the stream becomes null after the initial
check. To fix this, avoid using !! and instead safely handle the nullable stream
by checking its presence before accessing it or by using a let or if statement
to ensure the stream is not null before emitting the Remove event.
public final class io/ktor/client/webrtc/engine/AndroidWebRTCEngineConfig : io/ktor/client/webrtc/WebRTCConfig { | ||
public field context Landroid/content/Context; | ||
public fun <init> ()V | ||
public final fun getContext ()Landroid/content/Context; | ||
public final fun getRtcFactory ()Lorg/webrtc/PeerConnectionFactory; | ||
public final fun setContext (Landroid/content/Context;)V | ||
public final fun setRtcFactory (Lorg/webrtc/PeerConnectionFactory;)V | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Public mutable field violates encapsulation.
The AndroidWebRTCEngineConfig
class has a public mutable field:
public field context Landroid/content/Context;
This violates encapsulation principles. The field should be private with proper getter/setter methods. The class already has getter and setter methods defined, so the field visibility should be restricted.
🤖 Prompt for AI Agents
In ktor-client/ktor-client-webrtc/api/android/ktor-client-webrtc.api around
lines 520 to 527, the field 'context' is declared as public mutable, violating
encapsulation. Change the visibility of the 'context' field from public to
private, ensuring it is only accessible through the existing getter and setter
methods to properly encapsulate the field.
public final class io/ktor/client/webrtc/Add : io/ktor/client/webrtc/Operation { | ||
public fun <init> (Ljava/lang/Object;)V | ||
} | ||
|
||
public abstract interface class io/ktor/client/webrtc/MediaTrackFactory { | ||
public abstract fun createAudioTrack (Lio/ktor/client/webrtc/WebRTCMedia$AudioTrackConstraints;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; | ||
public abstract fun createVideoTrack (Lio/ktor/client/webrtc/WebRTCMedia$VideoTrackConstraints;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; | ||
} | ||
|
||
public abstract class io/ktor/client/webrtc/Operation { | ||
public synthetic fun <init> (Ljava/lang/Object;Lkotlin/jvm/internal/DefaultConstructorMarker;)V | ||
public final fun getItem ()Ljava/lang/Object; | ||
} | ||
|
||
public final class io/ktor/client/webrtc/Remove : io/ktor/client/webrtc/Operation { | ||
public fun <init> (Ljava/lang/Object;)V | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider using generics for type safety in Operation classes.
The Add
and Remove
operations take Object
parameters, which reduces type safety. Consider using generics to make these operations type-safe:
-public abstract class io/ktor/client/webrtc/Operation {
+public abstract class io/ktor/client/webrtc/Operation<T> {
- public final fun getItem ()Ljava/lang/Object;
+ public final fun getItem ()Ljava/lang/Object; // Should return T
-public final class io/ktor/client/webrtc/Add : io/ktor/client/webrtc/Operation {
- public fun <init> (Ljava/lang/Object;)V
+public final class io/ktor/client/webrtc/Add<T> : io/ktor/client/webrtc/Operation<T> {
+ public fun <init> (Ljava/lang/Object;)V // Should take T parameter
This would provide compile-time type safety and better API documentation.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In ktor-client/ktor-client-webrtc/api/android/ktor-client-webrtc.api around
lines 1 to 17, the Add and Remove classes use raw Object types for their
parameters, which reduces type safety. Refactor these classes to use generics by
defining a type parameter for the Operation class and applying it to Add and
Remove, so that the item they operate on is strongly typed. Update constructors
and methods accordingly to use the generic type instead of Object, ensuring
compile-time type safety and clearer API usage.
public static final field Companion Lio/ktor/client/webrtc/WebRTC$IceCandidate$Companion; | ||
public fun <init> (Ljava/lang/String;Ljava/lang/String;I)V | ||
public final fun component1 ()Ljava/lang/String; | ||
public final fun component2 ()Ljava/lang/String; | ||
public final fun component3 ()I | ||
public final fun copy (Ljava/lang/String;Ljava/lang/String;I)Lio/ktor/client/webrtc/WebRTC$IceCandidate; | ||
public static synthetic fun copy$default (Lio/ktor/client/webrtc/WebRTC$IceCandidate;Ljava/lang/String;Ljava/lang/String;IILjava/lang/Object;)Lio/ktor/client/webrtc/WebRTC$IceCandidate; | ||
public fun equals (Ljava/lang/Object;)Z | ||
public final fun getCandidate ()Ljava/lang/String; | ||
public final fun getSdpMLineIndex ()I | ||
public final fun getSdpMid ()Ljava/lang/String; | ||
public fun hashCode ()I | ||
public fun toString ()Ljava/lang/String; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential null safety issue with IceCandidate.sdpMLineIndex.
In the WebRTC specification, sdpMLineIndex
can be null when the ICE candidate is not associated with a specific media line. The current API uses Int
which cannot represent null values.
Consider changing the type to nullable:
-public fun <init> (Ljava/lang/String;Ljava/lang/String;I)V
+public fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;)V
-public final fun getSdpMLineIndex ()I
+public final fun getSdpMLineIndex ()Ljava/lang/Integer;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
public static final field Companion Lio/ktor/client/webrtc/WebRTC$IceCandidate$Companion; | |
public fun <init> (Ljava/lang/String;Ljava/lang/String;I)V | |
public final fun component1 ()Ljava/lang/String; | |
public final fun component2 ()Ljava/lang/String; | |
public final fun component3 ()I | |
public final fun copy (Ljava/lang/String;Ljava/lang/String;I)Lio/ktor/client/webrtc/WebRTC$IceCandidate; | |
public static synthetic fun copy$default (Lio/ktor/client/webrtc/WebRTC$IceCandidate;Ljava/lang/String;Ljava/lang/String;IILjava/lang/Object;)Lio/ktor/client/webrtc/WebRTC$IceCandidate; | |
public fun equals (Ljava/lang/Object;)Z | |
public final fun getCandidate ()Ljava/lang/String; | |
public final fun getSdpMLineIndex ()I | |
public final fun getSdpMid ()Ljava/lang/String; | |
public fun hashCode ()I | |
public fun toString ()Ljava/lang/String; | |
} | |
public static final field Companion Lio/ktor/client/webrtc/WebRTC$IceCandidate$Companion; | |
- public fun <init> (Ljava/lang/String;Ljava/lang/String;I)V | |
+ public fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;)V | |
public final fun component1 ()Ljava/lang/String; | |
public final fun component2 ()Ljava/lang/String; | |
public final fun component3 ()I | |
public final fun copy (Ljava/lang/String;Ljava/lang/String;I)Lio/ktor/client/webrtc/WebRTC$IceCandidate; | |
public static synthetic fun copy$default (Lio/ktor/client/webrtc/WebRTC$IceCandidate;Ljava/lang/String;Ljava/lang/String;IILjava/lang/Object;)Lio/ktor/client/webrtc/WebRTC$IceCandidate; | |
public fun equals (Ljava/lang/Object;)Z | |
public final fun getCandidate ()Ljava/lang/String; | |
- public final fun getSdpMLineIndex ()I | |
+ public final fun getSdpMLineIndex ()Ljava/lang/Integer; | |
public final fun getSdpMid ()Ljava/lang/String; | |
public fun hashCode ()I | |
public fun toString ()Ljava/lang/String; | |
} |
🤖 Prompt for AI Agents
In ktor-client/ktor-client-webrtc/api/android/ktor-client-webrtc.api around
lines 78 to 91, the sdpMLineIndex property in IceCandidate is currently typed as
a non-nullable Int, but per WebRTC spec it can be null. Change the type of
sdpMLineIndex from Int to a nullable Int (e.g., Int?) to correctly represent
cases where it is absent, and update all related methods and constructor
parameters accordingly to handle null values safely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oooook 😅 before I even start to review this, may I ask you to split this PR into at least 4 others: Infra changes, Android support, JS, WasmJs. If there is an overlap between those 3 targets, then also Core (so 5 PRs). Then in each of them, please make clean git history, so that the changes can be reviewed commit by commit. The last point is not very strict, so don't bother yourself too much, but nonetheless keep in mind that commit history should easily readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now I left comments only for infrastructure tings. But I should check it again after migration to KMP Android plugin
build-logic/settings.gradle.kts
Outdated
mavenCentral() | ||
gradlePluginPortal() | ||
google() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mavenCentral
and google
seem redundant here as they're added by settings plugin
//if (targets.hasAndroidJvm && plugins.hasPlugin("com.android.library")) { | ||
//// kotlin { | ||
//// sourceSets { | ||
//// androidMain | ||
//// androidUnitTest | ||
//// androidInstrumentedTest | ||
//// } | ||
//// } | ||
//} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be removed?
group("android") { | ||
withAndroidTarget() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This group could be removed as it contains only one target
pluginManagement { | ||
repositories { | ||
google() | ||
mavenCentral() | ||
gradlePluginPortal() | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this pluginManagement
block can be removed as we don't apply any plugins from custom repositories to the build-settings-logic
project
@@ -67,6 +67,7 @@ kotlin.daemon.useFallbackStrategy=false | |||
# Enable new project model to be prepared for enabling isolated projects | |||
# TODO: Remove when we enable isolated projects in Gradle | |||
kotlin.kmp.isolated-projects.support=enable | |||
android.useAndroidX=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor. Let's separate it from Kotlin properties with blank line and comment
android.useAndroidX=true | |
# Android | |
android.useAndroidX=true |
jvmToolchain(17) | ||
|
||
androidTarget { | ||
publishLibraryVariants("release") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be moved to the place where we declare androidTarget
the first time, as it will be needed for any android module.
Thank you! I will first separate the pull request as @Mr3zee suggested and apply those changes in the Android part. |
Sure, I agree it was too much code inside one PR! |
This PR was split into 4 smaller ones. |
Subsystem
Client
Motivation
KTOR-7958 WebRTC Client.
Solution
The solution contains a common KMP interface that allows
The API is "up to Ktor standards" and utilises flows and coroutines rather than callbacks. It is unified across those platforms (configs are custom for all engines), but still allows access to native objects if some custom logic is needed.
There are integration tests that
TODO:
Example project that uses AndroidWebRTCEngine: ktor-chat