Revisit transcoding pipeline

.github/workflows/build.yml

@@ -31,7 +31,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        EMULATOR_API: [23, 25, 29]
+        EMULATOR_API: [24, 27, 29, 31, 34]
     steps:
       - uses: actions/checkout@v4
       - uses: actions/setup-java@v4
@@ -54,10 +54,18 @@ jobs:
           arch: x86_64
           profile: Nexus 6
           emulator-options: -no-snapshot -no-window -no-boot-anim -camera-back none -camera-front none -gpu swiftshader_indirect
-          script: ./.github/workflows/emulator_script.sh
+          script: ./.github/workflows/emulator_script.sh logcat_${{ matrix.EMULATOR_API }}.txt
+
+      - name: Upload emulator logs
+        uses: actions/upload-artifact@v4
+        if: always()
+        with:
+          name: emulator_logs_${{ matrix.EMULATOR_API }}
+          path: ./logcat_${{ matrix.EMULATOR_API }}.txt
 
       - name: Upload emulator tests artifact
         uses: actions/upload-artifact@v4
+        if: always()
         with:
           name: emulator_tests_${{ matrix.EMULATOR_API }}
           path: ./lib/build/reports/androidTests/connected/debug/

.github/workflows/emulator_script.sh

@@ -1,8 +1,6 @@
 #!/usr/bin/env bash
-ADB_TAGS="Transcoder:I Engine:I"
-ADB_TAGS="$ADB_TAGS DefaultVideoStrategy:I DefaultAudioStrategy:I"
-ADB_TAGS="$ADB_TAGS VideoDecoderOutput:I VideoFrameDropper:I"
-ADB_TAGS="$ADB_TAGS AudioEngine:I"
 adb logcat -c
-adb logcat $ADB_TAGS *:E -v color &
+adb logcat *:V > "$1" &
+LOGCAT_PID=$!
+trap "kill $LOGCAT_PID" EXIT
 ./gradlew lib:connectedCheck --stacktrace

lib/src/androidTest/java/com/otaliastudios/transcoder/integration/IssuesTests.kt

@@ -1,16 +1,22 @@
 package com.otaliastudios.transcoder.integration
 
+import android.media.MediaFormat
 import android.media.MediaMetadataRetriever.METADATA_KEY_DURATION
 import android.media.MediaMetadataRetriever
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.platform.app.InstrumentationRegistry
 import com.otaliastudios.transcoder.Transcoder
 import com.otaliastudios.transcoder.TranscoderListener
 import com.otaliastudios.transcoder.TranscoderOptions
+import com.otaliastudios.transcoder.common.TrackType
 import com.otaliastudios.transcoder.internal.utils.Logger
 import com.otaliastudios.transcoder.source.AssetFileDescriptorDataSource
 import com.otaliastudios.transcoder.source.ClipDataSource
 import com.otaliastudios.transcoder.source.FileDescriptorDataSource
+import com.otaliastudios.transcoder.strategy.DefaultVideoStrategy
+import com.otaliastudios.transcoder.validator.WriteAlwaysValidator
+import org.junit.Assume
+import org.junit.AssumptionViolatedException
 import org.junit.Test
 import org.junit.runner.RunWith
 import java.io.File
