fix: videos with only audio stream

A number of video extension may only contain audio In these cases, we should show a play button Sources: * anki-15872-valid-1.mp4 -> https://github.com/chromium/chromium/blob/main/third_party/blink/web_tests/external/wpt/media/movie_5.mp4 * anki-15872-audio-only.webm -> https://github.com/chromium/chromium/blob/main/third_party/blink/web_tests/external/wpt/media/test-a-128k-44100Hz-1ch.webm * anki-15872-audio-only.mp4 -> https://github.com/chromium/chromium/blob/main/third_party/blink/web_tests/external/wpt/media-source/mp4/test-a-128k-44100Hz-1ch.mp4 I attempted to use MediaMetadataRetriever: ``` MediaMetadataRetriever().let { it.setDataSource(file.path) it.extractMetadata(MediaMetadataRetriever.METADATA_KEY_HAS_VIDEO) } ``` but it fails on API 23 with: ``` java.lang.RuntimeException: setDataSource failed: status = 0x80000000 at android.media.MediaMetadataRetriever.setDataSource(Native Method) ``` Fixes 15872 Co-authored-by: Brayan Oliveira <[email protected]>
ankidroid · Mar 14, 2024 · c8ec679 · c8ec679
1 parent 2ac9fd9
commit c8ec679
Show file tree

Hide file tree

Showing 9 changed files with 110 additions and 19 deletions.
diff --git a/AnkiDroid/src/androidTest/assets/anki-15872-audio-only.mp4 b/AnkiDroid/src/androidTest/assets/anki-15872-audio-only.mp4
diff --git a/AnkiDroid/src/androidTest/assets/anki-15872-audio-only.webm b/AnkiDroid/src/androidTest/assets/anki-15872-audio-only.webm
diff --git a/AnkiDroid/src/androidTest/assets/anki-15872-valid-1.mp4 b/AnkiDroid/src/androidTest/assets/anki-15872-valid-1.mp4
diff --git a/AnkiDroid/src/androidTest/java/com/ichi2/anki/libanki/SoundTest.kt b/AnkiDroid/src/androidTest/java/com/ichi2/anki/libanki/SoundTest.kt
@@ -0,0 +1,48 @@
+/*
+ *  Copyright (c) 2024 David Allison <[email protected]>
+ *
+ *  This program is free software; you can redistribute it and/or modify it under
+ *  the terms of the GNU General Public License as published by the Free Software
+ *  Foundation; either version 3 of the License, or (at your option) any later
+ *  version.
+ *
+ *  This program is distributed in the hope that it will be useful, but WITHOUT ANY
+ *  WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+ *  PARTICULAR PURPOSE. See the GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along with
+ *  this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package com.ichi2.anki.libanki
+
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.ichi2.anki.tests.InstrumentedTest
+import com.ichi2.anki.tests.Shared
+import com.ichi2.libanki.isAudioFileInVideoContainer
+import org.hamcrest.CoreMatchers.equalTo
+import org.hamcrest.MatcherAssert.assertThat
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@RunWith(AndroidJUnit4::class)
+class SoundTest : InstrumentedTest() {
+
+    @Test
+    fun mp4IsDetected() {
+        val mp4 = Shared.getTestFile(testContext, "anki-15872-valid-1.mp4")
+        assertThat("mp4 with video should be marked as video", isAudioFileInVideoContainer(mp4), equalTo(false))
+    }
+
+    @Test
+    fun audioOnlyMp4IsDetected() {
+        val mp4 = Shared.getTestFile(testContext, "anki-15872-audio-only.mp4")
+        assertThat("mp4 should be audio only", isAudioFileInVideoContainer(mp4), equalTo(true))
+    }
+
+    @Test
+    fun audioOnlyWebmIsDetected() {
+        val mp4 = Shared.getTestFile(testContext, "anki-15872-audio-only.webm")
+        assertThat("webm should be audio only", isAudioFileInVideoContainer(mp4), equalTo(true))
+    }
+}
diff --git a/AnkiDroid/src/main/java/com/ichi2/compat/Compat.kt b/AnkiDroid/src/main/java/com/ichi2/compat/Compat.kt
@@ -161,7 +161,7 @@ interface Compat {
      */
     @Throws(IOException::class)
     fun createDirectories(directory: File)
