commit c73ea4fc57f1de9976158bd93346395761970b44
author Xavier Lepaul <xalep@webrtc.org> Tue May 30 07:39:55 2023
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue May 30 09:06:21 2023
tree 9845ce84a8dbc4e742bfcd0b78cab7e74f3c28eb
parent a3e9c0ae5afa3e2d45e7a099ea22883081d35087

More systematic null checks before calling native methods

None of these native methods perform null checks. Some of the Java
delegates were doing some null checks, but calling others with null
parameters would just result in native crashes that often lack context.

These more systematic checks will make debugging easier.

Bug: b/282038690
Change-Id: I3363abeede84c1bd93da397fe87c3d638a607107
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/306961
Reviewed-by: Linus Nilsson <lnilsson@webrtc.org>
Commit-Queue: Xavier Lepaul‎ <xalep@webrtc.org>
Reviewed-by: Ranveer Aggarwal‎ <ranvr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40175}

sdk/android/api/org/webrtc/YuvHelper.java

diff --git a/sdk/android/api/org/webrtc/YuvHelper.java b/sdk/android/api/org/webrtc/YuvHelper.java
index e5ddb61..8a46a57 100644
--- a/sdk/android/api/org/webrtc/YuvHelper.java
+++ b/sdk/android/api/org/webrtc/YuvHelper.java
@@ -115,6 +115,10 @@
   public static void I420Rotate(ByteBuffer srcY, int srcStrideY, ByteBuffer srcU, int srcStrideU,
       ByteBuffer srcV, int srcStrideV, ByteBuffer dst, int srcWidth, int srcHeight,
       int rotationMode) {
+    checkNotNull(srcY, "srcY");
+    checkNotNull(srcU, "srcU");
+    checkNotNull(srcV, "srcV");
+    checkNotNull(dst, "dst");
     final int dstWidth = rotationMode % 180 == 0 ? srcWidth : srcHeight;
     final int dstHeight = rotationMode % 180 == 0 ? srcHeight : srcWidth;
 
@@ -145,14 +149,16 @@
   /** Helper method for copying a single colour plane. */
   public static void copyPlane(
       ByteBuffer src, int srcStride, ByteBuffer dst, int dstStride, int width, int height) {
-    nativeCopyPlane(src, srcStride, dst, dstStride, width, height);
+    nativeCopyPlane(
+        checkNotNull(src, "src"), srcStride, checkNotNull(dst, "dst"), dstStride, width, height);
   }
 
   /** Converts ABGR little endian (rgba in memory) to I420. */
   public static void ABGRToI420(ByteBuffer src, int srcStride, ByteBuffer dstY, int dstStrideY,
       ByteBuffer dstU, int dstStrideU, ByteBuffer dstV, int dstStrideV, int width, int height) {
-    nativeABGRToI420(
-        src, srcStride, dstY, dstStrideY, dstU, dstStrideU, dstV, dstStrideV, width, height);
+    nativeABGRToI420(checkNotNull(src, "src"), srcStride, checkNotNull(dstY, "dstY"), dstStrideY,
+        checkNotNull(dstU, "dstU"), dstStrideU, checkNotNull(dstV, "dstV"), dstStrideV, width,
+        height);
   }
 
   /**
@@ -163,9 +169,14 @@
   public static void I420Copy(ByteBuffer srcY, int srcStrideY, ByteBuffer srcU, int srcStrideU,
       ByteBuffer srcV, int srcStrideV, ByteBuffer dstY, int dstStrideY, ByteBuffer dstU,
       int dstStrideU, ByteBuffer dstV, int dstStrideV, int width, int height) {
-    if (srcY == null || srcU == null || srcV == null || dstY == null || dstU == null || dstV == null
-        || width <= 0 || height <= 0) {
-      throw new IllegalArgumentException("Invalid I420Copy input arguments");
+    checkNotNull(srcY, "srcY");
+    checkNotNull(srcU, "srcU");
+    checkNotNull(srcV, "srcV");
+    checkNotNull(dstY, "dstY");
+    checkNotNull(dstU, "dstU");
+    checkNotNull(dstV, "dstV");
+    if (width <= 0 || height <= 0) {
+      throw new IllegalArgumentException("I420Copy: width and height should not be negative");
     }
     nativeI420Copy(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV, dstY, dstStrideY, dstU,
         dstStrideU, dstV, dstStrideV, width, height);
@@ -174,9 +185,13 @@
   public static void I420ToNV12(ByteBuffer srcY, int srcStrideY, ByteBuffer srcU, int srcStrideU,
       ByteBuffer srcV, int srcStrideV, ByteBuffer dstY, int dstStrideY, ByteBuffer dstUV,
       int dstStrideUV, int width, int height) {
-    if (srcY == null || srcU == null || srcV == null || dstY == null || dstUV == null || width <= 0
-        || height <= 0) {
-      throw new IllegalArgumentException("Invalid I420ToNV12 input arguments");
+    checkNotNull(srcY, "srcY");
+    checkNotNull(srcU, "srcU");
+    checkNotNull(srcV, "srcV");
+    checkNotNull(dstY, "dstY");
+    checkNotNull(dstUV, "dstUV");
+    if (width <= 0 || height <= 0) {
+      throw new IllegalArgumentException("I420ToNV12: width and height should not be negative");
     }
     nativeI420ToNV12(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV, dstY, dstStrideY, dstUV,
         dstStrideUV, width, height);
@@ -186,10 +201,23 @@
       ByteBuffer srcV, int srcStrideV, ByteBuffer dstY, int dstStrideY, ByteBuffer dstU,
       int dstStrideU, ByteBuffer dstV, int dstStrideV, int srcWidth, int srcHeight,
       int rotationMode) {
+    checkNotNull(srcY, "srcY");
+    checkNotNull(srcU, "srcU");
+    checkNotNull(srcV, "srcV");
+    checkNotNull(dstY, "dstY");
+    checkNotNull(dstU, "dstU");
+    checkNotNull(dstV, "dstV");
     nativeI420Rotate(srcY, srcStrideY, srcU, srcStrideU, srcV, srcStrideV, dstY, dstStrideY, dstU,
         dstStrideU, dstV, dstStrideV, srcWidth, srcHeight, rotationMode);
   }
 
+  private static <T> T checkNotNull(T obj, String description) {
+    if (obj == null) {
+      throw new NullPointerException(description + " should not be null");
+    }
+    return obj;
+  }
+
   private static native void nativeCopyPlane(
       ByteBuffer src, int srcStride, ByteBuffer dst, int dstStride, int width, int height);
   private static native void nativeI420Copy(ByteBuffer srcY, int srcStrideY, ByteBuffer srcU,