@@ -24,20 +30,21 @@ class IssuesTests {
         val context = InstrumentationRegistry.getInstrumentation().context
 
         fun output(
-                name: String = System.currentTimeMillis().toString(),
-                extension: String = "mp4"
+            name: String = System.currentTimeMillis().toString(),
+            extension: String = "mp4"
         ) = File(context.cacheDir, "$name.$extension").also { it.parentFile!!.mkdirs() }
 
         fun input(filename: String) = AssetFileDescriptorDataSource(
-                context.assets.openFd("issue_$issue/$filename")
+            context.assets.openFd("issue_$issue/$filename")
         )
 
         fun transcode(
-                output: File = output(),
-                assertTranscoded: Boolean = true,
-                assertDuration: Boolean = true,
-                builder: TranscoderOptions.Builder.() -> Unit,
-        ): File {
+            output: File = output(),
+            assertTranscoded: Boolean = true,
+            assertDuration: Boolean = true,
+            builder: TranscoderOptions.Builder.() -> Unit,
+        ): File = runCatching {
+            Logger.setLogLevel(Logger.LEVEL_VERBOSE)
             val transcoder = Transcoder.into(output.absolutePath)
             transcoder.apply(builder)
             transcoder.setListener(object : TranscoderListener {
@@ -60,11 +67,17 @@ class IssuesTests {
                 retriever.release()
             }
             return output
+        }.getOrElse {
+            if (it.toString().contains("c2.android.avc.encoder was unable to create the input surface (1x1)")) {
+                log.w("Hit known emulator bug. Skipping the test.")
+                throw AssumptionViolatedException("Hit known emulator bug.")
+            }
+            throw it
         }
     }
 
 
-    @Test
+    @Test(timeout = 16000)
     fun issue137() = with(Helper(137)) {
         transcode {
             addDataSource(ClipDataSource(input("main.mp3"), 0L, 1000_000L))
@@ -88,4 +101,22 @@ class IssuesTests {
         }
         Unit
     }
+
+    @Test(timeout = 16000)
+    fun issue184() = with(Helper(184)) {
+        transcode {
+            addDataSource(TrackType.VIDEO, input("transcode.3gp"))
+            setVideoTrackStrategy(DefaultVideoStrategy.exact(400, 400).build())
+        }
+        Unit
+    }
+
+    @Test(timeout = 16000)
+    fun issue102() = with(Helper(102)) {
+        transcode {
+            addDataSource(input("sample.mp4"))
+            setValidator(WriteAlwaysValidator())
+        }
+        Unit
+    }
 }

lib/src/main/java/com/otaliastudios/transcoder/common/TrackType.kt

@@ -2,8 +2,9 @@ package com.otaliastudios.transcoder.common
 
 import android.media.MediaFormat
 
-enum class TrackType {
-    AUDIO, VIDEO
+enum class TrackType(internal val displayName: String) {
+    AUDIO("Audio"), VIDEO("Video");
+
 }
 
 internal val MediaFormat.trackType get() = requireNotNull(trackTypeOrNull) {

lib/src/main/java/com/otaliastudios/transcoder/internal/Codecs.kt

@@ -1,16 +1,18 @@
 package com.otaliastudios.transcoder.internal
 
 import android.media.MediaCodec
+import android.media.MediaCodecList
 import android.media.MediaFormat
-import android.view.Surface
+import android.opengl.EGL14
+import com.otaliastudios.opengl.core.EglCore
+import com.otaliastudios.opengl.surface.EglWindowSurface
 import com.otaliastudios.transcoder.common.TrackStatus
 import com.otaliastudios.transcoder.common.TrackType
-import com.otaliastudios.transcoder.internal.media.MediaFormatProvider
+import com.otaliastudios.transcoder.internal.media.MediaFormatConstants
 import com.otaliastudios.transcoder.internal.utils.Logger
 import com.otaliastudios.transcoder.internal.utils.TrackMap
-import com.otaliastudios.transcoder.internal.utils.trackMapOf
-import com.otaliastudios.transcoder.source.DataSource
-import com.otaliastudios.transcoder.strategy.TrackStrategy
+import java.nio.ByteBuffer
+import kotlin.properties.Delegates.observable
 
 /**
  * Encoders are shared between segments. This is not strictly needed but it is more efficient
@@ -25,24 +27,98 @@ internal class Codecs(
         private val current: TrackMap<Int>
 ) {
 
+    class Surface(
+        private val context: EglCore,
+        val window: EglWindowSurface,
+    ) {
+        fun release() {
+            window.release()
+            context.release()
+        }
+    }
+
+    class Codec(val codec: MediaCodec, val surface: Surface? = null, var log: Logger? = null) {
+        var dequeuedInputs by observable(0) { _, _, _ -> log?.v(state) }
+        var dequeuedOutputs by observable(0) { _, _, _ -> log?.v(state) }
+        val state get(): String = "dequeuedInputs=$dequeuedInputs dequeuedOutputs=$dequeuedOutputs heldInputs=${heldInputs.size}"
+
+        private val heldInputs = ArrayDeque<Pair<ByteBuffer, Int>>()
+
+        fun getInputBuffer(): Pair<ByteBuffer, Int>? {
+            if (heldInputs.isNotEmpty()) {
+                return heldInputs.removeFirst().also { log?.v(state) }
+            }
+            val id = codec.dequeueInputBuffer(100)
+            return if (id >= 0) {
+                dequeuedInputs++
+                val buf = checkNotNull(codec.getInputBuffer(id)) { "inputBuffer($id) should not be null." }
+                buf to id
+            } else {
+                log?.i("buffer() failed with $id. $state")
+                null
+            }
+        }
+
+        /**
+         * When we're not ready to write into this buffer, it can be held for later.
+         * Previously we were returning it to the codec with timestamp=0, flags=0, but especially
+         * on older Android versions that can create subtle issues.
+         * It's better to just keep the buffer here and reuse it on the next [getInputBuffer] call.
+         */
+        fun holdInputBuffer(buffer: ByteBuffer, id: Int) {
+            heldInputs.addLast(buffer to id)
+        }
+    }
+
     private val log = Logger("Codecs")
 
-    val encoders = object : TrackMap<Pair<MediaCodec, Surface?>> {
+    val encoders = object : TrackMap<Codec> {
 
         override fun has(type: TrackType) = tracks.all[type] == TrackStatus.COMPRESSING
 
         private val lazyAudio by lazy {
             val format = tracks.outputFormats.audio
             val codec = MediaCodec.createEncoderByType(format.getString(MediaFormat.KEY_MIME)!!)
             codec.configure(format, null, null, MediaCodec.CONFIGURE_FLAG_ENCODE)
-            codec to null
+            Codec(codec, null)
         }
 
         private val lazyVideo by lazy {
             val format = tracks.outputFormats.video
+            val width = format.getInteger(MediaFormat.KEY_WIDTH)
+            val height = format.getInteger(MediaFormat.KEY_HEIGHT)
+            log.i("Destination video surface size: ${width}x${height} @ ${format.getInteger(MediaFormatConstants.KEY_ROTATION_DEGREES)}")
+            log.i("Destination video format: $format")
+
+            val allCodecs = MediaCodecList(MediaCodecList.REGULAR_CODECS)
+            val videoEncoders = allCodecs.codecInfos.filter { it.isEncoder && it.supportedTypes.any { it.startsWith("video/") } }
+            log.i("Available encoders: ${videoEncoders.joinToString { "${it.name} (${it.supportedTypes.joinToString()})" }}")
+
+            // Could consider MediaCodecList(MediaCodecList.REGULAR_CODECS).findEncoderForFormat(format)
+            // But it's trickier, for example, format should not include frame rate on API 21 and maybe other quirks.
             val codec = MediaCodec.createEncoderByType(format.getString(MediaFormat.KEY_MIME)!!)
             codec.configure(format, null, null, MediaCodec.CONFIGURE_FLAG_ENCODE)
-            codec to codec.createInputSurface()
+            log.i("Selected encoder ${codec.name}")
+            val surface = codec.createInputSurface()
+
+            log.i("Creating OpenGL context on ${Thread.currentThread()} (${surface.isValid})")
+            val eglContext = EglCore(EGL14.EGL_NO_CONTEXT, EglCore.FLAG_RECORDABLE)
+            val eglWindow = EglWindowSurface(eglContext, surface, true)
+            eglWindow.makeCurrent()
+
+            // On API28 (possibly others) emulator, this happens. If we don't throw early, it fails later with unclear
+            // errors - a tombstone dump saying that src.width() & 1 == 0 (basically, complains that surface size is odd)
+            // and an error much later on during encoder's dequeue. Surface size is odd because it's 1x1.
+            val (eglWidth, eglHeight) = eglWindow.getWidth() to eglWindow.getHeight()
+            if (eglWidth != width || eglHeight != height) {
+                log.e("OpenGL surface has wrong size (expected: ${width}x${height}, found: ${eglWindow.getWidth()}x${eglWindow.getHeight()}).")
+                // Throw a clear error in this very specific scenario so we can catch it in tests.
+                if (codec.name == "c2.android.avc.encoder" && eglWidth == 1 && eglHeight == 1) {
+                    error("c2.android.avc.encoder was unable to create the input surface (1x1).")
+                }
+            }
+
+            Codec(codec, Surface(eglContext, eglWindow))
         }
 
         override fun get(type: TrackType) = when (type) {
@@ -63,7 +139,7 @@ internal class Codecs(
 
     fun release() {
         encoders.forEach {
-            it.first.release()
+            it.surface?.release()
         }
     }
 }

lib/src/main/java/com/otaliastudios/transcoder/internal/Segment.kt

@@ -3,15 +3,14 @@ package com.otaliastudios.transcoder.internal
 import com.otaliastudios.transcoder.common.TrackType
 import com.otaliastudios.transcoder.internal.pipeline.Pipeline
 import com.otaliastudios.transcoder.internal.pipeline.State
-import com.otaliastudios.transcoder.internal.utils.Logger
 
 internal class Segment(
         val type: TrackType,
         val index: Int,
         private val pipeline: Pipeline,
 ) {
 
-    private val log = Logger("Segment($type,$index)")
+    // private val log = Logger("Segment($type,$index)")
     private var state: State<Unit>? = null
 
     fun advance(): Boolean {
@@ -20,15 +19,14 @@ internal class Segment(
     }
 
     fun canAdvance(): Boolean {
-        log.v("canAdvance(): state=$state")
+        // log.v("canAdvance(): state=$state")
         return state == null || state !is State.Eos
     }
 
     fun needsSleep(): Boolean {
         when(val s = state ?: return false) {
             is State.Ok -> return false
-            is State.Retry -> return false
-            is State.Wait -> return s.sleep
+            is State.Failure -> return s.sleep
         }
     }
 

lib/src/main/java/com/otaliastudios/transcoder/internal/Segments.kt

@@ -9,9 +9,9 @@ import com.otaliastudios.transcoder.internal.utils.TrackMap
 import com.otaliastudios.transcoder.internal.utils.mutableTrackMapOf
 
 internal class Segments(
-        private val sources: DataSources,
-        private val tracks: Tracks,
-        private val factory: (TrackType, Int, TrackStatus, MediaFormat) -> Pipeline
+    private val sources: DataSources,
+    private val tracks: Tracks,
+    private val factory: (TrackType, Int, Int, TrackStatus, MediaFormat) -> Pipeline
 ) {
 
     private val log = Logger("Segments")
@@ -21,7 +21,7 @@ internal class Segments(
 
     fun hasNext(type: TrackType): Boolean {
         if (!sources.has(type)) return false
-        log.v("hasNext($type): segment=${current.getOrNull(type)} lastIndex=${sources.getOrNull(type)?.lastIndex} canAdvance=${current.getOrNull(type)?.canAdvance()}")
+        // log.v("hasNext($type): segment=${current.getOrNull(type)} lastIndex=${sources.getOrNull(type)?.lastIndex} canAdvance=${current.getOrNull(type)?.canAdvance()}")
         val segment = current.getOrNull(type) ?: return true // not started
         val lastIndex = sources.getOrNull(type)?.lastIndex ?: return false // no track!
         return segment.canAdvance() || segment.index < lastIndex
@@ -85,15 +85,16 @@ internal class Segments(
         // who check it during pipeline init.
         currentIndex[type] = index
         val pipeline = factory(
-                type,
-                index,
-                tracks.all[type],
-                tracks.outputFormats[type]
+            type,
+            index,
+            sources[type].size,
+            tracks.all[type],
+            tracks.outputFormats[type]
         )
         return Segment(
-                type = type,
-                index = index,
-                pipeline = pipeline
+            type = type,
+            index = index,
+            pipeline = pipeline
         ).also {
             current[type] = it
         }