test/ci: unit tests + lint in CI (#34); fix H.265 FU NAL header (#43)

.github/workflows/build.yml

@@ -1,6 +1,7 @@
 name: Build
 on:
   push:
+  pull_request:
   workflow_dispatch:
     inputs:
       commit_hash:
@@ -9,7 +10,51 @@ on:
         type: string
 
 jobs:
+  # Unit tests and lint; also runs for pull requests from forks (no secrets needed)
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Switch to specified commit (keep latest workflow files)
+        if: inputs.commit_hash != ''
+        run: |
+          git fetch --unshallow
+          git checkout ${{ inputs.commit_hash }}
+          git checkout ${{ github.ref_name }} -- .github/
+
+      - name: Set up JDK 17
+        uses: actions/setup-java@v4
+        with:
+          distribution: temurin
+          java-version: 17
+          cache: gradle
+
+      - name: Unit tests and lint
+        working-directory: android
+        run: |
+          ./gradlew testDebugUnitTest
+          # Commits from before the lint baseline would fail on known findings
+          if [ -f app/lint-baseline.xml ]; then
+            ./gradlew lintDebug
+          else
+            echo "No lint baseline in this commit, lint skipped"
+          fi
+
+      - name: Upload reports
+        if: failure()
+        uses: actions/upload-artifact@v4
+        with:
+          name: test-and-lint-reports
+          path: |
+            android/app/build/reports/
+            android/app/build/test-results/
+
+  # Signed release build; needs secrets, so it does not run for pull requests
   build:
+    needs: test
+    if: github.event_name != 'pull_request'
     runs-on: ubuntu-latest
     steps:
       - name: Checkout

android/app/build.gradle

@@ -57,4 +57,18 @@ android {
         sourceCompatibility = JavaVersion.VERSION_17
         targetCompatibility = JavaVersion.VERSION_17
     }
+
+    testOptions {
+        // android.jar stubs (Log, ...) return defaults instead of throwing in JVM unit tests
+        unitTests.returnDefaultValues = true
+    }
+
+    lint {
+        // Known issues are recorded in the baseline; only new ones fail the build
+        baseline = file("lint-baseline.xml")
+    }
+}
+
+dependencies {
+    testImplementation libs.junit
 }

android/app/lint-baseline.xml

