Closed Alaanor closed 5 months ago
welcome[bot]
commented
5 months ago Hello! 👋 Thanks for logging this issue. Please remember we are all volunteers here, so some patience may be required before we can get to the issue. Also remember that the fastest way to get resolution on an issue is to propose a change directly, https://github.com/ankidroid/Anki-Android/wiki/Contributing
david-allison
commented
5 months ago Hi, do you feel that this is a duplicate of:
I've got a pending fix:
Almost certainly released in 2.18alpha after discussion
Alaanor
commented
5 months ago Just to clarify, the .webm here is a audio file. There are no video at all. In the anki field they're clearly as [sound:foobar.webm] .
Not saying I disagree with the duplicate. Just pointing out in case you though it was a video as the webm format can also support video indeed.
david-allison
commented
5 months ago [sound:] is how Anki Desktop has specified videos for many years 🤷♂️
Would you mind if I modified this issue to involve better detection of audio vs video, since autoplay is handled in the linked issue above
Alaanor
commented
5 months ago I see. Sure, feel free to change the title.
david-allison
commented
5 months ago @BrayanDSO FYI
Pseudocode after https://github.com/ankidroid/Anki-Android/pull/15840
val VIDEO_EXTENSIONS = setOf("mp4", "mov", "mpg", "mpeg", "mkv", "webm")
fun SoundOrVideoTag.isVideo(): Boolean {
val extension = this.filename.substringAfterLast(".", "")
return extension in VIDEO_EXTENSIONS
}
fun expandSounds() =
return@replaceAvRefsWith if (tag.isVideo()) {
"""<video />"""
} else if (showAudioPlayButtons) {
"""<a class="soundLink"/>"""
} else {
""
}Would you support changing this isVideo check to something along the lines of our old code
boolean isVideo = Arrays.asList(VIDEO_WHITELIST).contains(extension);
if (!isVideo) {
final String guessedType = MimeTypeMap.getSingleton().getMimeTypeFromExtension(extension);
isVideo = (guessedType != null) && guessedType.startsWith("video/");
}
// Also check that there is a video thumbnail, as some formats like mp4 can be audio only
isVideo = isVideo && hasVideoThumbnail(soundUri);
BrayanDSO
commented
5 months ago Would you support changing this isVideo check to something along the lines of our old code
Definitely.
I tried using MediaMetadataRetriever and it worked fine in API 34. Still needs some tests in other APIs, with another files, and checking what other extensions are Audio or video, but my current timeblock is over.
Here's some audio-only samples (couldn't find some easily with a quick google)
Subject: [PATCH] foo
---
Index: AnkiDroid/src/main/java/com/ichi2/libanki/Sound.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/Sound.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/Sound.kt
--- a/AnkiDroid/src/main/java/com/ichi2/libanki/Sound.kt (revision da4b4a5ecd01b9d732f93d125e027b3b06b3e900)
+++ b/AnkiDroid/src/main/java/com/ichi2/libanki/Sound.kt (date 1710283417112)
@@ -64,7 +64,10 @@
// not in libAnki
object Sound {
- val VIDEO_EXTENSIONS = setOf("mp4", "mov", "mpg", "mpeg", "mkv", "webm")
+ val VIDEO_EXTENSIONS = setOf("mov", "mpg", "mpeg", "mkv")
+
+ /** Extensions that can be a video or just audio */
+ val AUDIO_OR_VIDEO_EXTENSIONS = setOf("mp4", "webm")
/**
* expandSounds takes content with embedded sound file placeholders and expands them to reference the actual media
Index: AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt
--- a/AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt (revision da4b4a5ecd01b9d732f93d125e027b3b06b3e900)
+++ b/AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt (date 1710283868777)
@@ -22,7 +22,9 @@
package com.ichi2.libanki
+import android.media.MediaMetadataRetriever
import com.ichi2.annotations.NeedsTest
+import com.ichi2.libanki.Sound.AUDIO_OR_VIDEO_EXTENSIONS
import com.ichi2.libanki.Sound.VIDEO_EXTENSIONS
import com.ichi2.libanki.TemplateManager.PartiallyRenderedCard.Companion.avTagsToNative
import com.ichi2.libanki.backend.BackendUtils
@@ -32,11 +34,11 @@
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
import org.jsoup.nodes.Document.OutputSettings
+import timber.log.Timber
import java.io.File
import java.net.URI
import java.net.URISyntaxException
@@ -311,19 +313,28 @@
@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_EXTENSIONS -> {
+ val path = Paths.get(mediaDir, fileName).toString()
+ toVideoTag(path)
+ }
+ in AUDIO_OR_VIDEO_EXTENSIONS -> {
+ val file = File(mediaDir, fileName)
+ if (file.exists() && isVideo(file)) {
+ toVideoTag(file.path)
+ } else {
+ match.value
+ }
+ }
+ else -> match.value
}
}
}
@@ -386,3 +397,18 @@
if (!p.startsWith("//")) p = "//$p"
return URI("file", p, null)
}
+
+@VisibleForTesting
+fun isVideo(file: File): Boolean {
+ val retriever = MediaMetadataRetriever()
+ return try {
+ retriever.setDataSource(file.path)
+ val hasVideo = retriever.extractMetadata(MediaMetadataRetriever.METADATA_KEY_HAS_VIDEO)
+ hasVideo == "yes"
+ } catch (e: Exception) {
+ Timber.w(e)
+ false
+ } finally {
+ retriever.release()
+ }
+}
\ No newline at end of file
Niemaa
commented
5 months ago Hi, just to add to this - apologies if it is not relevant here - I had the same problem not with webm or mp4 files but with a mpeg file that was an audio file but shown as a video. I saw that it was not included in the variable "val AUDIO_OR_VIDEO_EXTENSIONS = setOf("mp4", "webm")" but I am not a coder or tech savvy, so I might have overlooked something
david-allison
commented
5 months ago Thanks! Totally correct
david-allison
commented
5 months ago API 23 with the above audio-only MP4 throws:
java.lang.RuntimeException: setDataSource failed: status = 0x80000000
at android.media.MediaMetadataRetriever.setDataSource(Native Method)
at android.media.MediaMetadataRetriever.setDataSource(MediaMetadataRetriever.java:71)
at com.ichi2.libanki.TemplateManagerKt.isVideo(TemplateManager.kt:407)
at com.ichi2.anki.libanki.SoundTest.audioOnlyMp4IsDetected(SoundTest.kt:41)This is returned as false, but I suspect we want true, or 'null' as a default if we don't know (presume it has a video)
Checked for duplicates?
What are the steps to reproduce this bug?
Expected behaviour
Expected the audio player to
Actual behaviour
Here is how the audio player is shown.
Debug info
(Optional) Anything else you want to share?
Research