Merged
Conversation
WalkthroughThe PR removes the ML Kit barcode-scanning dependency and replaces ML Kit-based async barcode processing in Changes
Sequence Diagram(s)sequenceDiagram
participant Camera as Camera (CameraX)
participant Analyzer as Analyzer (ImageProxy)
participant ZXing as ZXing MultiFormatReader
participant Collector as FrameCollector
participant Main as MainExecutor / UI
Camera->>Analyzer: deliver frame (ImageProxy)
Analyzer->>Analyzer: extract luminance planes (LuminanceBuffers)
Analyzer->>Analyzer: rotate/copy luminance if needed
Analyzer->>ZXing: decode luminance -> try decode QR
ZXing-->>Analyzer: result (success/failure)
alt success & validator + scanned gate
Analyzer->>Collector: processQrContent(...)
Analyzer->>Main: post onCodeScanned / update UI
end
Analyzer->>Camera: close imageProxy (finally)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/main/kotlin/io/privkey/keep/ImportShareScreen.kt (1)
100-147: Per-frame allocations in the hot path.
rotateLuminanceplus theByteArray(expected)copy indecodeQrFromImageProxyallocate two fresh byte arrays sizedrowStride*heightandwidth*heighton every analyzer frame. WithSTRATEGY_KEEP_ONLY_LATESTthat's still steady allocation at camera cadence and creates noticeable GC pressure on low-end devices. Since the analyzer executor is single-threaded, a pair of reusable per-scanner buffers (reallocated only when dimensions change) would be safe and much cheaper. Not a correctness issue — feel free to defer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/io/privkey/keep/ImportShareScreen.kt` around lines 100 - 147, rotateLuminance and decodeQrFromImageProxy currently allocate fresh ByteArray buffers on every analyzer frame causing GC pressure; change them to reuse two per-scanner buffers (one sized rowStride*height for the source/copy and one sized width*height for rotated output) kept as class-level properties (e.g., on ImportShareScreen or the scanner instance used by the single-threaded analyzer) and only reallocate when width/height/rowStride change; update rotateLuminance to write into the reused output buffer instead of creating a new ByteArray and make decodeQrFromImageProxy use the reusable source buffer for the rowStride copy, ensuring synchronization is unnecessary because the analyzer executor is single-threaded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/kotlin/io/privkey/keep/ImportShareScreen.kt`:
- Around line 591-599: The cleanupResources() routine risks calling
reader.reset() while a decodeWithState is still running because
executor.shutdownNow() + awaitTermination(100 ms) doesn't guarantee the analyzer
thread has stopped; update cleanupResources() to only call reader.reset() after
confirming the executor terminated successfully (e.g., check awaitTermination
returned true) or otherwise synchronize access to the MultiFormatReader used by
decodeWithState and cleanupResources (e.g., a shared lock around reader.reset()
and decodeWithState) so that reader.reset() cannot run concurrently with decode
operations; reference the methods/fields cleanupResources, executor.shutdownNow,
executor.awaitTermination, reader.reset and decodeWithState when making the
change.
- Around line 56-66: The Y-plane buffer-size check uses rowStride * height which
rejects valid frames when rowStride > width; update the logic in the image
handling where you access imageProxy.planes.firstOrNull() / plane.buffer to
allocate data of size width * height and copy per-row into that array: for each
row read min(width, rowStride) bytes from buffer accounting for
plane.pixelStride (guarding for pixelStride != 1 by reading pixels
individually), rather than expecting contiguous rowStride*height bytes; keep
buffer.rewind() and ensure you don't return null just because buffer.remaining()
< rowStride * height but instead validate buffer has at least
rowStride*(height-1)+width or perform safe per-row reads.
---
Nitpick comments:
In `@app/src/main/kotlin/io/privkey/keep/ImportShareScreen.kt`:
- Around line 100-147: rotateLuminance and decodeQrFromImageProxy currently
allocate fresh ByteArray buffers on every analyzer frame causing GC pressure;
change them to reuse two per-scanner buffers (one sized rowStride*height for the
source/copy and one sized width*height for rotated output) kept as class-level
properties (e.g., on ImportShareScreen or the scanner instance used by the
single-threaded analyzer) and only reallocate when width/height/rowStride
change; update rotateLuminance to write into the reused output buffer instead of
creating a new ByteArray and make decodeQrFromImageProxy use the reusable source
buffer for the rowStride copy, ensuring synchronization is unnecessary because
the analyzer executor is single-threaded.
🪄 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: 511449ad-5fa0-4282-ac4f-78b2257b4a62
📒 Files selected for processing (2)
app/build.gradle.ktsapp/src/main/kotlin/io/privkey/keep/ImportShareScreen.kt
💤 Files with no reviewable changes (1)
- app/build.gradle.kts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/kotlin/io/privkey/keep/ImportShareScreen.kt (1)
72-72: Consider adding a defensive pixelStride check.While YUV_420_888 Y plane should always have
pixelStride == 1per the Android specification, adding a defensive check would guard against unexpected device behavior.🛡️ Optional defensive check
val plane = imageProxy.planes.firstOrNull() ?: return null +if (plane.pixelStride != 1) return null val buffer = plane.buffer🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/kotlin/io/privkey/keep/ImportShareScreen.kt` at line 72, The code currently assumes the Y plane's pixelStride is 1 when doing val plane = imageProxy.planes.firstOrNull() ?: return null; add a defensive check on plane.pixelStride (e.g., if (plane.pixelStride != 1) ...) and handle that case gracefully—either convert by copying/scaling the buffer into a contiguous byte array or return null/skip processing—so that the routine that reads the Y buffer (in ImportShareScreen's image-to-byte/bitmap logic using imageProxy and plane) does not misinterpret strided data on devices that violate the expectation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/kotlin/io/privkey/keep/ImportShareScreen.kt`:
- Line 72: The code currently assumes the Y plane's pixelStride is 1 when doing
val plane = imageProxy.planes.firstOrNull() ?: return null; add a defensive
check on plane.pixelStride (e.g., if (plane.pixelStride != 1) ...) and handle
that case gracefully—either convert by copying/scaling the buffer into a
contiguous byte array or return null/skip processing—so that the routine that
reads the Y buffer (in ImportShareScreen's image-to-byte/bitmap logic using
imageProxy and plane) does not misinterpret strided data on devices that violate
the expectation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 56d7d1c7-a56b-499c-a63f-7bfba02ec863
📒 Files selected for processing (1)
app/src/main/kotlin/io/privkey/keep/ImportShareScreen.kt
This was referenced Apr 19, 2026
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.
Replaces Google ML Kit barcode scanner with ZXing for F-Droid eligibility. Hardens scanner thread safety and buffer bounds.
Summary by CodeRabbit
Refactor
Chores