@@ -0,0 +1,174 @@
+<?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="802"
+            column="18"/>
+    </issue>
+
+    <issue
+        id="SuspiciousIndentation"
+        message="Suspicious indentation: This is indented but is not nested under the previous expression (`if (listenerCb != nu`...)"
+        errorLine1="            s.setSoTimeout(0);"
+        errorLine2="            ~~~~~~~~~~~~~~~~~">
+        <location
+            file="src/main/java/com/openipc/decoder/RtspClient.java"
+            line="303"
+            column="13"/>
+        <location
+            file="src/main/java/com/openipc/decoder/RtspClient.java"
+            line="301"
+            column="9"
+            message="Previous statement here"/>
+    </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"
+        errorLine1="        Thread.setDefaultUncaughtExceptionHandler((thread, throwable) -> {"
+        errorLine2="        ^">
+        <location
+            file="src/main/java/com/openipc/decoder/App.java"
+            line="21"
+            column="9"/>
+    </issue>
+
+    <issue
+        id="AndroidGradlePluginVersion"
+        message="A newer version of Gradle than 8.13 is available: 8.14.6"
+        errorLine1="distributionUrl=https\://services.gradle.org/distributions/gradle-8.13-bin.zip"
+        errorLine2="                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
+        <location
+            file="$HOME/Test/decoder/android/gradle/wrapper/gradle-wrapper.properties"
+            line="4"
+            column="17"/>
+    </issue>
+
+    <issue
+        id="AndroidGradlePluginVersion"
+        message="A newer version of com.android.application than 8.13.1 is available: 9.4.1. (There is also a newer version of 8.13.𝑥 available, if upgrading to 9.4.1 is difficult: 8.13.2)"
+        errorLine1="agp = &quot;8.13.1&quot;"
+        errorLine2="      ~~~~~~~~">
+        <location
+            file="$HOME/Test/decoder/android/gradle/libs.versions.toml"
+            line="8"
+            column="7"/>
+    </issue>
+
+    <issue
+        id="DiscouragedApi"
+        message="Fixed screen orientations will be ignored in most cases, starting from Android 16. Android is moving toward a model where apps are expected to adapt to various orientations, display sizes, and aspect ratios."
+        errorLine1="            android:screenOrientation=&quot;sensorLandscape&quot;"
+        errorLine2="            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
+        <location
+            file="src/main/AndroidManifest.xml"
+            line="27"
+            column="13"/>
+    </issue>
+
+    <issue
+        id="AuthLeak"
+        message="Possible credential leak"
+        errorLine1="    private static final String DEFAULT_URL = &quot;rtsp://root:12345@192.168.1.10:554/stream=0&quot;;"
+        errorLine2="                                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
+        <location
+            file="src/main/java/com/openipc/decoder/Decoder.java"
+            line="89"
+            column="48"/>
+    </issue>
+
+    <issue
+        id="AuthLeak"
+        message="Possible credential leak"
+        errorLine1="    private static final String DEFAULT_URL = &quot;rtsp://root:12345@192.168.1.10:554/stream=0&quot;;"
+        errorLine2="                                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
+        <location
+            file="src/main/java/com/openipc/decoder/MenuManager.java"
+            line="38"
+            column="48"/>
+    </issue>
+
+    <issue
+        id="DataExtractionRules"
+        message="The attribute `android:allowBackup` is deprecated from Android 12 and higher and may be removed in future versions. Consider adding the attribute `android:dataExtractionRules` specifying an `@xml` resource which configures cloud backups and device transfers on Android 12 and higher."
+        errorLine1="        android:allowBackup=&quot;false&quot;"
+        errorLine2="                             ~~~~~">
+        <location
+            file="src/main/AndroidManifest.xml"
+            line="15"
+            column="30"/>
+    </issue>
+
+    <issue
+        id="MergeRootFrame"
+        message="This `&lt;FrameLayout>` can be replaced with a `&lt;merge>` tag"
+        errorLine1="&lt;FrameLayout xmlns:android=&quot;http://schemas.android.com/apk/res/android&quot;"
+        errorLine2="^">
+        <location
+            file="src/main/res/layout/decoder.xml"
+            line="2"
+            column="1"/>
+    </issue>
+
+    <issue
+        id="ClickableViewAccessibility"
+        message="Custom view ``TextureView`` has `setOnTouchListener` called on it but does not override `performClick`"
+        errorLine1="        mSurface.setOnTouchListener((v, event) -> {"
+        errorLine2="        ^">
+        <location
+            file="src/main/java/com/openipc/decoder/Decoder.java"
+            line="213"
+            column="9"/>
+    </issue>
+
+    <issue
+        id="ClickableViewAccessibility"
+        message="`onTouch` lambda should call `View#performClick` when a click is detected"
+        errorLine1="        mSurface.setOnTouchListener((v, event) -> {"
+        errorLine2="                                    ^">
+        <location
+            file="src/main/java/com/openipc/decoder/Decoder.java"
+            line="213"
+            column="37"/>
+    </issue>
+
+    <issue
+        id="SetTextI18n"
+        message="Do not concatenate text displayed with `setText`. Use resource string with placeholders."
+        errorLine1="        statusText.setText(&quot;Connected • &quot; + videoCodec + aCodec + bps);"
+        errorLine2="                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
+        <location
+            file="src/main/java/com/openipc/decoder/Decoder.java"
+            line="419"
+            column="28"/>
+    </issue>
+
+    <issue
+        id="SetTextI18n"
+        message="String literal in `setText` can not be translated. Use Android resources instead."
+        errorLine1="        statusText.setText(&quot;Connected • &quot; + videoCodec + aCodec + bps);"
+        errorLine2="                           ~~~~~~~~~~~~~~">
+        <location
+            file="src/main/java/com/openipc/decoder/Decoder.java"
+            line="419"
+            column="28"/>
+    </issue>
+
+</issues>

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

@@ -16,7 +16,10 @@
 import android.util.Log;
 
 import java.nio.ByteBuffer;
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
 import java.util.concurrent.ArrayBlockingQueue;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.ExecutorService;
