diff --git a/Armadillo/src/main/java/com/scribd/armadillo/ArmadilloPlayerChoreographer.kt b/Armadillo/src/main/java/com/scribd/armadillo/ArmadilloPlayerChoreographer.kt index caa19b3..597ddf4 100644 --- a/Armadillo/src/main/java/com/scribd/armadillo/ArmadilloPlayerChoreographer.kt +++ b/Armadillo/src/main/java/com/scribd/armadillo/ArmadilloPlayerChoreographer.kt @@ -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 @@ -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] */ @@ -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 @@ -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 - get() = stateProvider.stateSubject + get() = stateProvider.stateSubject override val armadilloStateObservable: Observable get() = armadilloStateSubject.observeOn(AndroidSchedulers.mainThread()) @@ -235,6 +254,18 @@ 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() @@ -242,27 +273,27 @@ internal class ArmadilloPlayerChoreographer : ArmadilloPlayer { 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) @@ -279,25 +310,23 @@ 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) { + override fun updatePlaybackMetadata(title: String, chapters: List) = 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 @@ -305,58 +334,56 @@ internal class ArmadilloPlayerChoreographer : ArmadilloPlayer { 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) } @@ -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 diff --git a/Armadillo/src/main/java/com/scribd/armadillo/di/PlaybackModule.kt b/Armadillo/src/main/java/com/scribd/armadillo/di/PlaybackModule.kt index a8191b5..529e4d5 100644 --- a/Armadillo/src/main/java/com/scribd/armadillo/di/PlaybackModule.kt +++ b/Armadillo/src/main/java/com/scribd/armadillo/di/PlaybackModule.kt @@ -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 @@ -21,8 +20,8 @@ import com.scribd.armadillo.playback.PlaybackStateBuilderImpl import com.scribd.armadillo.playback.PlaybackStateCompatBuilder import com.scribd.armadillo.playback.mediasource.DrmMediaSourceHelper import com.scribd.armadillo.playback.mediasource.DrmMediaSourceHelperImpl -import com.scribd.armadillo.playback.mediasource.HeadersMediaSourceHelper -import com.scribd.armadillo.playback.mediasource.HeadersMediaSourceHelperImpl +import com.scribd.armadillo.playback.mediasource.HeadersMediaSourceFactoryFactory +import com.scribd.armadillo.playback.mediasource.HeadersMediaSourceFactoryFactoryImpl import com.scribd.armadillo.playback.mediasource.MediaSourceRetriever import com.scribd.armadillo.playback.mediasource.MediaSourceRetrieverImpl import dagger.Module @@ -67,7 +66,7 @@ internal class PlaybackModule { @Provides @Singleton - fun mediaSourceHelper(mediaSourceHelperImpl: HeadersMediaSourceHelperImpl): HeadersMediaSourceHelper = mediaSourceHelperImpl + fun mediaSourceHelper(mediaSourceHelperImpl: HeadersMediaSourceFactoryFactoryImpl): HeadersMediaSourceFactoryFactory = mediaSourceHelperImpl @Provides @Singleton diff --git a/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/DashMediaSourceGenerator.kt b/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/DashMediaSourceGenerator.kt index 4c41bb7..c3a3e28 100644 --- a/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/DashMediaSourceGenerator.kt +++ b/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/DashMediaSourceGenerator.kt @@ -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 @@ -20,7 +19,7 @@ import javax.inject.Inject /** For playback, both streaming and downloaded */ internal class DashMediaSourceGenerator @Inject constructor( context: Context, - private val mediaSourceHelper: HeadersMediaSourceHelper, + private val mediaSourceFactoryFactory: HeadersMediaSourceFactoryFactory, private val downloadTracker: DownloadTracker, private val drmMediaSourceHelper: DrmMediaSourceHelper, private val drmSessionManagerProvider: DrmSessionManagerProvider, @@ -34,7 +33,7 @@ internal class DashMediaSourceGenerator @Inject constructor( if (request.drmInfo != null) { stateStore.dispatch(OpeningLicenseAction(request.drmInfo.drmType)) } - val dataSourceFactory = mediaSourceHelper.createDataSourceFactory(context, request) + val dataSourceFactory = mediaSourceFactoryFactory.createDataSourceFactory(context, request) val download = downloadTracker.getDownload(id = mediaId, uri = request.url) val isDownloaded = download != null && download.state == Download.STATE_COMPLETED @@ -46,29 +45,35 @@ 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 } + } } } - override fun updateMediaSourceHeaders(request: AudioPlayable.MediaRequest) = mediaSourceHelper.updateMediaSourceHeaders(request) + override fun updateMediaSourceHeaders(request: AudioPlayable.MediaRequest) = + mediaSourceFactoryFactory.updateMediaSourceHeaders(request) } \ No newline at end of file diff --git a/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/DrmMediaSourceHelper.kt b/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/DrmMediaSourceHelper.kt index f500ff2..e43be2f 100644 --- a/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/DrmMediaSourceHelper.kt +++ b/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/DrmMediaSourceHelper.kt @@ -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) @@ -46,8 +46,9 @@ internal class DrmMediaSourceHelperImpl @Inject constructor(private val secureSt } } .build() + }?.let { drmConfig -> + setDrmConfiguration(drmConfig) } - setDrmConfiguration(drmConfig) } .build() } diff --git a/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/HeadersMediaSourceHelper.kt b/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/HeadersMediaSourceFactoryFactory.kt similarity index 87% rename from Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/HeadersMediaSourceHelper.kt rename to Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/HeadersMediaSourceFactoryFactory.kt index accb10d..f1dc46f 100644 --- a/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/HeadersMediaSourceHelper.kt +++ b/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/HeadersMediaSourceFactoryFactory.kt @@ -4,7 +4,7 @@ import android.content.Context import com.google.android.exoplayer2.upstream.DataSource import com.scribd.armadillo.models.AudioPlayable -internal interface HeadersMediaSourceHelper { +internal interface HeadersMediaSourceFactoryFactory { fun createDataSourceFactory(context: Context, request: AudioPlayable.MediaRequest): DataSource.Factory fun updateMediaSourceHeaders(request: AudioPlayable.MediaRequest) } \ No newline at end of file diff --git a/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/HeadersMediaSourceHelperImpl.kt b/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/HeadersMediaSourceFactoryFactoryImpl.kt similarity index 95% rename from Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/HeadersMediaSourceHelperImpl.kt rename to Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/HeadersMediaSourceFactoryFactoryImpl.kt index 644333c..ee9321d 100644 --- a/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/HeadersMediaSourceHelperImpl.kt +++ b/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/HeadersMediaSourceFactoryFactoryImpl.kt @@ -12,10 +12,10 @@ import javax.inject.Inject import javax.inject.Singleton @Singleton -internal class HeadersMediaSourceHelperImpl @Inject constructor( +internal class HeadersMediaSourceFactoryFactoryImpl @Inject constructor( private val cacheManager: CacheManager, private val headersStore: HeadersStore -): HeadersMediaSourceHelper { +): HeadersMediaSourceFactoryFactory { private val previousRequests = mutableMapOf() override fun createDataSourceFactory(context: Context, request: AudioPlayable.MediaRequest): DataSource.Factory { diff --git a/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/HlsMediaSourceGenerator.kt b/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/HlsMediaSourceGenerator.kt index 8893c95..3b39aaa 100644 --- a/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/HlsMediaSourceGenerator.kt +++ b/Armadillo/src/main/java/com/scribd/armadillo/playback/mediasource/HlsMediaSourceGenerator.kt @@ -8,7 +8,6 @@ import com.google.android.exoplayer2.offline.DownloadHelper import com.google.android.exoplayer2.source.MediaSource import com.google.android.exoplayer2.source.hls.HlsMediaSource import com.scribd.armadillo.download.DownloadTracker -import com.scribd.armadillo.extensions.toUri import com.scribd.armadillo.models.AudioPlayable import javax.inject.Inject @@ -17,7 +16,7 @@ import javax.inject.Inject * */ internal class HlsMediaSourceGenerator @Inject constructor( - private val mediaSourceHelper: HeadersMediaSourceHelper, + private val mediaSourceHelper: HeadersMediaSourceFactoryFactory, private val downloadTracker: DownloadTracker) : MediaSourceGenerator { diff --git a/Armadillo/src/test/java/com/scribd/armadillo/ArmadilloPlayerChoreographerTest.kt b/Armadillo/src/test/java/com/scribd/armadillo/ArmadilloPlayerChoreographerTest.kt index 79c5753..b850361 100644 --- a/Armadillo/src/test/java/com/scribd/armadillo/ArmadilloPlayerChoreographerTest.kt +++ b/Armadillo/src/test/java/com/scribd/armadillo/ArmadilloPlayerChoreographerTest.kt @@ -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 @@ -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 { diff --git a/RELEASE.md b/RELEASE.md index 6aaf8ef..2e73592 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -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 diff --git a/gradle.properties b/gradle.properties index d2d2fc3..a95d5d6 100644 --- a/gradle.properties +++ b/gradle.properties @@ -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