Give a video row a frame of the video, and fix what R8 was hiding
A video row showed a film-strip icon, and a phone holds more video than anything else, so a column of identical icons said nothing about which clip was which. Images and videos now take the same path - EntryIcon hands Coil the file, and for a video the request asks for a frame - which cost exactly one dependency: io.coil-kt.coil3:coil-video, at the same version as the Coil already in use, whose own POM adds nothing that Coil did not already bring (coil-core, kotlin-stdlib, core-ktx). No ExoPlayer, no media3, and no new permission: the decoder is the platform's own MediaMetadataRetriever. Two details are decisions rather than defaults. The frame is taken one second in, because the first frame of a recording is very often black or the middle of a fade-in, and a thumbnail of a black rectangle is no more useful than the icon it replaced. And a decode that fails - an unsupported codec, or a file only pretending to be a video - settles on the icon rather than leaving an empty square, which the row remembers so it is not retried on every recomposition. The dependency also nearly shipped a feature that did not work. coil-video registers its decoder through a META-INF/services entry, and in a debug APK that is enough: the file and the class it names are both there, and frames decoded. In the release APK, R8 removes both, because a class mentioned only from a resource looks unused to it. So the feature worked in every build that had been looked at and would have failed silently in the one users install - the worst shape a bug can have, since it needs no code change to "fix" and would have been reported as "video thumbnails don't work" against a build nobody had run. The factory is now registered explicitly against an ImageLoader in FileManagerApplication, which makes it a code reference R8 cannot drop, and the release APK was installed and driven to the Videos library to confirm the frames decode there too. That also closes the gap noted two changes ago, that no ImageLoader was registered at all; its cache sizes are left to Coil deliberately, since they are derived from the device's heap and a number written here would only be a guess. The same run found a smaller thing that no test could have: opening the search field from inside a library left the scope on "this folder" - the hint read "Search this folder" and a folder toggle sat beside it - while the search in fact ran everywhere, because the view model has always known that a library is not a folder and answered from the index instead. The behaviour was right and the screen was wrong, which is the same shape as the volume-list bug fixed earlier and the same root cause: the switch-on-open asked whether this was the root list rather than whether a folder was open. It now asks for a path, and the scope toggle appears only when there is a folder to choose about, because its two settings did the same thing otherwise. Every other use of isRootView was then checked for the same conflation and none was found. One test was added for the search behaviour; the scope mismatch itself is not testable, being a disagreement between two correct pieces. 324 JVM tests, lint clean. Verified on the phone in both builds: two clips made with ffmpeg (a testsrc2 pattern and SMPTE bars) drew their own patterns as thumbnails, beside a real 4.3 MB video from the device's own storage, and the demo library's three zero-filled stand-in .mp4s fell back to the icon - which exercised the failure path with material that happened to be lying around. The release APK is 2 294 021 bytes and still declares no INTERNET. Not done: the store screenshots still show the home screen from before the library views, and the release build running on a device is in nobody's checklist - which is exactly why the R8 problem survived until now, and is the first thing worth adding.
This commit is contained in:
@@ -1,6 +1,10 @@
|
||||
package de.softwareapp_hb.filemanager
|
||||
|
||||
import android.app.Application
|
||||
import android.content.Context
|
||||
import coil3.ImageLoader
|
||||
import coil3.SingletonImageLoader
|
||||
import coil3.video.VideoFrameDecoder
|
||||
import de.softwareapp_hb.filemanager.data.ApkInspector
|
||||
import de.softwareapp_hb.filemanager.data.ContentResolverIncomingSource
|
||||
import de.softwareapp_hb.filemanager.data.DeleteRequestFactory
|
||||
@@ -22,7 +26,7 @@ import de.softwareapp_hb.filemanager.service.ServiceOperationNotifier
|
||||
* are built once and handed to the one ViewModel that needs them. Nothing holds a reference
|
||||
* to an Activity, so none of this leaks.
|
||||
*/
|
||||
class FileManagerApplication : Application() {
|
||||
class FileManagerApplication : Application(), SingletonImageLoader.Factory {
|
||||
|
||||
/** Where files live and which backend is in effect. */
|
||||
lateinit var storage: StorageProvider
|
||||
@@ -62,4 +66,25 @@ class FileManagerApplication : Application() {
|
||||
apkInspector = PackageManagerApkInspector(this)
|
||||
searchIndex = SqliteSearchIndex(this)
|
||||
}
|
||||
|
||||
/**
|
||||
* The image loader every thumbnail and viewer goes through.
|
||||
*
|
||||
* Built here for one reason: **video frames**. Coil has no idea what to do with an `.mp4` until a
|
||||
* decoder is registered for it, and `coil-video` supplies one that understands the platform's own
|
||||
* `MediaMetadataRetriever`.
|
||||
*
|
||||
* The decoder is registered **explicitly** rather than left to `coil-video`'s `META-INF/services`
|
||||
* entry, because that entry does not survive a release build: R8 removes both the service file and
|
||||
* the class it names, since a class mentioned only from a resource looks unused. Checked against
|
||||
* the release APK, where neither was present — which would have left video thumbnails working in
|
||||
* debug and silently showing an icon in the build everyone installs.
|
||||
*
|
||||
* Only the decoder is configured. Coil's own cache sizes are left alone deliberately: they are
|
||||
* sized from the device's heap, which a number written here would only be guessing at.
|
||||
*/
|
||||
override fun newImageLoader(context: Context): ImageLoader =
|
||||
ImageLoader.Builder(context)
|
||||
.components { add(VideoFrameDecoder.Factory()) }
|
||||
.build()
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
package de.softwareapp_hb.filemanager.ui.browser
|
||||
|
||||
import android.content.Context
|
||||
import androidx.compose.foundation.ExperimentalFoundationApi
|
||||
import androidx.compose.foundation.combinedClickable
|
||||
import androidx.compose.foundation.layout.Arrangement
|
||||
@@ -30,16 +31,23 @@ import androidx.compose.material3.MaterialTheme
|
||||
import androidx.compose.material3.Surface
|
||||
import androidx.compose.material3.Text
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.runtime.setValue
|
||||
import androidx.compose.ui.Alignment
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.draw.clip
|
||||
import androidx.compose.ui.graphics.vector.ImageVector
|
||||
import androidx.compose.ui.layout.ContentScale
|
||||
import androidx.compose.ui.platform.LocalContext
|
||||
import androidx.compose.ui.res.stringResource
|
||||
import androidx.compose.ui.text.style.TextOverflow
|
||||
import androidx.compose.ui.unit.Dp
|
||||
import androidx.compose.ui.unit.dp
|
||||
import coil3.compose.AsyncImage
|
||||
import coil3.request.ImageRequest
|
||||
import coil3.video.videoFrameMillis
|
||||
import de.softwareapp_hb.filemanager.R
|
||||
import de.softwareapp_hb.filemanager.core.FileCategory
|
||||
import de.softwareapp_hb.filemanager.core.FileEntry
|
||||
@@ -62,19 +70,35 @@ private val GridThumbnailSize = 96.dp
|
||||
internal val GridCellMinSize = 144.dp
|
||||
|
||||
/**
|
||||
* The leading visual for an entry: a real thumbnail for an image we can actually read,
|
||||
* otherwise the category icon.
|
||||
* The leading visual for an entry: a real thumbnail for something we can actually read, otherwise
|
||||
* the category icon.
|
||||
*
|
||||
* Coil needs a filesystem path, so this only attempts a thumbnail in the all-files mode —
|
||||
* a SAF path is relative to the granted tree and would silently fail.
|
||||
* **A video is a picture too.** A phone holds more video than anything else, and a column of
|
||||
* identical film-strip icons says nothing about which clip is which — so a video's thumbnail is a
|
||||
* frame of the video itself, decoded by the platform's own retriever (through `coil-video`, which
|
||||
* registers itself with Coil rather than needing an `ImageLoader` to be assembled by hand).
|
||||
*
|
||||
* Two limits are inherited rather than invented. Coil needs a filesystem path, so this only attempts
|
||||
* a thumbnail in the all-files mode — a SAF path is relative to the granted tree and would fail
|
||||
* silently. And a decode that fails (an unsupported codec, a file that is really something else)
|
||||
* falls back to the icon rather than leaving an empty square.
|
||||
*/
|
||||
@Composable
|
||||
fun EntryIcon(entry: FileEntry, size: Dp) {
|
||||
if (entry.kind == FileKind.IMAGE && entry.path.value.startsWith("/")) {
|
||||
// A decode that has already failed is not retried on every recomposition: the row keeps showing
|
||||
// what it settled on, which is the icon.
|
||||
var failed by remember(entry.path.value) { mutableStateOf(false) }
|
||||
|
||||
if (entry.hasThumbnail() && !failed) {
|
||||
val context = LocalContext.current
|
||||
// Remembered, because Coil keys its caches by the request: a new request object on every
|
||||
// recomposition would be a new cache entry on every recomposition.
|
||||
val model = remember(entry.path.value, entry.kind) { entry.thumbnailRequest(context) }
|
||||
AsyncImage(
|
||||
model = File(entry.path.value),
|
||||
model = model,
|
||||
contentDescription = null,
|
||||
contentScale = ContentScale.Crop,
|
||||
onError = { failed = true },
|
||||
modifier = Modifier
|
||||
.size(size)
|
||||
.clip(RoundedCornerShape(6.dp)),
|
||||
@@ -93,6 +117,34 @@ fun EntryIcon(entry: FileEntry, size: Dp) {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a thumbnail is worth attempting: something with a picture in it, at a real path.
|
||||
*
|
||||
* `startsWith("/")` is this app's test for "a file the platform can open by path" — the same test the
|
||||
* built-in viewers make, and the reason the SAF fallback cannot show a thumbnail at all.
|
||||
*/
|
||||
private fun FileEntry.hasThumbnail(): Boolean =
|
||||
(kind == FileKind.IMAGE || kind == FileKind.VIDEO) && path.value.startsWith("/")
|
||||
|
||||
/**
|
||||
* What to hand Coil: the file itself for a picture, and for a video a request that asks for a frame.
|
||||
*
|
||||
* The frame is taken [VIDEO_FRAME_MILLIS] in rather than at zero, because the first frame of a
|
||||
* recording is very often black or the middle of a fade-in, and a thumbnail of a black rectangle is
|
||||
* no more useful than the icon it replaced.
|
||||
*/
|
||||
private fun FileEntry.thumbnailRequest(context: Context): Any {
|
||||
val file = File(path.value)
|
||||
if (kind != FileKind.VIDEO) return file
|
||||
return ImageRequest.Builder(context)
|
||||
.data(file)
|
||||
.videoFrameMillis(VIDEO_FRAME_MILLIS)
|
||||
.build()
|
||||
}
|
||||
|
||||
/** Where in a video its thumbnail is taken: far enough in to be past the usual fade-in. */
|
||||
private const val VIDEO_FRAME_MILLIS = 1_000L
|
||||
|
||||
/** The icon that stands for a file category. */
|
||||
fun iconFor(kind: FileKind): ImageVector = when (kind) {
|
||||
FileKind.DIRECTORY -> Icons.Filled.Folder
|
||||
|
||||
@@ -357,15 +357,20 @@ fun BrowserScreen(
|
||||
query = state.query,
|
||||
searchOpen = searchOpen,
|
||||
searchScope = state.searchScope,
|
||||
// The choice only exists when there is a folder to make it about.
|
||||
scopeChoosable = state.path != null,
|
||||
canPaste = state.clipboard?.isEmpty == false,
|
||||
onUp = viewModel::navigateUp,
|
||||
onQuery = viewModel::setQuery,
|
||||
onSearchOpen = { open ->
|
||||
searchOpen = open
|
||||
// With no folder open there is nothing for "this folder" to mean, so the
|
||||
// scope is switched as the field opens: its hint then says what the search
|
||||
// will actually do, instead of inviting a query that could never match.
|
||||
if (open && state.isRootView && state.searchScope != SearchScope.EVERYWHERE) {
|
||||
// With no folder open there is nothing for "this folder" to mean — which is
|
||||
// true of the volume list *and* of a library, since a library is not a folder
|
||||
// either. The scope is switched as the field opens so that its hint says what
|
||||
// the search will actually do, instead of inviting a query that could never
|
||||
// match. The view model makes the same assumption on its own account, so this
|
||||
// is the UI agreeing with the answer rather than deciding it.
|
||||
if (open && state.path == null && state.searchScope != SearchScope.EVERYWHERE) {
|
||||
viewModel.setSearchScope(SearchScope.EVERYWHERE)
|
||||
}
|
||||
},
|
||||
|
||||
@@ -47,6 +47,8 @@ fun BrowserTopBar(
|
||||
query: String,
|
||||
searchOpen: Boolean,
|
||||
searchScope: SearchScope,
|
||||
/** Whether the scope is the user's to choose at all; see the note beside the toggle. */
|
||||
scopeChoosable: Boolean,
|
||||
canPaste: Boolean,
|
||||
onUp: () -> Unit,
|
||||
onQuery: (String) -> Unit,
|
||||
@@ -116,22 +118,27 @@ fun BrowserTopBar(
|
||||
// Which folder tree a search answers for is a property of the search, not of
|
||||
// the folder being browsed, so it lives with the field rather than in the
|
||||
// overflow menu — and while the field is open it is one tap from either
|
||||
// answer to the other.
|
||||
IconButton(onClick = onSearchScope) {
|
||||
Icon(
|
||||
imageVector = if (searchScope == SearchScope.EVERYWHERE) {
|
||||
Icons.Filled.Public
|
||||
} else {
|
||||
Icons.Filled.Folder
|
||||
},
|
||||
contentDescription = stringResource(
|
||||
if (searchScope == SearchScope.EVERYWHERE) {
|
||||
R.string.search_scope_folder
|
||||
// answer to the other. It is absent when there is no folder to choose about
|
||||
// (the volume list, or a library, neither of which is one): a control whose
|
||||
// two settings do the same thing is worse than no control at all, because it
|
||||
// implies a choice the user does not have.
|
||||
if (scopeChoosable) {
|
||||
IconButton(onClick = onSearchScope) {
|
||||
Icon(
|
||||
imageVector = if (searchScope == SearchScope.EVERYWHERE) {
|
||||
Icons.Filled.Public
|
||||
} else {
|
||||
R.string.search_scope_everywhere
|
||||
Icons.Filled.Folder
|
||||
},
|
||||
),
|
||||
)
|
||||
contentDescription = stringResource(
|
||||
if (searchScope == SearchScope.EVERYWHERE) {
|
||||
R.string.search_scope_folder
|
||||
} else {
|
||||
R.string.search_scope_everywhere
|
||||
},
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
IconButton(onClick = { onSearchOpen(false); onQuery("") }) {
|
||||
Icon(
|
||||
|
||||
Reference in New Issue
Block a user