@@ -289,46 +292,68 @@ private void releaseAacDecoder() {
     }
 
     private void processAac(AudioConfig c, byte[] data, int start, int end) {
+        List<int[]> units;
         if (c.sizeLength > 0) {
-            // RFC 3640 mpeg4-generic: AU-headers-length (bits), AU headers, then AU data
-            if (end - start < 2) return;
-            int headerBits = ((data[start] & 0xFF) << 8) | (data[start + 1] & 0xFF);
-            int bitPos = (start + 2) * 8;
-            int bitEnd = bitPos + headerBits;
-            int pos = start + 2 + (headerBits + 7) / 8;
-            if (pos > end) return;
             ensureAacDecoder(c, false);
-            boolean first = true;
-            while (true) {
-                int headerLen = c.sizeLength + (first ? c.indexLength : c.indexDeltaLength);
-                if (bitPos + headerLen > bitEnd) break;
-                int size = readBits(data, bitPos, c.sizeLength);
-                bitPos += headerLen;
-                first = false;
-                // AUs fragmented across packets are not supported
-                if (size <= 0 || pos + size > end) break;
-                offerAac(Arrays.copyOfRange(data, pos, pos + size));
-                pos += size;
-            }
+            units = splitAuHeaders(data, start, end,
+                    c.sizeLength, c.indexLength, c.indexDeltaLength);
         } else if (end - start >= 7 && isAdtsSync(data, start)) {
             ensureAacDecoder(c, true);
-            int pos = start;
-            while (end - pos >= 7 && isAdtsSync(data, pos)) {
-                int frameLen = ((data[pos + 3] & 0x03) << 11)
-                        | ((data[pos + 4] & 0xFF) << 3)
-                        | ((data[pos + 5] & 0xE0) >>> 5);
-                if (frameLen < 7 || pos + frameLen > end) break;
-                offerAac(Arrays.copyOfRange(data, pos, pos + frameLen));
-                pos += frameLen;
-            }
+            units = splitAdts(data, start, end);
         } else {
             // Raw access unit without AU headers
             ensureAacDecoder(c, false);
-            offerAac(Arrays.copyOfRange(data, start, end));
+            units = Collections.singletonList(new int[] {start, end - start});
+        }
+        for (int[] u : units) {
+            offerAac(Arrays.copyOfRange(data, u[0], u[0] + u[1]));
+        }
+    }
+
+    /**
+     * RFC 3640 mpeg4-generic payload: AU-headers-length (bits), AU headers, then AU data.
+     * Returns {offset, length} of each complete AU; AUs fragmented across packets are
+     * not supported and end the list.
+     */
+    static List<int[]> splitAuHeaders(byte[] data, int start, int end,
+                                      int sizeLength, int indexLength, int indexDeltaLength) {
+        List<int[]> units = new ArrayList<>();
+        if (end - start < 2 || sizeLength <= 0) return units;
+        int headerBits = ((data[start] & 0xFF) << 8) | (data[start + 1] & 0xFF);
+        int bitPos = (start + 2) * 8;
+        int bitEnd = bitPos + headerBits;
+        int pos = start + 2 + (headerBits + 7) / 8;
+        if (pos > end) return units;
+        boolean first = true;
+        while (true) {
+            int headerLen = sizeLength + (first ? indexLength : indexDeltaLength);
+            if (bitPos + headerLen > bitEnd) break;
+            int size = readBits(data, bitPos, sizeLength);
+            bitPos += headerLen;
+            first = false;
+            if (size <= 0 || pos + size > end) break;
+            units.add(new int[] {pos, size});
+            pos += size;
+        }
+        return units;
+    }
+
+    /** ADTS frames: {offset, length} of each complete frame, stopping at bad sync or length. */
+    static List<int[]> splitAdts(byte[] data, int start, int end) {
+        List<int[]> frames = new ArrayList<>();
+        int pos = start;
+        while (end - pos >= 7 && isAdtsSync(data, pos)) {
+            int frameLen = ((data[pos + 3] & 0x03) << 11)
+                    | ((data[pos + 4] & 0xFF) << 3)
+                    | ((data[pos + 5] & 0xE0) >>> 5);
+            if (frameLen < 7 || pos + frameLen > end) break;
+            frames.add(new int[] {pos, frameLen});
+            pos += frameLen;
         }
+        return frames;
     }
 
-    private static boolean isAdtsSync(byte[] data, int pos) {
+    static boolean isAdtsSync(byte[] data, int pos) {
         return (data[pos] & 0xFF) == 0xFF && (data[pos + 1] & 0xF6) == 0xF0;
     }
 

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

@@ -68,9 +68,10 @@ Frame assemble(Frame frame, boolean codecH265) {
                     nalBuffer[nalSize++] = 0;
                     nalBuffer[nalSize++] = 0;
                     nalBuffer[nalSize++] = 1;
-                    nalBuffer[nalSize++] = (byte)((rx[cp] & 0x81) | ((rx[cp + 2] & 0x7F) << 1));
-                    int donlField = (rx[cp + 1] & 0x80) >> 7;
-                    cp += donlField != 0 ? 3 : 2;
+                    // 2-byte NAL header: F + FU type + layer-id MSB, then layer-id/TID as sent
+                    nalBuffer[nalSize++] = (byte)((rx[cp] & 0x81) | ((rx[cp + 2] & 0x3F) << 1));
+                    nalBuffer[nalSize++] = rx[cp + 1];
+                    cp += 3; // payload header + FU header
                 } else {
                     nalBuffer[nalSize++] = 0;
                     nalBuffer[nalSize++] = 0;