Add image and video thumbnails in file browser#638
Open
lewinpauli wants to merge 3 commits into
Open
Conversation
|
|
Author
|
ups i just noticed that #533 is already trying the same for a much longer time |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis PR implements thumbnail support for images and videos in the Cryptomator Android file browser. It adds a Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/adapter/BrowseFilesAdapter.kt`:
- Around line 145-151: In BrowseFilesAdapter's bind path where you set
non-thumbnail icons (the else branch that calls bindCloudNodeImage and
binding.cloudNodeImage.setImageResource), clear any previous thumbnail request
state on the ImageView before setting the static icon: e.g., remove/reset the
view tag used by thumbnailUtil (the same tag used when calling
thumbnailUtil.loadThumbnail) and cancel or detach any pending thumbnail callback
so an in-flight thumbnail callback cannot overwrite folder/other icons; ensure
this uses the same ImageView (binding.cloudNodeImage) and thumbnailUtil
interaction patterns as in the thumbnail branch.
In
`@presentation/src/main/java/org/cryptomator/presentation/util/ThumbnailUtil.kt`:
- Around line 41-46: The cached-thumbnail decode currently runs on the main
thread (BitmapFactory.decodeFile on cachedFile) inside the bind path; move the
decode work onto the provided executor and only post target.setImageBitmap(...)
back onto mainHandler. Specifically, in ThumbnailUtil replace the inline
BitmapFactory.decodeFile(cachedFile.absolutePath) call with executor.execute {
val bmp = BitmapFactory.decodeFile(cachedFile.absolutePath); if (bmp != null)
mainHandler.post { target.setImageBitmap(bmp) } } and ensure the original
early-return behavior is preserved (i.e., return from the bind method only after
scheduling the decode/posting so the cached path short-circuits further work).
- Around line 71-74: The current generateImageThumbnail uses downloadToByteArray
and decodeScaledBitmap which load the entire image into memory; instead, change
the flow to download the image to a temporary file (file-backed), then perform a
bounds-only decode (BitmapFactory.Options.inJustDecodeBounds = true) on that
temp file to compute an appropriate inSampleSize, and finally perform a sampled
decode from the file (e.g., decodeFile or decodeStream) and pass that Bitmap to
saveThumbnail; replace uses of downloadToByteArray/decodeScaledBitmap with a
file-backed download + bounds decode + sampled decode pattern, and apply the
same fix to the analogous code referenced at lines ~95-100 so both image
thumbnail paths use file-backed decoding to avoid OOMs.
- Around line 82-89: Replace the MediaMetadataRetriever().use { ... } block with
an explicit try/finally to call retriever.release() (because AutoCloseable was
added API 29 and .use() will crash on minSdk 26). Instantiate
MediaMetadataRetriever as a val (e.g., retriever = MediaMetadataRetriever()),
then in try setDataSource(tmpFile.absolutePath), call getFrameAtTime and handle
the null return the same way, call scaleBitmap(frame) and saveThumbnail(bitmap,
cachedFile) inside the try, and ensure retriever.release() is called in the
finally block; keep existing return behavior and references to
MediaMetadataRetriever, scaleBitmap, saveThumbnail, tmpFile and cachedFile.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cc9d2ea7-8e11-4bfd-8229-389f45034dfa
📒 Files selected for processing (8)
.gitignore.idea/misc.xmlpresentation/src/main/java/org/cryptomator/presentation/di/component/ApplicationComponent.javapresentation/src/main/java/org/cryptomator/presentation/ui/adapter/BrowseFilesAdapter.ktpresentation/src/main/java/org/cryptomator/presentation/util/ThumbnailUtil.ktpresentation/src/main/res/layout/item_browse_files_node.xmlpresentation/src/main/res/values/dimens.xmlutil/src/main/java/org/cryptomator/util/file/LruFileCacheUtil.kt
💤 Files with no reviewable changes (1)
- .idea/misc.xml
Comment on lines
+71
to
+74
| private fun generateImageThumbnail(file: CloudFileModel, cachedFile: File): Bitmap? { | ||
| val bytes = downloadToByteArray(file) ?: return null | ||
| val bitmap = decodeScaledBitmap(bytes) ?: return null | ||
| saveThumbnail(bitmap, cachedFile) |
There was a problem hiding this comment.
Current image path loads entire files into heap before sampling.
Downloading full image bytes first defeats memory savings from inSampleSize and can trigger OOM on large photos. Prefer file-backed decode (download to temp file, bounds decode + sampled decode from file).
Also applies to: 95-100
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@presentation/src/main/java/org/cryptomator/presentation/util/ThumbnailUtil.kt`
around lines 71 - 74, The current generateImageThumbnail uses
downloadToByteArray and decodeScaledBitmap which load the entire image into
memory; instead, change the flow to download the image to a temporary file
(file-backed), then perform a bounds-only decode
(BitmapFactory.Options.inJustDecodeBounds = true) on that temp file to compute
an appropriate inSampleSize, and finally perform a sampled decode from the file
(e.g., decodeFile or decodeStream) and pass that Bitmap to saveThumbnail;
replace uses of downloadToByteArray/decodeScaledBitmap with a file-backed
download + bounds decode + sampled decode pattern, and apply the same fix to the
analogous code referenced at lines ~95-100 so both image thumbnail paths use
file-backed decoding to avoid OOMs.
…triever API 26 compat and stale tag cleanup on rebind
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #41
Adds image and video thumbnails in the file browser vault view.
in cacheDir/thumbnails/