diff --git a/.changeset/gold-cats-knock.md b/.changeset/gold-cats-knock.md new file mode 100644 index 00000000..28e13324 --- /dev/null +++ b/.changeset/gold-cats-knock.md @@ -0,0 +1,5 @@ +--- +"client-sdk-android": patch +--- + +Fix crash when publishing disposed tracks diff --git a/livekit-android-sdk/src/main/java/io/livekit/android/room/participant/LocalParticipant.kt b/livekit-android-sdk/src/main/java/io/livekit/android/room/participant/LocalParticipant.kt index f6ee22c5..35837473 100644 --- a/livekit-android-sdk/src/main/java/io/livekit/android/room/participant/LocalParticipant.kt +++ b/livekit-android-sdk/src/main/java/io/livekit/android/room/participant/LocalParticipant.kt @@ -432,6 +432,7 @@ internal constructor( * * @param track The track to publish. * @param options The publish options to use, or [Room.audioTrackPublishDefaults] if none is passed. + * @return true if the track published successfully */ suspend fun publishAudioTrack( track: LocalAudioTrack, @@ -441,6 +442,11 @@ internal constructor( ).copy(preconnect = defaultsManager.isPrerecording), publishListener: PublishListener? = null, ): Boolean { + if (track.isDisposed) { + LKLog.w { "Attempting to publish a disposed track, ignoring." } + return false + } + val encodings = listOf( RtpParameters.Encoding(null, true, null).apply { if (options.audioBitrate != null && options.audioBitrate > 0) { @@ -448,18 +454,23 @@ internal constructor( } }, ) - val publication = publishTrackImpl( - track = track, - options = options, - requestConfig = { - disableDtx = !options.dtx - disableRed = !options.red - addAllAudioFeatures(options.getFeaturesList()) - source = options.source?.toProto() ?: LivekitModels.TrackSource.MICROPHONE - }, - encodings = encodings, - publishListener = publishListener, - ) + var publication: LocalTrackPublication? = null + try { + publication = publishTrackImpl( + track = track, + options = options, + requestConfig = { + disableDtx = !options.dtx + disableRed = !options.red + addAllAudioFeatures(options.getFeaturesList()) + source = options.source?.toProto() ?: LivekitModels.TrackSource.MICROPHONE + }, + encodings = encodings, + publishListener = publishListener, + ) + } catch (e: TrackException.PublishException) { + LKLog.e(e) { "Error thrown when publishing track:" } + } if (publication != null) { val job = scope.launch { @@ -478,6 +489,7 @@ internal constructor( * * @param track The track to publish. * @param options The publish options to use, or [Room.videoTrackPublishDefaults] if none is passed. + * @return true if the track published successfully */ suspend fun publishVideoTrack( track: LocalVideoTrack, @@ -489,6 +501,17 @@ internal constructor( ): Boolean { @Suppress("NAME_SHADOWING") var options = options + if (track.isDisposed) { + LKLog.w { "Attempting to publish a disposed track, ignoring." } + return false + } + + val rtcTrackId = track.withRTCTrack(null) { id() } + if (rtcTrackId == null) { + LKLog.w { "Attempting to publish a disposed track, ignoring." } + return false + } + synchronized(enabledPublishVideoCodecs) { if (enabledPublishVideoCodecs.isNotEmpty()) { if (enabledPublishVideoCodecs.none { allowedCodec -> allowedCodec.mime.mimeTypeToVideoCodec() == options.videoCodec }) { @@ -522,40 +545,47 @@ internal constructor( val videoLayers = EncodingUtils.videoLayersFromEncodings(track.dimensions.width, track.dimensions.height, encodings, isSVC) - return publishTrackImpl( - track = track, - options = options, - requestConfig = { - width = track.dimensions.width - height = track.dimensions.height - source = options.source?.toProto() ?: if (track.options.isScreencast) { - LivekitModels.TrackSource.SCREEN_SHARE - } else { - LivekitModels.TrackSource.CAMERA - } - addAllLayers(videoLayers) + var publication: LocalTrackPublication? = null + try { + publication = publishTrackImpl( + track = track, + options = options, + requestConfig = { + width = track.dimensions.width + height = track.dimensions.height + source = options.source?.toProto() ?: if (track.options.isScreencast) { + LivekitModels.TrackSource.SCREEN_SHARE + } else { + LivekitModels.TrackSource.CAMERA + } + addAllLayers(videoLayers) - addSimulcastCodecs( - with(SimulcastCodec.newBuilder()) { - codec = options.videoCodec - cid = track.rtcTrack.id() - build() - }, - ) - // set up backup codec - if (options.backupCodec?.codec != null && options.videoCodec != options.backupCodec?.codec) { addSimulcastCodecs( with(SimulcastCodec.newBuilder()) { - codec = options.backupCodec!!.codec - cid = "" + codec = options.videoCodec + cid = rtcTrackId build() }, ) - } - }, - encodings = encodings, - publishListener = publishListener, - ) != null + // set up backup codec + if (options.backupCodec?.codec != null && options.videoCodec != options.backupCodec?.codec) { + addSimulcastCodecs( + with(SimulcastCodec.newBuilder()) { + codec = options.backupCodec!!.codec + cid = "" + build() + }, + ) + } + }, + encodings = encodings, + publishListener = publishListener, + ) + } catch (e: TrackException.PublishException) { + LKLog.e(e) { "Error thrown when publishing track:" } + } + + return publication != null } private fun hasPermissionsToPublish(source: Track.Source): Boolean { @@ -581,6 +611,7 @@ internal constructor( * @throws TrackException.PublishException thrown when the publish fails. see [TrackException.PublishException.message] for details. * @return true if the track publish was successful. */ + @Throws(TrackException.PublishException::class) private suspend fun publishTrackImpl( track: Track, options: TrackPublishOptions, @@ -588,6 +619,11 @@ internal constructor( encodings: List = emptyList(), publishListener: PublishListener? = null, ): LocalTrackPublication? { + if (track.isDisposed) { + LKLog.w { "Attempting to publish a disposed track, ignoring." } + return null + } + fun onPublishFailure(e: TrackException.PublishException, triggerEvent: Boolean = true) { publishListener?.onPublishFailure(e) if (triggerEvent) { @@ -1164,6 +1200,7 @@ internal constructor( continuation.cancel(RpcError.BuiltinRpcError.RESPONSE_TIMEOUT.create()) } } + responseTimeoutJob // workaround for lint marking this unused. used in cleanup() pendingResponses[requestId] = PendingRpcResponse( participantIdentity = destinationIdentity, diff --git a/livekit-android-sdk/src/main/java/io/livekit/android/room/track/Track.kt b/livekit-android-sdk/src/main/java/io/livekit/android/room/track/Track.kt index 696c5146..653eed42 100644 --- a/livekit-android-sdk/src/main/java/io/livekit/android/room/track/Track.kt +++ b/livekit-android-sdk/src/main/java/io/livekit/android/room/track/Track.kt @@ -191,6 +191,9 @@ abstract class Track( } } + /** + * Ensures the track is valid before attempting to run [action]. + */ @OptIn(ExperimentalContracts::class) internal inline fun withRTCTrack(crossinline action: MediaStreamTrack.() -> T) { contract { callsInPlace(action, InvocationKind.AT_MOST_ONCE) } diff --git a/livekit-android-test/src/main/java/io/livekit/android/test/mock/MockRTCThreadToken.kt b/livekit-android-test/src/main/java/io/livekit/android/test/mock/MockRTCThreadToken.kt index a6c85dd2..301e805b 100644 --- a/livekit-android-test/src/main/java/io/livekit/android/test/mock/MockRTCThreadToken.kt +++ b/livekit-android-test/src/main/java/io/livekit/android/test/mock/MockRTCThreadToken.kt @@ -20,5 +20,5 @@ import io.livekit.android.webrtc.peerconnection.RTCThreadToken class MockRTCThreadToken : RTCThreadToken { override val isDisposed: Boolean - get() = true + get() = false } diff --git a/livekit-android-test/src/main/java/io/livekit/android/test/mock/dagger/TestRTCModule.kt b/livekit-android-test/src/main/java/io/livekit/android/test/mock/dagger/TestRTCModule.kt index 282640bb..e3d1e272 100644 --- a/livekit-android-test/src/main/java/io/livekit/android/test/mock/dagger/TestRTCModule.kt +++ b/livekit-android-test/src/main/java/io/livekit/android/test/mock/dagger/TestRTCModule.kt @@ -90,7 +90,6 @@ object TestRTCModule { appContext: Context, ): PeerConnectionFactory { WebRTCInitializer.initialize(appContext) - return MockPeerConnectionFactory() } diff --git a/livekit-android-test/src/test/java/io/livekit/android/room/participant/LocalParticipantMockE2ETest.kt b/livekit-android-test/src/test/java/io/livekit/android/room/participant/LocalParticipantMockE2ETest.kt index 43b7b5db..30463e5b 100644 --- a/livekit-android-test/src/test/java/io/livekit/android/room/participant/LocalParticipantMockE2ETest.kt +++ b/livekit-android-test/src/test/java/io/livekit/android/room/participant/LocalParticipantMockE2ETest.kt @@ -789,7 +789,7 @@ class LocalParticipantMockE2ETest : MockE2ETest() { } @Test - fun lackOfPublishPermissionCausesException() = runTest { + fun lackOfPublishPermissionReturnsFalse() = runTest { val noCanPublishJoin = with(TestData.JOIN.toBuilder()) { join = with(join.toBuilder()) { participant = with(participant.toBuilder()) { @@ -805,14 +805,7 @@ class LocalParticipantMockE2ETest : MockE2ETest() { } connect(noCanPublishJoin) - var didThrow = false - try { - room.localParticipant.publishVideoTrack(createLocalTrack()) - } catch (e: TrackException.PublishException) { - didThrow = true - } - - assertTrue(didThrow) + assertFalse(room.localParticipant.publishVideoTrack(createLocalTrack())) } @Test