diff --git a/android/app/src/androidTest/assets/ffmpeg/truehd_mismatch_repro.mka b/android/app/src/androidTest/assets/ffmpeg/truehd_mismatch_repro.mka new file mode 100644 index 00000000..fe391789 Binary files /dev/null and b/android/app/src/androidTest/assets/ffmpeg/truehd_mismatch_repro.mka differ diff --git a/android/app/src/androidTest/kotlin/com/edde746/plezy/exoplayer/TrueHdSpeedTransitionTest.kt b/android/app/src/androidTest/kotlin/com/edde746/plezy/exoplayer/TrueHdSpeedTransitionTest.kt index 3743fbee..0153fd9e 100644 --- a/android/app/src/androidTest/kotlin/com/edde746/plezy/exoplayer/TrueHdSpeedTransitionTest.kt +++ b/android/app/src/androidTest/kotlin/com/edde746/plezy/exoplayer/TrueHdSpeedTransitionTest.kt @@ -5,7 +5,6 @@ import android.os.Handler import android.os.HandlerThread import android.util.Log import androidx.media3.common.MediaItem -import androidx.media3.common.Player import androidx.media3.datasource.DefaultDataSource import androidx.media3.exoplayer.DefaultRenderersFactory import androidx.media3.exoplayer.ExoPlayer @@ -185,11 +184,128 @@ class TrueHdSpeedTransitionTest { fixture.delete() } - private fun copyFixture(context: android.content.Context): File { + private fun copyFixture( + context: android.content.Context, + asset: String = "ffmpeg/truehd_speed_repro.mka" + ): File { val output = File.createTempFile("truehd-speed-", null, context.cacheDir) InstrumentationRegistry.getInstrumentation().context.assets - .open("ffmpeg/truehd_speed_repro.mka") + .open(asset) .use { input -> output.outputStream().use { input.copyTo(it) } } return output } + + /** + * The container announces 48kHz, which selection trusts, but the bitstream is genuinely 44.1kHz + * TrueHD, which the carrier does not cover (#1804). The packer emits nothing in that state, so + * the sink has to hand the stream to the decoder rather than play silence. + */ + @Test + fun aRateFamilyMismatchFallsBackToTheDecoderInsteadOfGoingSilent() { + val context = InstrumentationRegistry.getInstrumentation().targetContext + if (!supportsTrueHdMatCarrier(context)) { + Log.i(TAG, "==== MISMATCH SKIPPED: device has no carrier route ====") + return + } + val fixture = copyFixture(context, "ffmpeg/truehd_mismatch_repro.mka") + val thread = HandlerThread("truehd-mismatch-test").apply { start() } + val handler = Handler(thread.looper) + + val playing = CountDownLatch(1) + val audioDecoder = AtomicReference(null) + val trackRate = AtomicReference(-1) + val rateSequence = java.util.Collections.synchronizedList(mutableListOf()) + val diagnostics = java.util.Collections.synchronizedList(mutableListOf()) + val player = AtomicReference(null) + + handler.post { + val factory = PlezyRenderersFactory(context).apply { + setEnableDecoderFallback(true) + setExtensionRendererMode(DefaultRenderersFactory.EXTENSION_RENDERER_MODE_ON) + audioDiagnosticsLogger = { _, _, message -> + diagnostics.add(message) + Log.i(TAG, "mismatch sink: $message") + } + } + val selector = DefaultTrackSelector(context).apply { + setParameters( + buildUponParameters().setAllowInvalidateSelectionsOnRendererCapabilitiesChange(true) + ) + } + val exo = ExoPlayer.Builder(context, factory).setTrackSelector(selector).build() + player.set(exo) + exo.addAnalyticsListener( + object : AnalyticsListener { + override fun onAudioDecoderInitialized( + eventTime: AnalyticsListener.EventTime, + decoderName: String, + initializedTimestampMs: Long, + initializationDurationMs: Long + ) { + audioDecoder.set(decoderName) + Log.i(TAG, "mismatch audio decoder: $decoderName") + } + + override fun onAudioTrackInitialized( + eventTime: AnalyticsListener.EventTime, + config: AudioSink.AudioTrackConfig + ) { + trackRate.set(config.sampleRate) + rateSequence.add(config.sampleRate) + Log.i(TAG, "mismatch AudioTrack rate=${config.sampleRate}") + } + + override fun onAudioInputFormatChanged( + eventTime: AnalyticsListener.EventTime, + format: androidx.media3.common.Format, + decoderReuseEvaluation: androidx.media3.exoplayer.DecoderReuseEvaluation? + ) { + Log.i(TAG, "mismatch INPUT format: mime=${format.sampleMimeType} rate=${format.sampleRate} ch=${format.channelCount}") + } + + override fun onIsPlayingChanged(eventTime: AnalyticsListener.EventTime, isPlaying: Boolean) { + if (isPlaying) playing.countDown() + } + } + ) + val source = ProgressiveMediaSource.Factory( + DefaultDataSource.Factory(context), + DefaultExtractorsFactory() + ).createMediaSource(MediaItem.fromUri(Uri.fromFile(fixture))) + exo.setMediaSource(source) + exo.prepare() + exo.playWhenReady = true + } + + assertTrue("playback never started", playing.await(30, TimeUnit.SECONDS)) + Thread.sleep(SETTLE_MS) + val positionFirst = positionOf(handler, player) + Thread.sleep(SETTLE_MS) + val positionSecond = positionOf(handler, player) + + val decoder = audioDecoder.get() + val rate = trackRate.get() + Log.i( + TAG, + "==== MISMATCH RESULT: rates=$rateSequence decoder=$decoder rate=$rate " + + "position=${positionFirst}ms -> ${positionSecond}ms ====" + ) + teardown(handler, player, thread, fixture) + + // Without this the test would also pass on a build where the carrier was never selected at all: + // the mismatch fires on the first access unit, before the carrier writes a burst, so no 192kHz + // AudioTrack is ever opened and the rate sequence alone cannot tell the two apart. + assertTrue( + "the carrier must have been entered and then left at runtime, saw: $diagnostics", + diagnostics.any { it.contains("MAT/IEC 61937 carrier") } && + diagnostics.any { it.contains("44.1kHz-family rate its container did not") } + ) + assertTrue("a decoder must take the stream over instead of the carrier", decoder != null) + assertNotEquals( + "the stream must not still be riding the carrier", + TrueHdMatPacker.CARRIER_SAMPLE_RATE, + rate + ) + assertTrue("playback must keep advancing after the fallback", positionSecond > positionFirst) + } } diff --git a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerCore.kt b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerCore.kt index e751e55f..0985842b 100644 --- a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerCore.kt +++ b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerCore.kt @@ -3442,6 +3442,11 @@ class ExoPlayerCore(private val activity: Activity) : ) emitSeekable(false, force = true) + // Only here: this is the one caller that is a genuinely new item. The recovery, DV-mode and + // subtitle reloads all reuse setCurrentMediaSource for the *same* stream, and clearing + // per-stream audio decisions there would undo them and loop. + renderersFactory?.beginMediaItem() + exoPlayer?.apply { setCurrentMediaSource(this, uri, startPositionMs) prepare() diff --git a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/PlezyRenderersFactory.kt b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/PlezyRenderersFactory.kt index dc5c6514..c5cc95fd 100644 --- a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/PlezyRenderersFactory.kt +++ b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/PlezyRenderersFactory.kt @@ -151,6 +151,16 @@ class PlezyRenderersFactory(context: Context) : DefaultRenderersFactory(context) ) } + private var trueHdCarrierSink: TrueHdCarrierSink? = null + + /** + * Clears per-stream carrier state that must survive renderer resets but not a new media item. + * Call before setting a new source; see [TrueHdCarrierSink.beginMediaItem]. + */ + fun beginMediaItem() { + trueHdCarrierSink?.beginMediaItem() + } + override fun buildAudioSink( context: Context, enableFloatOutput: Boolean, @@ -201,7 +211,7 @@ class PlezyRenderersFactory(context: Context) : DefaultRenderersFactory(context) carrierRouteAvailable = { supportsTrueHdMatCarrier(context) }, directOutputBlocked = { format -> shouldBlockDirectAudioOutput?.invoke(format) == true }, log = audioDiagnosticsLogger - ) + ).also { trueHdCarrierSink = it } } /** diff --git a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/TrueHdCarrierSink.kt b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/TrueHdCarrierSink.kt index 585c4379..3eb9234a 100644 --- a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/TrueHdCarrierSink.kt +++ b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/TrueHdCarrierSink.kt @@ -13,6 +13,7 @@ import androidx.media3.common.util.UnstableApi import androidx.media3.exoplayer.analytics.PlayerId import androidx.media3.exoplayer.audio.AudioSink import java.nio.ByteBuffer +import java.util.concurrent.atomic.AtomicInteger /** * Routes Dolby TrueHD through a MAT/IEC 61937 carrier, and everything else through the normal sink @@ -78,6 +79,33 @@ internal class TrueHdCarrierSink( private var playbackParameters: PlaybackParameters = PlaybackParameters.DEFAULT private var sinkListener: AudioSink.Listener? = null + /** + * Latched when a stream's bitstream contradicts the rate its container announced, which selection + * was made from. + * + * Deliberately outlives [flush] and [reset]: media3 resets every renderer disabled by a new + * selection before enabling the replacement (ExoPlayerImplInternal.enableRenderers), and both + * audio renderers share this sink, so the outgoing renderer's reset lands here in the middle of + * the very handover this latch exists to cause. Clearing it there would re-offer the carrier and + * loop straight back into the mismatch. + * + * The real boundary is a new media item, which only the caller knows and which happens before + * selection asks anything. [beginMediaItem] is that hook; without it one malformed item would + * cost bitstreaming for every later item sharing the player. + * + * It is held as a generation rather than a flag because that hook is called on the app thread + * while the mismatch is discovered on the playback thread. A late buffer from the outgoing stream + * can land after the new item began; latching the generation the carrier was configured for makes + * that write name the stream it belongs to instead of poisoning its successor. + */ + private val mediaGeneration = AtomicInteger(0) + + /** Generation [mediaGeneration] held when the carrier was last configured; playback thread. */ + private var configuredGeneration = 0 + + @Volatile + private var mismatchGeneration = -1 + // --- Selection --- /** @@ -93,6 +121,7 @@ internal class TrueHdCarrierSink( */ private fun shouldUseCarrier(format: Format): Boolean { if (format.sampleMimeType != MimeTypes.AUDIO_TRUEHD) return false + if (mismatchGeneration == mediaGeneration.get()) return false if (!isCarrierRateFamily(format.sampleRate)) return false if (playbackParameters.speed != 1f) return false if (directOutputBlocked(format)) return false @@ -153,6 +182,7 @@ internal class TrueHdCarrierSink( ) } carrierActive = useCarrier + configuredGeneration = mediaGeneration.get() active = if (useCarrier) carrierSink else defaultSink discardCarrierState() @@ -193,6 +223,23 @@ internal class TrueHdCarrierSink( return true } val burst = packer.packAccessUnit(remaining, offset, length) + if (packer.unsupportedRateFamily) { + // Selection is made from Format.sampleRate, so the bitstream disagrees with its container. + // The packer emits nothing in that state; consuming here would turn the stream into + // silence. Leave this unit in the buffer, latch the carrier off, and ask for reselection so + // the decoder takes over and receives it. + if (mismatchGeneration != configuredGeneration) { + mismatchGeneration = configuredGeneration + log?.invoke( + "warn", + "audio", + "TrueHD bitstream announced a 44.1kHz-family rate its container did not; " + + "leaving the carrier so it decodes" + ) + sinkListener?.onAudioCapabilitiesChanged() + } + return false + } offset += length buffer.position(buffer.position() + length) if (burst == null) continue @@ -205,16 +252,6 @@ internal class TrueHdCarrierSink( return false } } - - if (packer.unsupportedRateFamily) { - // Selection is made from Format.sampleRate, so this means the bitstream disagrees with the - // container. Nothing is packed in that state, which would be silence rather than a glitch. - log?.invoke( - "warn", - "audio", - "TrueHD bitstream announced a 44.1kHz-family rate the container did not; carrier is producing nothing" - ) - } return true } @@ -366,7 +403,16 @@ internal class TrueHdCarrierSink( carrierSink.reset() } + /** + * Called by the owner immediately before a new media item is set, which is the only point where + * "this stream lied about its rate" stops being true and the carrier can be offered again. + */ + fun beginMediaItem() { + mediaGeneration.incrementAndGet() + } + override fun release() { + mediaGeneration.incrementAndGet() discardCarrierState() defaultSink.release() carrierSink.release() diff --git a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/TrueHdMatPacker.kt b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/TrueHdMatPacker.kt index 19d1dcab..11f6054f 100644 --- a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/TrueHdMatPacker.kt +++ b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/TrueHdMatPacker.kt @@ -105,6 +105,9 @@ internal class TrueHdMatPacker { previousTiming = 0 previousSize = 0 samplesPerFrame = 0 + // The family is re-learned from the next major sync. Leaving it latched here would make every + // later stream on this packer emit nothing. + unsupportedRateFamily = false java.util.Arrays.fill(matBuffers[0], 0) java.util.Arrays.fill(matBuffers[1], 0) } diff --git a/android/app/src/test/kotlin/com/edde746/plezy/exoplayer/TrueHdCarrierSinkTest.kt b/android/app/src/test/kotlin/com/edde746/plezy/exoplayer/TrueHdCarrierSinkTest.kt index 91df4936..4848914c 100644 --- a/android/app/src/test/kotlin/com/edde746/plezy/exoplayer/TrueHdCarrierSinkTest.kt +++ b/android/app/src/test/kotlin/com/edde746/plezy/exoplayer/TrueHdCarrierSinkTest.kt @@ -154,6 +154,130 @@ class TrueHdCarrierSinkTest { assertEquals(0, listener.capabilityInvalidations) } + // --- Rate-family mismatch discovered mid-stream --- + + /** + * Selection reads Format.sampleRate; the rate family is only certain once a major sync is parsed. + * When they disagree the packer emits nothing, so consuming the input would turn the stream into + * silence. The unit has to survive for whoever takes over. + */ + @Test + fun aRateFamilyMismatchLeavesTheCarrierInsteadOfGoingSilent() { + val carrier = FakeSink() + val carrierSink = sink(carrier = carrier) + val listener = RecordingSinkListener() + carrierSink.setListener(listener) + carrierSink.configure(trueHdFormat(), 0, null) + + val buffer = ByteBuffer.wrap(fortyFourFamilyAccessUnits()) + val accepted = carrierSink.handleBuffer(buffer, 0L, 1) + + assertFalse("a mismatch must apply back pressure, not report success", accepted) + assertEquals("the offending access unit must stay in the buffer", 0, buffer.position()) + assertEquals("the renderer must be asked to reselect", 1, listener.capabilityInvalidations) + assertEquals( + "TrueHD must now decode rather than ride the carrier", + AudioSink.SINK_FORMAT_UNSUPPORTED, + carrierSink.getFormatSupport(trueHdFormat()) + ) + assertEquals("nothing may reach the carrier delegate", 0, carrier.written.size()) + } + + /** + * media3 resets every renderer disabled by a new selection before enabling the replacement, and + * both audio renderers share this sink. That reset lands mid-handover, so a latch cleared there + * would re-offer the carrier and loop straight back into the mismatch. + */ + @Test + fun theMismatchLatchSurvivesTheResetThatAccompaniesTheHandover() { + val carrierSink = sink() + carrierSink.setListener(RecordingSinkListener()) + carrierSink.configure(trueHdFormat(), 0, null) + carrierSink.handleBuffer(ByteBuffer.wrap(fortyFourFamilyAccessUnits()), 0L, 1) + + carrierSink.flush() + carrierSink.reset() + + assertEquals( + "the carrier must stay off across the handover", + AudioSink.SINK_FORMAT_UNSUPPORTED, + carrierSink.getFormatSupport(trueHdFormat()) + ) + } + + /** The latch is scoped to the sink, so a released sink starts clean. */ + @Test + fun releasingTheSinkClearsTheMismatchLatch() { + val carrierSink = sink() + carrierSink.setListener(RecordingSinkListener()) + carrierSink.configure(trueHdFormat(), 0, null) + carrierSink.handleBuffer(ByteBuffer.wrap(fortyFourFamilyAccessUnits()), 0L, 1) + + carrierSink.release() + + assertEquals( + AudioSink.SINK_FORMAT_SUPPORTED_DIRECTLY, + carrierSink.getFormatSupport(trueHdFormat()) + ) + } + + /** + * Genuine 44.1kHz TrueHD access units, encoder-produced. Flipping the rate nibble in a 48kHz + * stream would invalidate the major sync checksum, which is not the case being modelled. + */ + private val fortyFourFamilyUnits by lazy { + checkNotNull(javaClass.classLoader?.getResourceAsStream("truehd_441_access_units.bin")) + .use { it.readBytes() } + } + + private fun fortyFourFamilyAccessUnits(): ByteArray = fortyFourFamilyUnits + + /** + * The mismatch is a property of one stream. The owner signals the real boundary, because the sink + * cannot see it: renderer resets and flushes happen constantly within a single item. + */ + @Test + fun aNewMediaItemClearsTheMismatchLatch() { + val carrierSink = sink() + carrierSink.setListener(RecordingSinkListener()) + carrierSink.configure(trueHdFormat(), 0, null) + carrierSink.handleBuffer(ByteBuffer.wrap(fortyFourFamilyAccessUnits()), 0L, 1) + assertEquals( + AudioSink.SINK_FORMAT_UNSUPPORTED, + carrierSink.getFormatSupport(trueHdFormat()) + ) + + carrierSink.beginMediaItem() + + assertEquals( + "a new item must be able to bitstream again", + AudioSink.SINK_FORMAT_SUPPORTED_DIRECTLY, + carrierSink.getFormatSupport(trueHdFormat()) + ) + } + + /** + * The boundary hook runs on the app thread while the mismatch is found on the playback thread, so + * a buffer from the outgoing stream can be handled after the new item began. That write must name + * the stream it came from rather than disabling the carrier for its successor. + */ + @Test + fun aLateMismatchFromThePreviousItemDoesNotPoisonTheNewOne() { + val carrierSink = sink() + carrierSink.setListener(RecordingSinkListener()) + carrierSink.configure(trueHdFormat(), 0, null) + + // The new item starts before the outgoing stream's last buffer is handled. + carrierSink.beginMediaItem() + carrierSink.handleBuffer(ByteBuffer.wrap(fortyFourFamilyAccessUnits()), 0L, 1) + + assertEquals( + "the stale mismatch belongs to the previous item", + AudioSink.SINK_FORMAT_SUPPORTED_DIRECTLY, + carrierSink.getFormatSupport(trueHdFormat()) + ) + } + /** Everything that is not TrueHD keeps going to the existing processed sink. */ @Test fun otherFormatsAreLeftToTheNormalSink() { diff --git a/android/app/src/test/kotlin/com/edde746/plezy/exoplayer/TrueHdMatPackerTest.kt b/android/app/src/test/kotlin/com/edde746/plezy/exoplayer/TrueHdMatPackerTest.kt index 688ea0e6..354c8ee3 100644 --- a/android/app/src/test/kotlin/com/edde746/plezy/exoplayer/TrueHdMatPackerTest.kt +++ b/android/app/src/test/kotlin/com/edde746/plezy/exoplayer/TrueHdMatPackerTest.kt @@ -2,6 +2,7 @@ package com.edde746.plezy.exoplayer import org.junit.Assert.assertArrayEquals import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Test @@ -153,4 +154,27 @@ class TrueHdMatPackerTest { } private fun readLittleEndianShort(data: ByteArray, offset: Int): Int = (data[offset].toInt() and 0xFF) or ((data[offset + 1].toInt() and 0xFF) shl 8) + + /** + * The flag gates every later call, so leaving it latched across a reset would make a packer that + * once saw a 44.1kHz-family stream emit nothing for the rest of its life. + */ + @Test + fun resetClearsTheUnsupportedRateFamilyFlag() { + val packer = TrueHdMatPacker() + val units = resource("truehd_441_access_units.bin") + + val length = TrueHdMatPacker.accessUnitLength(units, 0, units.size) + packer.packAccessUnit(units, 0, length) + assertTrue("the fixture must actually announce the 44.1kHz family", packer.unsupportedRateFamily) + + packer.reset() + + assertFalse("reset must clear the flag", packer.unsupportedRateFamily) + assertArrayEquals( + "a clean packer must reproduce the stream after a poisoned one", + golden, + packAll(accessUnits, TrueHdMatPacker()) + ) + } } diff --git a/android/app/src/test/resources/truehd_441_access_units.bin b/android/app/src/test/resources/truehd_441_access_units.bin new file mode 100644 index 00000000..ea53cda9 Binary files /dev/null and b/android/app/src/test/resources/truehd_441_access_units.bin differ