Fix crash on fishjam when updateMetadata is sent before renegotiation ends. #21
Conversation
| @@ -220,6 +223,7 @@ internal class InternalMembraneRTC( | |||
| trackId: String, | |||
| trackMetadata: Metadata | |||
| ) { | |||
| if (!canUpdateMetadata)return | |||
There was a problem hiding this comment.
ktlint does not complain
| @@ -347,6 +351,7 @@ internal class InternalMembraneRTC( | |||
| } | |||
| } | |||
| } | |||
| canUpdateMetadata = true | |||
There was a problem hiding this comment.
maybe we should use a mutex / coroutineScope instead (or other such mechanism)?
user can also do other things other than updating metadata so I'd prefer something that blocks other things completely
| @@ -210,6 +212,7 @@ internal class InternalMembraneRTC( | |||
| } | |||
|
|
|||
| fun updateEndpointMetadata(endpointMetadata: Metadata) { | |||
| if (!canUpdateMetadata)return | |||
There was a problem hiding this comment.
also when we cannot update metadata we just fail silently?
| package org.membraneframework.rtc.error | ||
|
|
||
| enum class MembraneError(val message: String) { | ||
| TRACK_NOT_READY("Track was not confirmed by backed. Try again later") |
There was a problem hiding this comment.
I'd change this message to something like "Track updated while being negotiated in peer connection. Wait for the track to be negotiated.". "Track was not confirmed by the backend" and "Try again later" is too vague imo.
Now that I think about it, how user should wait for the negotiation?
| @@ -219,11 +230,14 @@ internal class InternalMembraneRTC( | |||
| fun updateTrackMetadata( | |||
| trackId: String, | |||
| trackMetadata: Metadata | |||
| ) { | |||
| ): Result<Unit> { | |||
There was a problem hiding this comment.
Wouldn't it be easier to just throw exception?
No description provided.