fix: screenshot permission (#28), back handling (#30), cleanup (#35)

android/app/lint-baseline.xml

@@ -1,28 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <issues format="6" by="lint 8.13.1" type="baseline" client="gradle" dependencies="false" name="AGP (8.13.1)" variant="all" version="8.13.1">
 
-    <issue
-        id="GestureBackNavigation"
-        message="If intercepting back events, this should be handled through the registration of callbacks; see https://developer.android.com/guide/navigation/custom-back/predictive-back-gesture"
-        errorLine1="            case KeyEvent.KEYCODE_BACK:"
-        errorLine2="                 ~~~~~~~~~~~~~~~~~~~~~">
-        <location
-            file="src/main/java/com/openipc/decoder/Decoder.java"
-            line="799"
-            column="18"/>
-    </issue>
-
-    <issue
-        id="UnusedAttribute"
-        message="Attribute `usesCleartextTraffic` is only used in API level 23 and higher (current min is 21)"
-        errorLine1="        android:usesCleartextTraffic=&quot;true&quot;>"
-        errorLine2="        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
-        <location
-            file="src/main/AndroidManifest.xml"
-            line="20"
-            column="9"/>
-    </issue>
-
     <issue
         id="DefaultUncaughtExceptionDelegation"
         message="Must call `getDefaultUncaughtExceptionHandler()` to get the existing handler, and call `existingHandler.uncaughtException(thread, throwable)` from your new handler"
@@ -63,7 +41,7 @@
         errorLine2="            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/AndroidManifest.xml"
-            line="27"
+            line="30"
             column="13"/>
     </issue>
 
@@ -96,7 +74,7 @@
         errorLine2="                             ~~~~~">
         <location
             file="src/main/AndroidManifest.xml"
-            line="15"
+            line="16"
             column="30"/>
     </issue>
 

android/app/proguard-rules.pro

@@ -4,5 +4,4 @@
 # proguard-rules.pro — R8/ProGuard keep rules for the Decoder application
 #
 
-# Keep the Frame record used across threads via BlockingQueue
--keep class com.openipc.decoder.Decoder$Frame { *; }
+# No keep rules needed: nothing is accessed by reflection

android/app/src/main/AndroidManifest.xml

@@ -1,5 +1,6 @@
 <?xml version="1.0" encoding="utf-8"?>
-<manifest xmlns:android="http://schemas.android.com/apk/res/android">
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:tools="http://schemas.android.com/tools">
     <uses-permission android:name="android.permission.INTERNET" />
     <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"
         android:maxSdkVersion="28" />
@@ -17,7 +18,9 @@
         android:banner="@mipmap/ic_launcher"
         android:icon="@mipmap/ic_launcher"
         android:label="@string/app_name"
-        android:usesCleartextTraffic="true">
+        android:usesCleartextTraffic="true"
+        android:enableOnBackInvokedCallback="true"
+        tools:targetApi="33">
         <!-- Separate process: it survives the crashed main process being killed -->
         <activity android:name="Crash" android:exported="false" android:process=":crash" />
         <activity

android/app/src/main/java/com/openipc/decoder/Decoder.java

@@ -66,7 +66,7 @@ public class Decoder extends Activity
     // --- Shared state ---
     final BlockingQueue<Frame> nalQueue = new ArrayBlockingQueue<>(32);
     final FramePool framePool = new FramePool(50);
-    final NalAssembler nalAssembler = new NalAssembler(1024 * 1024, () -> {
+    final NalAssembler nalAssembler = new NalAssembler(MediaCodecManager.MAX_NAL_SIZE, () -> {
         closeDecoder();
         nalQueue.clear();
     }, framePool);
@@ -696,17 +696,44 @@ private void removeQuadOverlay(int index) {
 
     // ====================== Screen / zoom ======================
 
+    private static final int REQUEST_STORAGE_FOR_SCREENSHOT = 1;
+
     private void takeScreenshot() {
         if (isTakingScreenshot) return;
-        isTakingScreenshot = true;
+
+        // Android 6-9 save through the shared Pictures folder: needs the runtime permission (#28)
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M
+                && Build.VERSION.SDK_INT < Build.VERSION_CODES.Q
+                && checkSelfPermission(android.Manifest.permission.WRITE_EXTERNAL_STORAGE)
+                        != PackageManager.PERMISSION_GRANTED) {
+            requestPermissions(new String[] {android.Manifest.permission.WRITE_EXTERNAL_STORAGE},
+                    REQUEST_STORAGE_FOR_SCREENSHOT);
+            return;
+        }
 
         Bitmap bitmap = mSurface.getBitmap();
         if (bitmap == null) {
             Toast.makeText(this, "No video frame available", Toast.LENGTH_SHORT).show();
-            isTakingScreenshot = false;
             return;
         }
+        isTakingScreenshot = true;
+        // Encoding a full-resolution PNG takes long: do it off the UI thread (#35)
+        new Thread(() -> saveScreenshot(bitmap), "screenshot").start();
+    }
 
+    @Override
+    public void onRequestPermissionsResult(int requestCode, String[] permissions, int[] results) {
+        super.onRequestPermissionsResult(requestCode, permissions, results);
+        if (requestCode != REQUEST_STORAGE_FOR_SCREENSHOT) return;
+        if (results.length > 0 && results[0] == PackageManager.PERMISSION_GRANTED) {
+            takeScreenshot();
+        } else {
+            Toast.makeText(this, "Storage permission is needed to save screenshots",
+                    Toast.LENGTH_LONG).show();
+        }
+    }
+
+    private void saveScreenshot(Bitmap bitmap) {
         String timeStamp = new SimpleDateFormat("yyyyMMdd_HHmmss", Locale.getDefault()).format(new Date());
         String fileName = "Screenshot_" + timeStamp + ".png";
 
@@ -758,7 +785,7 @@ private void takeScreenshot() {
             }
         }
         bitmap.recycle();
-        isTakingScreenshot = false;
+        runOnUiThread(() -> isTakingScreenshot = false);
     }
 
     private void updateResolution(int width, int height) {
@@ -970,12 +997,16 @@ public boolean onKeyDown(int keyCode, KeyEvent event) {
                             mVersion, BuildConfig.GIT_HASH, mHosts);
                 }
                 return true;
-            case KeyEvent.KEYCODE_BACK:
+            // Back is not intercepted here (#30): the menu is a focusable PopupWindow and the
+            // dialogs are Dialogs, which handle back themselves, also as predictive back
+            // gestures (OnBackInvokedDispatcher, Android 13+). Without a popup, back keeps its
+            // default action.
             case KeyEvent.KEYCODE_ESCAPE:
                 if (menuManager.isPopupShowing()) {
                     menuManager.dismissPopup();
                     return true;
                 }
+                return super.onKeyDown(keyCode, event);
             default:
                 return super.onKeyDown(keyCode, event);
         }

android/app/src/main/java/com/openipc/decoder/FramePool.java

@@ -10,31 +10,58 @@
 import java.util.concurrent.ArrayBlockingQueue;
 import java.util.concurrent.BlockingQueue;
 
+/**
+ * Two size classes: small frames (RTP packets, audio, most NAL units) and large frames
+ * (key frames). Keeping them apart stops small requests from taking the large buffers and
+ * key frames from always allocating; the large class is bounded to cap retained memory.
+ */
 class FramePool {
-    private final BlockingQueue<Frame> pool;
-    private final int maxSize;
+    /** Frames up to this capacity are "small": one interleaved RTP packet fits. */
+    static final int SMALL_MAX = 65536 + 4;
+    private static final int SMALL_MIN = 2048;
+    private static final int LARGE_STEP = 256 * 1024;
+
+    private final BlockingQueue<Frame> small;
+    private final BlockingQueue<Frame> large;
 
     FramePool(int maxSize) {
-        this.maxSize = maxSize;
-        this.pool = new ArrayBlockingQueue<>(maxSize);
+        this.small = new ArrayBlockingQueue<>(maxSize);
+        this.large = new ArrayBlockingQueue<>(Math.max(1, maxSize / 8));
     }
 
     Frame obtain(int size) {
-        Frame frame = pool.poll();
-        if (frame != null && frame.data().length >= size) {
-            frame.setLength(0);
-            return frame;
+        BlockingQueue<Frame> queue = size <= SMALL_MAX ? small : large;
+        // Look at each pooled frame at most once; undersized ones go back for later requests
+        for (int i = queue.size(); i > 0; i--) {
+            Frame frame = queue.poll();
+            if (frame == null) break;
+            if (frame.data().length >= size) {
+                frame.setLength(0);
+                return frame;
+            }
+            queue.offer(frame);
         }
-        return new Frame(new byte[size], 0);
+        // Round the capacity up so that the frame can be reused for similar sizes
+        return new Frame(new byte[capacityFor(size)], 0);
     }
 
     void recycle(Frame frame) {
-        if (frame != null && pool.size() < maxSize) {
-            pool.offer(frame);
-        }
+        if (frame == null) return;
+        // offer() drops the frame when its class is full
+        (frame.data().length <= SMALL_MAX ? small : large).offer(frame);
     }
 
     void clear() {
-        pool.clear();
+        small.clear();
+        large.clear();
+    }
+
+    static int capacityFor(int size) {
+        if (size <= SMALL_MAX) {
+            int c = SMALL_MIN;
+            while (c < size) c <<= 1;
+            return Math.min(c, SMALL_MAX);
+        }
+        return ((size + LARGE_STEP - 1) / LARGE_STEP) * LARGE_STEP;
     }
 }

android/app/src/main/java/com/openipc/decoder/MediaCodecManager.java

@@ -24,6 +24,9 @@ class MediaCodecManager {
     static final int H264_NAL_SPS = 7;
     static final int H264_NAL_PPS = 8;
 
+    /** Largest NAL unit (single view); 4K key frames at high bitrate can exceed 1 MiB. */
+    static final int MAX_NAL_SIZE = 2 * 1024 * 1024;
+
     private volatile MediaCodec decoder;
     private final Object decoderLock = new Object();
     /** Back-off after failures, instead of giving up for good (#21). */
@@ -125,7 +128,8 @@ void createDecoder(boolean codecH265) {
         String type = codecH265 ? "video/hevc" : "video/avc";
 
         MediaFormat format = MediaFormat.createVideoFormat(type, 1280, 720);
-        format.setInteger(MediaFormat.KEY_MAX_INPUT_SIZE, 1024 * 1024);
+        // Must hold the largest NAL unit the assembler can deliver
+        format.setInteger(MediaFormat.KEY_MAX_INPUT_SIZE, MAX_NAL_SIZE);
 
         MediaCodec local;
         try {

android/app/src/test/java/com/openipc/decoder/FramePoolTest.java

@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) OpenIPC  https://openipc.org  Prosperity Public License 3.0.0
+ *
+ * FramePoolTest.java — size classes and reuse in FramePool
+ *
+ */
+
+package com.openipc.decoder;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+
+import org.junit.Test;
+
+public class FramePoolTest {
+    @Test
+    public void capacityIsRoundedUpForReuse() {
+        assertEquals(2048, FramePool.capacityFor(100));
+        assertEquals(2048, FramePool.capacityFor(1400));
+        assertEquals(4096, FramePool.capacityFor(2049));
+        assertEquals(FramePool.SMALL_MAX, FramePool.capacityFor(65536 + 4));
+        assertEquals(256 * 1024, FramePool.capacityFor(70_000));
+        assertEquals(512 * 1024, FramePool.capacityFor(300_000));
+    }
+
+    @Test
+    public void smallFramesAreReused() {
+        FramePool pool = new FramePool(4);
+        Frame a = pool.obtain(1400);
+        pool.recycle(a);
+        Frame b = pool.obtain(1200);
+        assertSame("same capacity class is reused", a, b);
+        assertEquals(0, b.length());
+    }
+
+    @Test
+    public void smallRequestsDoNotTakeLargeBuffers() {
+        FramePool pool = new FramePool(8);
+        Frame big = pool.obtain(500_000);
+        pool.recycle(big);
+        Frame small = pool.obtain(1400);
+        assertNotSame(big, small);
+        assertTrue(small.data().length <= FramePool.SMALL_MAX);
+        // ...and the large buffer is still there for the next key frame
+        assertSame(big, pool.obtain(400_000));
+    }
+
+    @Test
+    public void largeClassIsBounded() {
+        FramePool pool = new FramePool(16); // large class holds 2
+        Frame[] f = {pool.obtain(300_000), pool.obtain(300_000), pool.obtain(300_000)};
+        for (Frame x : f) pool.recycle(x);
+        assertSame(f[0], pool.obtain(300_000));
+        assertSame(f[1], pool.obtain(300_000));
+        assertNotSame(f[2], pool.obtain(300_000)); // the third was dropped, not retained
+    }
+
+    @Test
+    public void undersizedFramesAreKeptNotDiscarded() {
+        FramePool pool = new FramePool(8);
+        Frame small = pool.obtain(1000);   // 2 KiB
+        Frame bigger = pool.obtain(10_000); // 16 KiB
+        pool.recycle(small);
+        pool.recycle(bigger);
+        // A 10 KB request skips the 2 KiB frame without losing it
+        assertSame(bigger, pool.obtain(10_000));
+        assertSame(small, pool.obtain(1000));
+    }
+}