Skip to content

Commit

Permalink
[APT-10449] Clients Call From Any Thread
Browse files Browse the repository at this point in the history 
Makes no assumptions about what thread the client is calling ArmadilloPlayer from, aside from initialization.  For those recently updating since 1.5.1, this should resolve any strange bugs from client behavior.

Avoids making DRM system calls for content without DRM. This probably doesn't change anything.
  • Loading branch information
@kabliz
kabliz committed Sep 20, 2024
1 parent 27c8256 commit b2f97df
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 62 deletions.
109 changes: 68 additions & 41 deletions Armadillo/src/main/java/com/scribd/armadillo/ArmadilloPlayerChoreographer.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@ package com.scribd.armadillo

import android.content.Context
import android.os.Bundle
import android.os.Handler
import android.os.HandlerThread
import android.support.v4.media.MediaBrowserCompat
import android.support.v4.media.session.MediaControllerCompat
import android.support.v4.media.session.MediaSessionCompat
import android.support.v4.media.session.PlaybackStateCompat
import android.util.Log
import androidx.annotation.VisibleForTesting
import androidx.annotation.VisibleForTesting.PRIVATE
import com.scribd.armadillo.actions.Action
import com.scribd.armadillo.actions.ErrorAction
import com.scribd.armadillo.actions.MediaRequestUpdateAction
Expand Down Expand Up @@ -65,6 +68,7 @@ interface ArmadilloPlayer {
fun removeDownload(audioPlayable: AudioPlayable)
fun removeAllDownloads()
fun clearCache()

/**
* Starts playback with the given [AudioPlayable], allowing for configuration through a given [ArmadilloConfiguration]
*/
Expand Down Expand Up @@ -189,22 +193,37 @@ internal class ArmadilloPlayerChoreographer : ArmadilloPlayer {

@Inject
internal lateinit var context: Context

@Inject
internal lateinit var downloadEngine: DownloadEngine

@Inject
internal lateinit var cacheManager: CacheManager

@Inject
internal lateinit var stateProvider: StateStore.Provider

@Inject
internal lateinit var stateModifier: StateStore.Modifier

@Inject
internal lateinit var actionTransmitter: PlaybackActionTransmitter

@Inject
internal lateinit var mediaContentSharer: ArmadilloMediaBrowse.ContentSharer

private companion object {
const val TAG = "ArmadilloChoreographer"
companion object {
private const val TAG = "ArmadilloChoreographer"
private val observerPollIntervalMillis = 500.milliseconds

@VisibleForTesting(otherwise = PRIVATE)
val handlerThread: HandlerThread by lazy {
HandlerThread("ArmadilloChoreographer")
.also { it.start() }
}

@VisibleForTesting(otherwise = PRIVATE)
val handler: Handler by lazy { Handler(handlerThread.looper) }
}

override val playbackCacheSize: Long
Expand All @@ -216,7 +235,7 @@ internal class ArmadilloPlayerChoreographer : ArmadilloPlayer {
* emits the most recently emitted state and all the subsequent states when an observer subscribes to it.
*/
val armadilloStateSubject: BehaviorSubject<ArmadilloState>
get() = stateProvider.stateSubject
get() = stateProvider.stateSubject
override val armadilloStateObservable: Observable<ArmadilloState>
get() = armadilloStateSubject.observeOn(AndroidSchedulers.mainThread())

Expand All @@ -235,34 +254,46 @@ internal class ArmadilloPlayerChoreographer : ArmadilloPlayer {
@VisibleForTesting
internal var playbackConnection: MediaSessionConnection? = null

private fun runHandler(lambda: () -> Unit) {
handler.post { lambda() }
}

private fun runIfPlaybackReady(lambda: (controls: MediaControllerCompat.TransportControls, playbackState: PlaybackState) -> Unit) {
runHandler {
doIfPlaybackReady { controls, playbackState ->
lambda(controls, playbackState)
}
}
}

override fun initDownloadEngine(): ArmadilloPlayer {
isDownloadEngineInit = true
downloadEngine.init()
updateProgressPollTask()
return this
}

override fun beginDownload(audioPlayable: AudioPlayable) {
override fun beginDownload(audioPlayable: AudioPlayable) = runHandler {
if (!isDownloadEngineInit) {
stateModifier.dispatch(ErrorAction(EngineNotInitialized("download engine cannot start download.")))
return
} else {
downloadEngine.download(audioPlayable)
}
downloadEngine.download(audioPlayable)
}

override fun clearCache() {
override fun clearCache() = runHandler {
cacheManager.clearPlaybackCache()
}

override fun removeAllDownloads() {
override fun removeAllDownloads() = runHandler {
if (!isDownloadEngineInit) {
stateModifier.dispatch(ErrorAction(EngineNotInitialized("Cannot remove all the downloads.")))
return
} else {
downloadEngine.removeAllDownloads()
}
downloadEngine.removeAllDownloads()
}

override fun beginPlayback(audioPlayable: AudioPlayable, config: ArmadilloConfiguration) {
override fun beginPlayback(audioPlayable: AudioPlayable, config: ArmadilloConfiguration) = runHandler {
disposables.clear()
actionTransmitter.begin(observerPollIntervalMillis)
val mediaSessionConnection = MediaSessionConnection(context)
Expand All @@ -279,84 +310,80 @@ internal class ArmadilloPlayerChoreographer : ArmadilloPlayer {
})
}

override fun updateMediaRequest(mediaRequest: AudioPlayable.MediaRequest) {
doIfPlaybackReady { controls, _ ->
override fun updateMediaRequest(mediaRequest: AudioPlayable.MediaRequest) =
runIfPlaybackReady { controls, _ ->
stateModifier.dispatch(MediaRequestUpdateAction(mediaRequest))
controls.sendCustomAction(CustomAction.UpdateMediaRequest(mediaRequest))
}
}

override fun updatePlaybackMetadata(title: String, chapters: List<Chapter>) {
override fun updatePlaybackMetadata(title: String, chapters: List<Chapter>) =
doWhenPlaybackReady { controls ->
stateModifier.dispatch(MetadataUpdateAction(title, chapters))
controls.sendCustomAction(CustomAction.UpdatePlaybackMetadata(title, chapters))
}
}

override fun endPlayback() {
override fun endPlayback() = runHandler {
playbackConnection?.transportControls?.stop()
}

override fun deinit() {
override fun deinit() = runHandler {
Log.v(TAG, "deinit")
disposables.clear()
pollingSubscription = null
isDownloadEngineInit = false
actionTransmitter.destroy()
}

override fun playOrPause() {
doIfPlaybackReady { controls, playbackState ->
when (playbackState) {
PlaybackState.PLAYING -> controls.pause()
PlaybackState.PAUSED -> controls.play()
else -> {
stateModifier.dispatch(ErrorAction(
UnexpectedException(cause = IllegalStateException("Neither playing nor paused"),
actionThatFailedMessage = "Trying to play or pause media."))
)
}
override fun playOrPause() = runIfPlaybackReady { controls, playbackState ->
when (playbackState) {
PlaybackState.PLAYING -> controls.pause()
PlaybackState.PAUSED -> controls.play()
else -> {
stateModifier.dispatch(ErrorAction(
UnexpectedException(cause = IllegalStateException("Neither playing nor paused"),
actionThatFailedMessage = "Trying to play or pause media."))
)
}
}
}

// Note: chapter skip and jump-skip behaviours are swapped. See MediaSessionCallback - we are using a jump-skip for skip-forward, as
// most headphones only have a skip-forward button, and this is the ideal behaviour for spoken audio.
override fun nextChapter() = doIfPlaybackReady { controls, _ -> controls.fastForward() }
override fun nextChapter() = runIfPlaybackReady { controls, _ -> controls.fastForward() }

override fun previousChapter() = doIfPlaybackReady { controls, _ -> controls.rewind() }
override fun previousChapter() = runIfPlaybackReady { controls, _ -> controls.rewind() }

override fun skipForward() = doIfPlaybackReady { controls, _ -> controls.skipToNext() }
override fun skipForward() = runIfPlaybackReady { controls, _ -> controls.skipToNext() }

override fun skipBackward() = doIfPlaybackReady { controls, _ -> controls.skipToPrevious() }
override fun skipBackward() = runIfPlaybackReady { controls, _ -> controls.skipToPrevious() }

override fun seekTo(position: Milliseconds) = doIfPlaybackReady { controls, _ ->
override fun seekTo(position: Milliseconds) = runIfPlaybackReady { controls, _ ->
// Add a shift constant to all seeks originating from the client application
// as opposed to system originated, such as from notification
controls.seekTo(position.longValue + Constants.AUDIO_POSITION_SHIFT_IN_MS)
}

override fun seekWithinChapter(percent: Int) {
override fun seekWithinChapter(percent: Int) = runHandler {
val position = stateProvider.currentState.positionFromChapterPercent(percent)
?: run {
stateModifier.dispatch(ErrorAction(
UnexpectedException(cause = KotlinNullPointerException("Current state's position is null"),
actionThatFailedMessage = "seeking within chapter"))
)
return
return@runHandler
}
seekTo(position)
}

override fun removeDownload(audioPlayable: AudioPlayable) {
override fun removeDownload(audioPlayable: AudioPlayable) = runHandler {
if (!isDownloadEngineInit) {
stateModifier.dispatch(ErrorAction(EngineNotInitialized("Cannot remove a download.")))
return
} else {
downloadEngine.removeDownload(audioPlayable)
}
downloadEngine.removeDownload(audioPlayable)
}

override fun addPlaybackActionListener(listener: PlaybackActionListener) {
override fun addPlaybackActionListener(listener: PlaybackActionListener) = runHandler {
PlaybackActionListenerHolder.actionlisteners.add(listener)
}

Expand All @@ -370,7 +397,7 @@ internal class ArmadilloPlayerChoreographer : ArmadilloPlayer {
mediaContentSharer.browseController = null
}

override fun notifyMediaBrowseContentChanged(rootId: String) = mediaContentSharer.notifyContentChanged(rootId)
override fun notifyMediaBrowseContentChanged(rootId: String) = runHandler { mediaContentSharer.notifyContentChanged(rootId) }

/**
* [ArmadilloPlayerChoreographer] polls for updates of playback & downloading
Expand Down
1 change: 0 additions & 1 deletion Armadillo/src/main/java/com/scribd/armadillo/di/PlaybackModule.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package com.scribd.armadillo.di

import android.app.Application
import android.content.Context
import com.google.android.exoplayer2.drm.DefaultDrmSessionManagerProvider
import com.google.android.exoplayer2.drm.DrmSessionManagerProvider
import com.scribd.armadillo.StateStore
import com.scribd.armadillo.broadcast.ArmadilloNoisyReceiver
Expand Down
36 changes: 20 additions & 16 deletions ...dillo/src/main/java/com/scribd/armadillo/playback/mediasource/DashMediaSourceGenerator.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import com.scribd.armadillo.actions.OpeningLicenseAction
import com.scribd.armadillo.download.DownloadEngine
import com.scribd.armadillo.download.DownloadTracker
import com.scribd.armadillo.download.drm.events.WidevineSessionEventListener
import com.scribd.armadillo.extensions.toUri
import com.scribd.armadillo.models.AudioPlayable
import com.scribd.armadillo.models.DrmType
import javax.inject.Inject
Expand Down Expand Up @@ -46,27 +45,32 @@ internal class DashMediaSourceGenerator @Inject constructor(
)

return if (isDownloaded) {
val drmManager = drmSessionManagerProvider.get(mediaItem)
if(request.drmInfo?.drmType == DrmType.WIDEVINE) {
val drmManager = if (request.drmInfo == null) {
drmSessionManagerProvider.get(mediaItem)
} else null

if (request.drmInfo?.drmType == DrmType.WIDEVINE) {
downloadEngine.redownloadDrmLicense(id = mediaId, request = request)
}
DownloadHelper.createMediaSource(download!!.request, dataSourceFactory, drmManager)
} else {
DashMediaSource.Factory(dataSourceFactory)
.setDrmSessionManagerProvider(drmSessionManagerProvider)
.createMediaSource(mediaItem).also { source ->
//download equivalent is in DashDrmLicenseDownloader
when (request.drmInfo?.drmType) {
DrmType.WIDEVINE -> {
source.addDrmEventListener(
drmHandler,
WidevineSessionEventListener()
)
}

else -> Unit //no DRM
var factory = DashMediaSource.Factory(dataSourceFactory)
if (request.drmInfo != null) {
factory = factory.setDrmSessionManagerProvider(drmSessionManagerProvider)
}
factory.createMediaSource(mediaItem).also { source ->
//download equivalent is in DashDrmLicenseDownloader
when (request.drmInfo?.drmType) {
DrmType.WIDEVINE -> {
source.addDrmEventListener(
drmHandler,
WidevineSessionEventListener()
)
}

else -> Unit //no DRM
}
}
}
}

Expand Down
5 changes: 3 additions & 2 deletions Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/DrmMediaSourceHelper.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ internal class DrmMediaSourceHelperImpl @Inject constructor(private val secureSt
.setUri(request.url)
.apply {
// Apply DRM config if content is DRM-protected
val drmConfig = request.drmInfo?.let { drmInfo ->
request.drmInfo?.let { drmInfo ->
MediaItem.DrmConfiguration.Builder(drmInfo.drmType.toExoplayerConstant())
.setLicenseUri(drmInfo.licenseServer)
.setLicenseRequestHeaders(drmInfo.drmHeaders)
Expand All @@ -46,8 +46,9 @@ internal class DrmMediaSourceHelperImpl @Inject constructor(private val secureSt
}
}
.build()
}?.let { drmConfig ->
setDrmConfiguration(drmConfig)
}
setDrmConfiguration(drmConfig)
}
.build()
}
3 changes: 2 additions & 1 deletion Armadillo/src/test/java/com/scribd/armadillo/ArmadilloPlayerChoreographerTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import org.robolectric.annotation.Config
// Fixes issue in mockito. typealias is insufficient https://github.com/nhaarman/mockito-kotlin/issues/272#issuecomment-513971465
private interface Callback : (MediaControllerCompat.TransportControls) -> Unit

@Config(manifest = Config.NONE)
@Config(manifest = Config.NONE, sdk = [25])
@RunWith(RobolectricTestRunner::class)
class ArmadilloPlayerChoreographerTest {
@Rule
Expand Down Expand Up @@ -90,6 +90,7 @@ class ArmadilloPlayerChoreographerTest {
)
val newRequest = AudioPlayable.MediaRequest.createHttpUri(newUrl, newHeaders)
choreographer.updateMediaRequest(newRequest)
ArmadilloPlayerChoreographer.handler.hasMessages(1) //magic looper processor

verify(choreographer.stateModifier).dispatch(MediaRequestUpdateAction(newRequest))
verify(transportControls).sendCustomAction(eq("update_media_request"), argWhere {
Expand Down
4 changes: 4 additions & 0 deletions RELEASE.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Project Armadillo Release Notes

## 1.6.5
- ArmadilloPlayer handles client calls from any thread appropriately, without blocking. For those recently updating since 1.5.1, this
should resolve any strange bugs from client behavior.

## 1.6.4
- Resolves download issue resulting in downloaded storage often being out of alignment

Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ org.gradle.jvmargs=-Xmx1536m
# org.gradle.parallel=true
PACKAGE_NAME=com.scribd.armadillo
GRADLE_PLUGIN_VERSION=7.2.0
LIBRARY_VERSION=1.6.4
LIBRARY_VERSION=1.6.5
EXOPLAYER_VERSION=2.19.1
RXJAVA_VERSION=2.2.4
RXANDROID_VERSION=2.0.1
Expand Down

0 comments on commit b2f97df

Please sign in to comment.