From d37c391c60fd54e3c740261dd77b76a4625f8a91 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Tue, 25 Aug 2026 09:45:00 -0500 Subject: [PATCH] Stop telling people to stage the benchmark the one way it cannot be staged RealMediaBenchmark's class KDoc said: Populate with: adb push .mp4 /sdcard/Android/data/org.libremediaconverter/files/ Twelve lines below, the `samples` property KDoc -- on `get() = context.filesDir` -- says: Internal storage, not the external files dir. Files placed in the external dir by `adb push` or `adb shell cp` stay owned by the shell user, and the app then gets EACCES trying to read them -- which presents as an unparseable input rather than a permission problem. Different directories, and the second exists specifically to explain why the first fails. Anyone following the class KDoc stages files the benchmark cannot read, gets a skip, and reads the skip as "not staged yet" -- the failure mode the property KDoc warns about, walked into by the instruction in the same file. The fix is not a corrected command. Restating the mechanism in a second place is what let these drift, and a replacement command I have not executed would be the same defect with a fresher date. The class KDoc now names [samples] as the single place that answers it. Two things added that are checkable rather than remembered: the exact filenames the tests look for, via [H264_SAMPLE] and [AV1_SAMPLE] -- the old text said `.mp4`, so even the right directory left you guessing -- and a note that the two skips every green E2E leg reports are these. Not claimed: that the benchmark misbehaves on CI. An earlier version of the ticket said so; it was wrong, and measuring settled it -- both tests report SKIPPED on the gating legs, the guards work, and "harmless in CI" is accurate. The failure that prompted the look is Media3EngineTest, tracked as #102. Closes #101. --- .../bench/RealMediaBenchmark.kt | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/app/src/androidTest/java/org/libremediaconverter/bench/RealMediaBenchmark.kt b/app/src/androidTest/java/org/libremediaconverter/bench/RealMediaBenchmark.kt index 3787388..d5f97ea 100644 --- a/app/src/androidTest/java/org/libremediaconverter/bench/RealMediaBenchmark.kt +++ b/app/src/androidTest/java/org/libremediaconverter/bench/RealMediaBenchmark.kt @@ -36,8 +36,20 @@ import java.io.File * 1. that the hardware path is worth having a second engine for at all, and * 2. that x264's CRF is worth the GPL licence the app carries for it. * - * Skips itself when the sample files are absent, so it is harmless in CI. Populate with: - * adb push .mp4 /sdcard/Android/data/org.libremediaconverter/files/ + * Skips itself when the sample files are absent, so it is harmless in CI — every green E2E + * leg reports two skips, and these are they. + * + * The two files it looks for, by exact name: + * + * - [H264_SAMPLE] for [hardwareVersusSoftwareOnRealVideo] + * - [AV1_SAMPLE] for [av1InputRoutesAccordingToDeviceDecodeSupport] + * + * **Where they go, and how, is on [samples] — read it before staging anything.** This used to + * carry an `adb push` line naming the external files dir, which [samples] then explains cannot + * work: a pushed file stays owned by the shell user and the app reads EACCES, surfacing as an + * unparseable input rather than a permission error. The instruction and its own refutation sat + * twelve lines apart. It is named in one place now rather than restated here, because restating + * it is what let the two drift. */ @UnstableApi @RunWith(AndroidJUnit4::class)