Mitigate missing aspect ratio issue (#507)

pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/PlayerCallbackFlow.kt

@@ -5,7 +5,6 @@
 package ch.srgssr.pillarbox.player
 
 import androidx.media3.common.C
-import androidx.media3.common.Format
 import androidx.media3.common.MediaItem
 import androidx.media3.common.MediaMetadata
 import androidx.media3.common.PlaybackException
@@ -17,6 +16,7 @@ import androidx.media3.common.TrackSelectionParameters
 import androidx.media3.common.Tracks
 import androidx.media3.common.VideoSize
 import ch.srgssr.pillarbox.player.asset.Chapter
+import ch.srgssr.pillarbox.player.extension.computeAspectRatioOrNull
 import ch.srgssr.pillarbox.player.extension.getChapterAtPosition
 import ch.srgssr.pillarbox.player.extension.getCurrentMediaItems
 import ch.srgssr.pillarbox.player.extension.getPlaybackSpeed
@@ -28,6 +28,7 @@ import kotlinx.coroutines.currentCoroutineContext
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.callbackFlow
+import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.flow
 import kotlinx.coroutines.flow.map
@@ -295,10 +296,16 @@ fun Player.videoSizeAsFlow(): Flow<VideoSize> = callbackFlow {
  *
  * @param defaultAspectRatio The aspect ratio when the video size is unknown, or for audio content.
  */
+@OptIn(ExperimentalCoroutinesApi::class)
 fun Player.getAspectRatioAsFlow(defaultAspectRatio: Float): Flow<Float> {
-    return getCurrentTracksAsFlow()
-        .map { it.getVideoAspectRatioOrElse(defaultAspectRatio) }
-        .distinctUntilChanged()
+    return combine(
+        getCurrentTracksAsFlow(),
+        videoSizeAsFlow(),
+    ) { currentTracks, videoSize ->
+        currentTracks.getVideoAspectRatioOrNull()
+            ?: videoSize.computeAspectRatioOrNull()
+            ?: defaultAspectRatio
+    }.distinctUntilChanged()
 }
 
 /**
@@ -403,11 +410,11 @@ private suspend fun <T> ProducerScope<T>.addPlayerListener(player: Player, liste
     }
 }
 
-private fun Tracks.getVideoAspectRatioOrElse(defaultAspectRatio: Float): Float {
+private fun Tracks.getVideoAspectRatioOrNull(): Float? {
     val format = videoTracks.find { it.isSelected }?.format
 
-    return if (format == null || format.height <= 0 || format.width == Format.NO_VALUE) {
-        defaultAspectRatio
+    return if (format == null || format.height <= 0 || format.width <= 0) {
+        null
     } else {
         format.width * format.pixelWidthHeightRatio / format.height.toFloat()
     }

pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/extension/VideoSize.kt

@@ -8,12 +8,10 @@ import android.util.Rational
 import androidx.media3.common.VideoSize
 
 /**
- * Compute aspect ratio, return [unknownAspectRatioValue] if aspect ratio can't be computed.
- *
- * @param unknownAspectRatioValue
+ * Compute the aspect ratio, return `null` if the aspect ratio can't be computed.
  */
-fun VideoSize.computeAspectRatio(unknownAspectRatioValue: Float): Float {
-    return if (height == 0 || width == 0) unknownAspectRatioValue else width * this.pixelWidthHeightRatio / height
+fun VideoSize.computeAspectRatioOrNull(): Float? {
+    return if (height == 0 || width == 0) null else width * this.pixelWidthHeightRatio / height
 }
 
 /**

pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/TestPlayerCallbackFlow.kt

@@ -432,6 +432,54 @@ class TestPlayerCallbackFlow {
         }
     }
 
+    @Test
+    fun `get aspect ratio as flow, video tracks no video size, has video size`() = runTest {
+        val videoTracks = Tracks(
+            listOf(
+                createTrackGroup(
+                    selectedIndex = 1,
+                    createVideoFormat("v1", width = Format.NO_VALUE, height = Format.NO_VALUE),
+                    createVideoFormat("v2", width = Format.NO_VALUE, height = Format.NO_VALUE),
+                    createVideoFormat("v3", width = Format.NO_VALUE, height = Format.NO_VALUE),
+                )
+            )
+        )
+
+        val fakePlayer = PlayerListenerCommander(player)
+
+        fakePlayer.getAspectRatioAsFlow(0f).test {
+            fakePlayer.onVideoSizeChanged(VideoSize(1920, 1080))
+            fakePlayer.onTracksChanged(videoTracks)
+
+            assertEquals(0f, awaitItem())
+            assertEquals(16 / 9f, awaitItem())
+            ensureAllEventsConsumed()
+        }
+    }
+
+    @Test
+    fun `get aspect ratio as flow, video tracks no video size, no video size`() = runTest {
+        val videoTracks = Tracks(
+            listOf(
+                createTrackGroup(
+                    selectedIndex = 1,
+                    createVideoFormat("v1", width = Format.NO_VALUE, height = Format.NO_VALUE),
+                    createVideoFormat("v2", width = Format.NO_VALUE, height = Format.NO_VALUE),
+                    createVideoFormat("v3", width = Format.NO_VALUE, height = Format.NO_VALUE),
+                )
+            )
+        )
+
+        val fakePlayer = PlayerListenerCommander(player)
+
+        fakePlayer.getAspectRatioAsFlow(0f).test {
+            fakePlayer.onTracksChanged(videoTracks)
+
+            assertEquals(0f, awaitItem())
+            ensureAllEventsConsumed()
+        }
+    }
+
     @Test
     fun `get aspect ratio as flow, video tracks without selection`() = runTest {
         val videoTracksWithoutSelection = Tracks(

pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/extension/VideoSizeTest.kt

@@ -10,71 +10,60 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
 import org.junit.runner.RunWith
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertNull
 
 @RunWith(AndroidJUnit4::class)
 class VideoSizeTest {
     @Test
     fun `computeAspectRatio unknown video size`() {
         val input = VideoSize.UNKNOWN
-        val expectedAspectRatio = 16f / 9
-        val unknownAspectRatio = 16f / 9
-        assertEquals(expectedAspectRatio, input.computeAspectRatio(unknownAspectRatio))
+        assertNull(input.computeAspectRatioOrNull())
     }
 
     @Test
     fun `computeAspectRatio with height set to 0`() {
         val input = VideoSize(0, 100)
-        val expectedAspectRatio = 16f / 9
-        val unknownAspectRatio = 16f / 9
-        assertEquals(expectedAspectRatio, input.computeAspectRatio(unknownAspectRatio))
+        assertNull(input.computeAspectRatioOrNull())
     }
 
     @Test
     fun `computeAspectRatio with width set to 0`() {
         val input = VideoSize(240, 0)
-        val expectedAspectRatio = 16f / 9
-        val unknownAspectRatio = 16f / 9
-        assertEquals(expectedAspectRatio, input.computeAspectRatio(unknownAspectRatio))
+        assertNull(input.computeAspectRatioOrNull())
     }
 
     @Test
     fun `computeAspectRatio with width and height set to 0`() {
         val input = VideoSize(0, 0)
-        val expectedAspectRatio = 16f / 9
-        val unknownAspectRatio = 16f / 9
-        assertEquals(expectedAspectRatio, input.computeAspectRatio(unknownAspectRatio))
+        assertNull(input.computeAspectRatioOrNull())
     }
 
     @Test
     fun `computeAspectRatio with a 16-9 aspect ratio`() {
         val input = VideoSize(1920, 1080)
         val expectedAspectRatio = 16f / 9
-        val unknownAspectRatio = 1f
-        assertEquals(expectedAspectRatio, input.computeAspectRatio(unknownAspectRatio))
+        assertEquals(expectedAspectRatio, input.computeAspectRatioOrNull())
     }
 
     @Test
     fun `computeAspectRatio with a 9-16 aspect ratio`() {
         val input = VideoSize(1080, 1920)
         val expectedAspectRatio = 9f / 16
-        val unknownAspectRatio = 1f
-        assertEquals(expectedAspectRatio, input.computeAspectRatio(unknownAspectRatio))
+        assertEquals(expectedAspectRatio, input.computeAspectRatioOrNull())
     }
 
     @Test
     fun `computeAspectRatio with a 4-3 aspect ratio`() {
         val input = VideoSize(800, 600)
         val expectedAspectRatio = 4f / 3
-        val unknownAspectRatio = 1f
-        assertEquals(expectedAspectRatio, input.computeAspectRatio(unknownAspectRatio))
+        assertEquals(expectedAspectRatio, input.computeAspectRatioOrNull())
     }
 
     @Test
     fun `computeAspectRatio with a square aspect ratio`() {
         val input = VideoSize(500, 500)
         val expectedAspectRatio = 1f
-        val unknownAspectRatio = 0f
-        assertEquals(expectedAspectRatio, input.computeAspectRatio(unknownAspectRatio))
+        assertEquals(expectedAspectRatio, input.computeAspectRatioOrNull())
     }
 
     @Test

pillarbox-ui/src/main/java/ch/srgssr/pillarbox/ui/widget/player/PlayerSurface.kt

@@ -54,7 +54,7 @@ fun PlayerSurface(
     displayDebugView: Boolean = false,
     surfaceContent: @Composable (BoxScope.() -> Unit)? = { ExoPlayerSubtitleView(player = player) },
 ) {
-    var lastKnownVideoAspectRatio by remember { mutableFloatStateOf(defaultAspectRatio ?: 0f) }
+    var lastKnownVideoAspectRatio by remember { mutableFloatStateOf(defaultAspectRatio ?: 1f) }
     val videoAspectRatio by player.getAspectRatioAsState(defaultAspectRatio = lastKnownVideoAspectRatio)
 
     // If the media has tracks, but no video tracks, we reset the aspect ratio to 0