-    fun hasVideoThumbnail(path: String): Boolean
+    fun hasVideoThumbnail(path: String): Boolean?
 
     /**
      * Writes an image represented by bitmap to the Pictures/AnkiDroid directory under the primary

diff --git a/AnkiDroid/src/main/java/com/ichi2/compat/CompatV23.kt b/AnkiDroid/src/main/java/com/ichi2/compat/CompatV23.kt
@@ -185,8 +185,12 @@ open class CompatV23 : Compat {
     }
 
     // Until API 29
-    override fun hasVideoThumbnail(path: String): Boolean {
-        return ThumbnailUtils.createVideoThumbnail(path, MediaStore.Images.Thumbnails.MINI_KIND) != null
+    override fun hasVideoThumbnail(path: String): Boolean? {
+        return try {
+            ThumbnailUtils.createVideoThumbnail(path, MediaStore.Images.Thumbnails.MINI_KIND) != null
+        } catch (e: Exception) {
+            null
+        }
     }
 
     // Until API31 the MediaRecorder constructor was default, ignoring the Context

diff --git a/AnkiDroid/src/main/java/com/ichi2/compat/CompatV29.kt b/AnkiDroid/src/main/java/com/ichi2/compat/CompatV29.kt
@@ -26,20 +26,25 @@ import android.provider.MediaStore
 import android.util.Size
 import com.ichi2.libanki.utils.TimeManager
 import java.io.File
+import java.io.IOException
 
 /** Implementation of [Compat] for SDK level 29  */
 @TargetApi(29)
 open class CompatV29 : CompatV26(), Compat {
-    override fun hasVideoThumbnail(path: String): Boolean {
+    override fun hasVideoThumbnail(path: String): Boolean? {
         return try {
             ThumbnailUtils.createVideoThumbnail(File(path), THUMBNAIL_MINI_KIND, null)
             // createVideoThumbnail throws an exception if it's null
             true
-        } catch (e: Exception) {
+        } catch (e: IOException) {
             // The default for audio is an IOException, so don't log it
             // A log line is still produced:
             // E/MediaMetadataRetrieverJNI: getEmbeddedPicture: Call to getEmbeddedPicture failed
-            false
+            if (e.message == "Failed to create thumbnail") return false
+            null
+        } catch (e: Exception) {
+            // unexpected exception
+            null
         }
     }
 

diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/Sound.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/Sound.kt
@@ -64,7 +64,10 @@ fun stripAvRefs(text: String, replacement: String = "") = Sound.AV_REF_RE.replac
 
 // not in libAnki
 object Sound {
-    val VIDEO_EXTENSIONS = setOf("mp4", "mov", "mpg", "mpeg", "mkv", "webm")
+    val VIDEO_ONLY_EXTENSIONS = setOf("mov", "mkv")
+
+    /** Extensions that can be audio-only using a video wrapper */
+    val AUDIO_OR_VIDEO_EXTENSIONS = setOf("mp4", "mpg", "mpeg", "webm")
 
     /**
      * expandSounds takes content with embedded sound file placeholders and expands them to reference the actual media

diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt
@@ -23,7 +23,9 @@
 package com.ichi2.libanki
 
 import com.ichi2.annotations.NeedsTest
-import com.ichi2.libanki.Sound.VIDEO_EXTENSIONS
+import com.ichi2.compat.CompatHelper
+import com.ichi2.libanki.Sound.AUDIO_OR_VIDEO_EXTENSIONS
+import com.ichi2.libanki.Sound.VIDEO_ONLY_EXTENSIONS
 import com.ichi2.libanki.TemplateManager.PartiallyRenderedCard.Companion.avTagsToNative
 import com.ichi2.libanki.backend.BackendUtils
 import com.ichi2.libanki.backend.model.toBackendNote
@@ -32,7 +34,6 @@ import com.ichi2.libanki.utils.append
 import com.ichi2.libanki.utils.len
 import com.ichi2.utils.deepClone
 import net.ankiweb.rsdroid.exceptions.BackendTemplateException
-import org.intellij.lang.annotations.Language
 import org.jetbrains.annotations.VisibleForTesting
 import org.json.JSONObject
 import org.jsoup.Jsoup
@@ -311,19 +312,28 @@ class TemplateManager {
 @NotInLibAnki
 @VisibleForTesting
 fun parseVideos(text: String, mediaDir: String): String {
+    fun toVideoTag(path: String): String {
+        val uri = getFileUri(path)
+        return """<video src="$uri" controls controlsList="nodownload"></video>"""
+    }
+
     return SOUND_RE.replace(text) { match ->
         val fileName = match.groupValues[1]
         val extension = fileName.substringAfterLast(".", "")
-        if (extension in VIDEO_EXTENSIONS) {
-            val path = Paths.get(mediaDir, fileName).toString()
-            val uri = getFileUri(path)
-
-            @Language("HTML")
-            val result =
-                """<video src="$uri" controls controlsList="nodownload"></video>"""
-            result
-        } else {
-            match.value
+        when (extension) {
+            in VIDEO_ONLY_EXTENSIONS -> {
+                val path = Paths.get(mediaDir, fileName).toString()
+                toVideoTag(path)
+            }
+            in AUDIO_OR_VIDEO_EXTENSIONS -> {
+                val file = File(mediaDir, fileName)
+                if (isAudioFileInVideoContainer(file) == true) {
+                    match.value
+                } else {
+                    toVideoTag(file.path)
+                }
+            }
+            else -> match.value
         }
     }
 }
@@ -386,3 +396,24 @@ fun getFileUri(path: String): URI {
     if (!p.startsWith("//")) p = "//$p"
     return URI("file", p, null)
 }
+
+/**
+ * Whether a video file only contains an audio stream
+ *
+ * @return `null` - file is not a video, or not found
+ */
+@VisibleForTesting
+fun isAudioFileInVideoContainer(file: File): Boolean? {
+    if (file.extension !in VIDEO_ONLY_EXTENSIONS && file.extension !in AUDIO_OR_VIDEO_EXTENSIONS) {
+        return null
+    }
+
+    if (file.extension in VIDEO_ONLY_EXTENSIONS) return false
+
+    // file.extension is in AUDIO_OR_VIDEO_EXTENSIONS
+    if (!file.exists()) return null
+
+    // Also check that there is a video thumbnail, as some formats like mp4 can be audio only
+    val isVideo = CompatHelper.compat.hasVideoThumbnail(file.absolutePath) ?: return null
+    return !isVideo
+}