-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BUG] AnkiDroid API no longer lists media on cards #17062
Comments
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 |
Hey there - that seems feasible. We already have a ContentResolver - it allows you to fetch a card IIRC, it could also allow you to fetch an array (or whatever ContentResolver calls it - IIRC it is kind of a database-ish design so maybe a "result set" or whatever) of all media related to that card I imagine you'd want file name, media object size then the data stream or similar Anything that did something like that seems reasonable to me |
Thanks @mikehardy for the reply, but unfortunately, I'm not sure if I understood it correctly (I'm not an Android developer, I've only learned a bit about it). Are you saying that I can already retrieve image data using the AnkiDroid API, or did you mean that a new method needs to be added to the API, which would return file data for a specific card? |
@piotrbrzezina I should have looked more closely before, and I'm sorry I was ambiguous. It appears that media files are on the ReviewInfo object, which sounds like it might be just what you need:
|
Thanks for the information. I was trying to do what you proposed, but in my case, it is not working. In that JSON, I only get an empty array '[]'. Could you check if I'm doing it correctly? This is the code I used to get data from the JSON array. val deckSelector = "limit=?,deckID=?"
val deckArguments = arrayOfNulls<String>(2)
deckArguments[0] = "" + 1
deckArguments[1] = "" + deckId
val card = HashMap<String, String>()
val cardCursor = context.contentResolver.query(
FlashCardsContract.ReviewInfo.CONTENT_URI,
null,
deckSelector,
deckArguments,
null
)
if (cardCursor?.moveToFirst() == true) {
val noteIdIndex = cardCursor.getColumnIndex(FlashCardsContract.ReviewInfo.NOTE_ID)
val cardIdIndex = cardCursor.getColumnIndex(FlashCardsContract.ReviewInfo.CARD_ORD)
val buttonCountIndex = cardCursor.getColumnIndex(FlashCardsContract.ReviewInfo.BUTTON_COUNT)
val nextReviewIndex = cardCursor.getColumnIndex(FlashCardsContract.ReviewInfo.NEXT_REVIEW_TIMES)
val mediaFilesIndex = cardCursor.getColumnIndex(FlashCardsContract.ReviewInfo.MEDIA_FILES)
if (noteIdIndex > -1 && cardIdIndex > -1) {
val noteId = cardCursor.getString(noteIdIndex)
val specificCardId = cardCursor.getString(cardIdIndex)
val buttonCount = cardCursor.getString(buttonCountIndex)
val reviewIndex = cardCursor.getString(nextReviewIndex)
val mediaFiles = cardCursor.getString(mediaFilesIndex) Maybe the issue is that the deck I'm trying to work with is not following some guidelines? |
Sorry - I honestly don't know, this may be a case of needing to get AnkiDroid building locally and adding debug code into the part of AnkiDroid that populates (or should populate) the media files, and perhaps doing the same with the api that gets built into your app. I apologize but I won't have time to pursue this myself, so getting a locally test/debug environment for the other side of the API seems like fastest path forward |
Relevant: Anki-Android/AnkiDroid/src/main/java/com/ichi2/anki/provider/CardContentProvider.kt Line 1077 in 2910ce5
Also not going to have time to dig deep here, apologies! Anki-Android/AnkiDroid/src/main/java/com/ichi2/libanki/Media.kt Lines 60 to 72 in c63e93a
|
My presumption is that I suspect it previously returned images and audio/video, and now only returns audio/video. A unit test will confirm |
Good first issue: Produce a unit test which confirms this hypothesis It should fail on Assuming the test does portray a problem: Either:
There are many sample tests to follow: Anki-Android/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/ContentProviderTest.kt Lines 975 to 1028 in 06e8ac2
|
Oh my, this:
...is correct. It was definitely broken then. And as near as I can tell there is a lot of code and infrastructure around parsing out AV tags (audio, or video) but nothing about images in the upstream anki (where this is all implemented). A unit test will certainly confirm this I think the most correct path is likely to lobby for + propose a PR into upstream anki that also implements something like I think the most expedient path is likely to be re-implementing something like the logic of Media.filesInStr before dropping legacy backend, but focused solely on card contents text parse for images, and adding those to the list already returned by the AV tag backend method: https://github.com/ankidroid/Anki-Android/blame/4d39b5b9dd932d86826dd9dacc5ec2d94603046a/AnkiDroid/src/main/java/com/ichi2/libanki/Media.kt#L218 |
I'd prefer to reimplement our old code, and leave a lower priority TODO to make it faster by implementing it in the backend Just for speed of the bugfix |
I agree with that, especially since the old code objectively worked - not saying it's an awesome path or anything just saying that pulling the old code that found images from prior commits on git blame has a high chance of "just working" since the code actually did work... |
Started on a Unit Test. Audio is also not working, as we pass in |
Please assign me |
Brief notes:
Each audio and video file should be listed once. Note that this is different from past behaviour, where media files were listed multiple times. Then fix the bug. Use the code from before the linked commit as a base, but change the call to This is because |
@david-allison no update for more than 2 months. Can I work on this? |
Sure |
Hi @david-allison, just to confirm I have to write Unit tests for the filesInStr function right? To check weather it can return images or not? |
#17062 (comment) should be sufficient. I don't believe filesInStr is what you want to work with. Use the public API surface to be sure the value is correct |
Hi @david-allison, @Test
fun testQueryAddsMediaFilesCorrectly() {
val frontContent = """<img src="img.jpg"> [sound:test.mp3]"""
val backContent = """[sound:back.mp3]"""
val note = addNoteUsingBasicModel(frontContent,backContent)
val ord = 0
val noteUri = Uri.withAppendedPath(FlashCardsContract.Note.CONTENT_URI,note.id.toString())
val cardsUri = Uri.withAppendedPath(noteUri, "cards")
val specificCardUri = Uri.withAppendedPath(cardsUri, ord.toString())
val projection = arrayOf(
FlashCardsContract.ReviewInfo.MEDIA_FILES
)
contentResolver.query(
specificCardUri,
projection,
null,
null,
null
)?. let { cursor ->
if (!cursor.moveToFirst()) {
fail("failed")
}
assertNotNull("Cursor should not be null", cursor)
}
} When I try to log the cursor content I am getting this: |
@ujjol1234 |
@david-allison you are absolutely right. It was my mistake. I have corrected the code and my unit test seems to be working fine now. Here's the code for the test: @Test
fun testMediaFilesAddedCorrectlyInReviewInfo() {
val frontContent = """<img src="img.jpg"> [sound:test.mp3]"""
val imageFileName = "img.jpg"
val audioFileName = "test.mp3"
val note = addNoteUsingBasicModel("Hello $frontContent","backContent")
val ord = 0
val card = note.cards(col)[ord]
card.queue = QueueType.New
card.due = col.sched.today
col.updateCard(card)
contentResolver.query(
FlashCardsContract.ReviewInfo.CONTENT_URI,
null,
null,
null,
null
)?. let { cursor ->
if (!cursor.moveToFirst()) {
fail("failed")
}
assertNotNull("Cursor should not be null", cursor)
val mediaFilesArray = cursor.getString(cursor.getColumnIndex(FlashCardsContract.ReviewInfo.MEDIA_FILES))
var imageFilePresent = false
var audioFilePresent = false
var allMediaFilesPresent =false
for (ele in mediaFilesArray.split(",")){
if(imageFileName == ele){
imageFilePresent =true
}
if(audioFileName == ele){
audioFilePresent = true
}
}
if(imageFilePresent and audioFilePresent){allMediaFilesPresent = true}
assertTrue("All media files should be present in the media_files array",allMediaFilesPresent)
}
} Now I am trying to fix the bug... |
Quick refactor of the above: Index: AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/ContentProviderTest.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/ContentProviderTest.kt b/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/ContentProviderTest.kt
--- a/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/ContentProviderTest.kt (revision 1a8fb3e3b1b0347cfc297e24ae040a9d901f8cd8)
+++ b/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/ContentProviderTest.kt (date 1736990564130)
@@ -52,6 +52,7 @@
import com.ichi2.utils.emptyStringArray
import net.ankiweb.rsdroid.exceptions.BackendNotFoundException
import org.hamcrest.MatcherAssert.assertThat
+import org.hamcrest.Matchers.contains
import org.hamcrest.Matchers.containsString
import org.hamcrest.Matchers.equalTo
import org.hamcrest.Matchers.greaterThan
@@ -329,6 +330,24 @@
col.notetypes.rem(noteType)
}
+ @Test
+ fun testMediaFilesAddedCorrectlyInReviewInfo() {
+ val imageFileName = "img.jpg"
+ val audioFileName = "test.mp3"
+ val note = addNoteUsingBasicModel(
+ """Hello <img src="$imageFileName"> [sound:$audioFileName]""",
+ "backContent"
+ )
+ note.firstCard(col).update {
+ queue = QueueType.New
+ due = col.sched.today
+ }
+
+ val media = contentResolver.queryAnkiMediaFiles()
+ assertThat("image found", media, contains(imageFileName))
+ assertThat("audio found", media, contains(audioFileName))
+ }
+
/**
* Check that inserting and removing a note into default deck works as expected
*/
@@ -1436,3 +1455,20 @@
unburyDeck(did)
}
}
+
+
+fun ContentResolver.queryAnkiMediaFiles(): List<String> {
+ this.query(
+ FlashCardsContract.ReviewInfo.CONTENT_URI,
+ null,
+ null,
+ null,
+ null
+ ).use { cursor ->
+ assertNotNull(cursor)
+ assertTrue("cursor has elements", cursor.moveToFirst())
+ return cursor
+ .getString(cursor.getColumnIndex(FlashCardsContract.ReviewInfo.MEDIA_FILES))
+ .split(",")
+ }
+}
\ No newline at end of file
Index: AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/InstrumentedTest.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/InstrumentedTest.kt b/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/InstrumentedTest.kt
--- a/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/InstrumentedTest.kt (revision 1a8fb3e3b1b0347cfc297e24ae040a9d901f8cd8)
+++ b/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/InstrumentedTest.kt (date 1736901346504)
@@ -157,6 +157,11 @@
col.updateCard(this, true)
}
+ fun Card.update(block: Card.() -> Unit) {
+ block(this)
+ col.updateCard(this)
+ }
+
@DuplicatedCode("This is copied from RobolectricTest. This will be refactored into a shared library later")
internal fun addNoteUsingBasicModel(
front: String = "Front", |
Hi @david-allison, fun filesInStr(mid: Long?, string: String, includeRemote: Boolean = false): List<String> {
val l: MutableList<String> = ArrayList()
val model = col.models.get(mid!!)
var strings: MutableList<String?> = ArrayList()
if (model!!.isCloze && string.contains("{{c")) {
// if the field has clozes in it, we'll need to expand the possibilities so we can render LaTeX
strings = _expandClozes(string)
} else {
strings.add(string)
}
for (s in strings) {
@Suppress("NAME_SHADOWING")
var s = s
// handle LaTeX
@KotlinCleanup("change to .map { }")
val svg = model.optBoolean("latexsvg", false)
s = LaTeX.mungeQA(s!!, col, svg)
// extract filenames
var m: Matcher
for (p in REGEXPS) {
// NOTE: python uses the named group 'fname'. Java doesn't have named groups, so we have to determine
// the index based on which pattern we are using
val fnameIdx = if (p == fSoundRegexps) 2 else if (p == fImgAudioRegExpU) 2 else 3
m = p.matcher(s)
while (m.find()) {
val fname = m.group(fnameIdx)!!
val isLocal = !fRemotePattern.matcher(fname.lowercase(Locale.getDefault())).find()
if (isLocal || includeRemote) {
l.add(fname)
}
}
}
}
return l
}
or should I try something else? |
Hi @ujjol1234 With apologies, I'm busy this week, could you try our Discord, or ping me next Monday |
Sure. Thank you for letting me know. I completely understand! I'll reach out on Discord in the meantime, and if needed, I'll ping you again next Monday. |
Prior direction was:
Prior filesInStr was what you proposed, however the signature needs to change:
should become more like
and then I assume you want to call Anki-Android/AnkiDroid/src/main/java/com/ichi2/libanki/Card.kt Lines 187 to 196 in a9b9529
It looks like it returns a type that has arrays with the AV tags that filesInStr previous logic should be able to work through? Anki-Android/AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt Lines 234 to 239 in a9b9529
|
Thank you for the help @mikehardy. I’ll try this approach and let you know how it goes. |
@mikehardy Thanks for the guidance earlier! I followed the steps you provided and I have fixed the filesInStr function. |
Hi, I'm trying to write an app that will launch upon unlocking the phone, and its only task will be to present one card, then close after receiving a response. Unfortunately, I'm struggling with one problem, namely how to display images and play audio files because I don't have access to them due to the new Android permissions. I'm wondering if there's a way around this?
I was thinking about whether it would be possible to add another method to the AnkiDroid API that, return an image/audio binary data if it had access to such a file. Do you think this is doable in Android?
The text was updated successfully, but these errors were encountered: