From b3bce36cccf5695cf9ce7cce8e9fbee80aba8c4c Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Tue, 23 Apr 2024 11:35:11 +0200 Subject: [PATCH] fix: Improve transitions in/out of playing video (#636) Previous code didn't trigger the transition from `ViewMediaActivity` when playing a video until the video had loaded. If the connection was slow or had other issues this resulted in the video "sticking" in the timeline until it loaded. Change this, and trigger the transition immediately. Fixes #598 While looking at this: - Save the play/pause state of the video during a swipe, pause the video, and restore the state when the swipe is cancelled. - Transition the entire video view, to improve the animated transition back to the activity. - Remove the custom progress spinner, use the one provided by the player. - Display the player controller via the layout XML instead of code --- .../app/pachli/fragment/ViewVideoFragment.kt | 59 ++++++++++--------- .../main/res/layout/fragment_view_video.xml | 15 +---- 2 files changed, 33 insertions(+), 41 deletions(-) diff --git a/app/src/main/java/app/pachli/fragment/ViewVideoFragment.kt b/app/src/main/java/app/pachli/fragment/ViewVideoFragment.kt index f3cc8ad4b..04370768a 100644 --- a/app/src/main/java/app/pachli/fragment/ViewVideoFragment.kt +++ b/app/src/main/java/app/pachli/fragment/ViewVideoFragment.kt @@ -44,11 +44,8 @@ import androidx.media3.datasource.okhttp.OkHttpDataSource import androidx.media3.exoplayer.ExoPlayer import androidx.media3.exoplayer.source.DefaultMediaSourceFactory import androidx.media3.exoplayer.util.EventLogger -import androidx.media3.ui.AspectRatioFrameLayout import app.pachli.BuildConfig import app.pachli.R -import app.pachli.core.common.extensions.hide -import app.pachli.core.common.extensions.show import app.pachli.core.common.extensions.viewBinding import app.pachli.core.common.extensions.visible import app.pachli.core.network.model.Attachment @@ -87,8 +84,6 @@ class ViewVideoFragment : ViewMediaFragment() { private lateinit var mediaPlayerListener: Player.Listener private var isAudio = false - private lateinit var mediaAttachment: Attachment - private var player: ExoPlayer? = null /** The saved seek position, if the fragment is being resumed */ @@ -96,6 +91,7 @@ class ViewVideoFragment : ViewMediaFragment() { private lateinit var mediaSourceFactory: DefaultMediaSourceFactory + @Volatile private var startedTransition = false override fun onAttach(context: Context) { @@ -128,16 +124,13 @@ class ViewVideoFragment : ViewMediaFragment() { binding.videoView.controllerShowTimeoutMs = CONTROLS_TIMEOUT.inWholeMilliseconds.toInt() isAudio = attachment.type == Attachment.Type.AUDIO - binding.progressBar.show() /** Handle single taps, flings, and dragging */ val touchListener = object : View.OnTouchListener { var lastY = 0f - /** The view that contains the playing content */ - // binding.videoView is fullscreen, and includes the controls, so don't use that - // when scaling in response to the user dragging on the screen - val contentFrame = binding.videoView.findViewById(androidx.media3.ui.R.id.exo_content_frame) + /** True if the player was playing when the user started touch interactions */ + var wasPlaying: Boolean? = null /** Handle taps and flings */ val simpleGestureDetector = GestureDetectorCompat( @@ -174,18 +167,27 @@ class ViewVideoFragment : ViewMediaFragment() { lastY = event.rawY } else if (event.pointerCount == 1 && event.action == MotionEvent.ACTION_MOVE) { val diff = event.rawY - lastY - if (contentFrame.translationY != 0f || abs(diff) > 40) { - contentFrame.translationY += diff - val scale = (-abs(contentFrame.translationY) / 720 + 1).coerceAtLeast(0.5f) - contentFrame.scaleY = scale - contentFrame.scaleX = scale + if (binding.videoView.translationY != 0f || abs(diff) > 40) { + // Save the playing state, if not already saved + if (wasPlaying == null) { + wasPlaying = player?.isPlaying + player?.pause() + } + + binding.videoView.translationY += diff + val scale = (-abs(binding.videoView.translationY) / 720 + 1).coerceAtLeast(0.5f) + binding.videoView.scaleY = scale + binding.videoView.scaleX = scale lastY = event.rawY } } else if (event.action == MotionEvent.ACTION_UP || event.action == MotionEvent.ACTION_CANCEL) { - if (abs(contentFrame.translationY) > 180) { + if (abs(binding.videoView.translationY) > 180) { + wasPlaying = null mediaActionsListener.onMediaDismiss() } else { - contentFrame.animate().translationY(0f).scaleX(1f).scaleY(1f).start() + binding.videoView.animate().translationY(0f).scaleX(1f).scaleY(1f).start() + if (wasPlaying == true) player?.play() + wasPlaying = null } } @@ -207,9 +209,6 @@ class ViewVideoFragment : ViewMediaFragment() { // be hidden until then. binding.videoView.setOnTouchListener(touchListener) - binding.progressBar.hide() - binding.videoView.useController = true - if (shouldCallMediaReady && !startedTransition) { startedTransition = true mediaActivity.onMediaReady() @@ -238,7 +237,6 @@ class ViewVideoFragment : ViewMediaFragment() { } override fun onPlayerError(error: PlaybackException) { - binding.progressBar.hide() val message = getString( R.string.error_media_playback, error.cause?.message ?: error.message, @@ -250,8 +248,6 @@ class ViewVideoFragment : ViewMediaFragment() { } savedSeekPosition = savedInstanceState?.getLong(SEEK_POSITION) ?: 0 - - mediaAttachment = attachment } override fun onStart() { @@ -315,7 +311,7 @@ class ViewVideoFragment : ViewMediaFragment() { .setMediaSourceFactory(mediaSourceFactory) .build().apply { if (BuildConfig.DEBUG) addAnalyticsListener(EventLogger("${javaClass.simpleName}:ExoPlayer")) - setMediaItem(MediaItem.fromUri(mediaAttachment.url)) + setMediaItem(MediaItem.fromUri(attachment.url)) addListener(mediaPlayerListener) repeatMode = Player.REPEAT_MODE_ONE @@ -331,7 +327,7 @@ class ViewVideoFragment : ViewMediaFragment() { // Audio-only files might have a preview image. If they do, set it as the artwork if (isAudio) { - mediaAttachment.previewUrl?.let { url -> + attachment.previewUrl?.let { url -> Glide.with(this).load(url).into( object : CustomTarget() { override fun onResourceReady( @@ -365,12 +361,17 @@ class ViewVideoFragment : ViewMediaFragment() { binding.videoView.transitionName = attachment.url + if (!startedTransition && shouldCallMediaReady) { + startedTransition = true + mediaActionsListener.onMediaReady() + } + // Clicking the description should play/pause the video binding.mediaDescription.setOnClickListener { - if (binding.videoView.player?.isPlaying == true) { - binding.videoView.player?.pause() + if (player?.isPlaying == true) { + player?.pause() } else { - binding.videoView.player?.play() + player?.play() } } @@ -405,7 +406,7 @@ class ViewVideoFragment : ViewMediaFragment() { } override fun shouldScheduleToolbarHide(): Boolean { - return (binding.videoView.player?.isPlaying == true) && !isAudio + return (player?.isPlaying == true) && !isAudio } companion object { diff --git a/app/src/main/res/layout/fragment_view_video.xml b/app/src/main/res/layout/fragment_view_video.xml index 7e7fdcb35..4ac765f9a 100644 --- a/app/src/main/res/layout/fragment_view_video.xml +++ b/app/src/main/res/layout/fragment_view_video.xml @@ -32,17 +32,8 @@ app:layout_constraintLeft_toLeftOf="parent" app:layout_constraintRight_toRightOf="parent" app:layout_constraintTop_toTopOf="parent" - app:use_controller="false" + app:use_controller="true" app:show_previous_button="false" - app:show_next_button="false" /> - - - + app:show_next_button="false" + app:show_buffering="always" />