Skip to content

Commit

Permalink
ensure setMediaData thread-safe
Browse files Browse the repository at this point in the history
  • Loading branch information
StageGuard committed Jan 12, 2025
1 parent cd5c784 commit c1af32a
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 26 deletions.
24 changes: 17 additions & 7 deletions mediamp-api/src/commonMain/kotlin/AbstractMediampPlayer.kt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2024 OpenAni and contributors.
* Copyright (C) 2024-2025 OpenAni and contributors.
*
* Use of this source code is governed by the GNU GENERAL PUBLIC LICENSE version 3 license, which can be found at the following link.
*
Expand Down Expand Up @@ -28,7 +28,7 @@ import kotlin.coroutines.cancellation.CancellationException
* Method [setMediaData], [resume], [pause], [stopPlayback] and [close] are wrapped,
* please implement the actual playback control logic in corresponding `xxxImpl` methods. Note that:
*
* - These methods are called in main/UI thread, so implementations should not do any heavy work.
* - These methods are called in UI thread, so implementations should not do any heavy work.
* - These methods will only be called when the playback state is valid at its state transformation path,
* so it is not necessary to validate playback state.
* - These methods should ensure that playback state must be transformed to target state in the future.
Expand Down Expand Up @@ -68,19 +68,23 @@ public abstract class AbstractMediampPlayer<D : AbstractMediampPlayer.Data>(
private val closed = MutableStateFlow(false)

final override suspend fun setMediaData(data: MediaData): Unit = withContext(defaultDispatcher) {
if (closed.value || playbackState.value == PlaybackState.DESTROYED) {
return@withContext
}
setVideoSourceMutex.withLock {
if (playbackState.value == PlaybackState.DESTROYED) {
val currentState = playbackState.value
if (closed.value || currentState == PlaybackState.DESTROYED) {
return@withLock
}

// playback has set media data, stop previous first.
if (playbackState.value >= PlaybackState.READY) {
if (currentState >= PlaybackState.READY) {
val previousResource = openResource.value
if (data == previousResource?.mediaData) {
return@withLock
}
// stop playback if running
if (playbackState.value >= PlaybackState.PAUSED) {
if (currentState >= PlaybackState.PAUSED) {
stopPlaybackImpl()
}

Expand All @@ -97,8 +101,15 @@ public abstract class AbstractMediampPlayer<D : AbstractMediampPlayer.Data>(
playbackState.value = PlaybackState.ERROR
throw e
}

// Player is closed before setMediaDataImpl is finished
if (closed.value) {
opened.releaseResource.invoke()
return@withLock
}

this@AbstractMediampPlayer.openResource.value = opened
openResource.value = opened
playbackState.value = PlaybackState.READY
}
}

Expand All @@ -113,7 +124,6 @@ public abstract class AbstractMediampPlayer<D : AbstractMediampPlayer.Data>(
val currState = playbackState.value
if (currState == PlaybackState.READY || currState == PlaybackState.PAUSED) {
resumeImpl()
playbackState.value = PlaybackState.PLAYING
}
}

Expand Down
36 changes: 17 additions & 19 deletions mediamp-api/src/commonMain/kotlin/MediampPlayer.kt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2024 OpenAni and contributors.
* Copyright (C) 2024-2025 OpenAni and contributors.
*
* Use of this source code is governed by the GNU GENERAL PUBLIC LICENSE version 3 license, which can be found at the following link.
*
Expand Down Expand Up @@ -56,7 +56,8 @@ import kotlin.reflect.KClass
* | DESTROYED | | ERROR | | CREATED | | FINISHED | | READY | | PAUSED | | PLAYING | | BUFFERING |
* +-----+-----+ +---+---+ +----+----+ +----+-----+ +---+---+ +---+----+ +----+----+ +-----+-----+
* | | | | | | | |
* | |-----------+------------+----------->*<---------+------------+-------------| setMediaData
* | |-----------+------------+----------->* | | | setMediaData
* | | | +<-----------+----------+------------+-------------| setMediaData
* | | | | | | | |
* | | | | |----------+----------->| | resume
* | | | | | | | |
Expand Down Expand Up @@ -84,9 +85,6 @@ import kotlin.reflect.KClass
*
* For example, calling [stopPlayback] when `state >= READY`(incl [READY][PlaybackState.READY], [PAUSED][PlaybackState.PAUSED],
* [PLAYING][PlaybackState.PLAYING], [BUFFERING][PlaybackState.PAUSED_BUFFERING]) will always transform state to `FINISHED`.
*
* Playback control methods are expected to be called from the main/UI thread.
* Asynchronous operations of actual player implementations should ensure that playback state must be transformed to target state.
*
* ### Invalid calls are ignored
*
Expand All @@ -108,6 +106,8 @@ import kotlin.reflect.KClass
* Because user can set new media data at any state or any time except [DESTROYED][PlaybackState.DESTROYED],
* including [READY][PlaybackState.READY] itself.
*
* If the player has set media data, the previous media will be [closed][close] and set playback state to [FINISHED][PlaybackState.FINISHED] first.
*
* ### Error can occurred at any time
*
* When *fatal error* occurred, state will always be transformed to [ERROR][PlaybackState.ERROR] directly.
Expand All @@ -128,8 +128,8 @@ import kotlin.reflect.KClass
* This interface is not thread-safe. Concurrent calls to [resume] will lead to undefined behavior.
* However, flows might be collected from multiple threads simultaneously while performing another call like [resume] on a single thread.
*
* All methods in this interface are expected to be called from the **UI thread** on Android.
* Calls from illegal threads will cause an exception.
* Playback control methods are expected to be called from the UI thread. Calls from illegal threads will cause an exception.
* Asynchronous operations of actual player implementations should ensure that playback state must be transformed to target state.
*
* On other platforms, calls are not required to be on the UI thread but should still be called from a single thread.
* The implementation is guaranteed to be non-blocking and fast so, it is a recommended approach of making all calls from the UI thread in common code.
Expand Down Expand Up @@ -363,20 +363,11 @@ public class DummyMediampPlayer(
return playbackState.value
}

override fun stopPlaybackImpl() {
currentPositionMillis.value = 0
mediaProperties.value = null
playbackState.value = PlaybackState.READY
// TODO: 2025/1/5 We should encapsulate the mutable states to ensure consistency in flow emissions
}

override suspend fun setMediaDataImpl(data: MediaData): Data {
return Data(
data,
releaseResource = {
backgroundScope.launch(NonCancellable) {
data.close()
}
data.close()
},
)
}
Expand All @@ -386,15 +377,22 @@ public class DummyMediampPlayer(
}

override fun resumeImpl() {

playbackState.value = PlaybackState.PLAYING
}

override fun pauseImpl() {
playbackState.value = PlaybackState.PAUSED
}

override fun stopPlaybackImpl() {
currentPositionMillis.value = 0
mediaProperties.value = null
playbackState.value = PlaybackState.FINISHED
// TODO: 2025/1/5 We should encapsulate the mutable states to ensure consistency in flow emissions
}

override fun closeImpl() {

playbackState.value = PlaybackState.DESTROYED
}

override val features: PlayerFeatures = buildPlayerFeatures {
Expand Down

0 comments on commit c1af32a

Please sign in to comment.