From 3caebbf5ad2241c5fda9f02412f04cb44ca2d1f7 Mon Sep 17 00:00:00 2001
From: Googler <noreply@google.com>
Date: Thu, 12 Sep 2024 07:27:42 -0700
Subject: [PATCH] Fix erroneous use of "cpu_features" in AV1 JNI exports.

`CPU_FEATURES_COMPILED_ANY_ARM_NEON` is always defined when `CPU_FEATURES_ARCH_ARM` is but it can be defined as `0` or `1` ([cpu_features source](https://github.com/google/cpu_features/blob/8e60d3f9be36e451d23d1e1a0caa6b853c182632/include/cpu_features_macros.h#L237-L245)). This patch makes sure to use the value of `CPU_FEATURES_COMPILED_ANY_ARM_NEON` instead of whether it is defined or not.

PiperOrigin-RevId: 673837522
---
 .../decoder_av1/src/main/jni/gav1_jni.cc      | 38 ++++++++++++-------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/libraries/decoder_av1/src/main/jni/gav1_jni.cc b/libraries/decoder_av1/src/main/jni/gav1_jni.cc
index 5143e51687..8c06a2aa43 100644
--- a/libraries/decoder_av1/src/main/jni/gav1_jni.cc
+++ b/libraries/decoder_av1/src/main/jni/gav1_jni.cc
@@ -19,12 +19,23 @@
 #include <android/native_window_jni.h>
 
 #include "cpu_features_macros.h"  // NOLINT
+
+// For ARMv7, we use `cpu_feature` to detect availability of NEON at runtime.
 #ifdef CPU_FEATURES_ARCH_ARM
 #include "cpuinfo_arm.h"  // NOLINT
 #endif                    // CPU_FEATURES_ARCH_ARM
-#ifdef CPU_FEATURES_COMPILED_ANY_ARM_NEON
-#include <arm_neon.h>
+
+// For ARM in general (v7/v8) we detect compile time availability of NEON.
+#ifdef CPU_FEATURES_ARCH_ANY_ARM
+#if CPU_FEATURES_COMPILED_ANY_ARM_NEON  // always defined to 0 or 1.
+#define HAS_COMPILE_TIME_NEON_SUPPORT
 #endif  // CPU_FEATURES_COMPILED_ANY_ARM_NEON
+#endif  // CPU_FEATURES_ARCH_ANY_ARM
+
+#ifdef HAS_COMPILE_TIME_NEON_SUPPORT
+#include <arm_neon.h>
+#endif
+
 #include <jni.h>
 
 #include <cstdint>
@@ -400,7 +411,7 @@ void Convert10BitFrameTo8BitDataBuffer(
   }
 }
 
-#ifdef CPU_FEATURES_COMPILED_ANY_ARM_NEON
+#ifdef HAS_COMPILE_TIME_NEON_SUPPORT
 void Convert10BitFrameTo8BitDataBufferNeon(
     const libgav1::DecoderBuffer* decoder_buffer, jbyte* data) {
   uint32x2_t lcg_value = vdup_n_u32(random());
@@ -497,7 +508,7 @@ void Convert10BitFrameTo8BitDataBufferNeon(
     }
   }
 }
-#endif  // CPU_FEATURES_COMPILED_ANY_ARM_NEON
+#endif  // HAS_COMPILE_TIME_NEON_SUPPORT
 
 }  // namespace
 
@@ -507,20 +518,19 @@ DECODER_FUNC(jlong, gav1Init, jint threads) {
     return kStatusError;
   }
 
-#ifdef CPU_FEATURES_ARCH_ARM
-  // Libgav1 requires NEON with arm ABIs.
-#ifdef CPU_FEATURES_COMPILED_ANY_ARM_NEON
-  const cpu_features::ArmFeatures arm_features =
-      cpu_features::GetArmInfo().features;
-  if (!arm_features.neon) {
+#ifdef CPU_FEATURES_ARCH_ANY_ARM       // Arm v7/v8
+#ifndef HAS_COMPILE_TIME_NEON_SUPPORT  // no compile time NEON support
+#ifdef CPU_FEATURES_ARCH_ARM           // check runtime support for ARMv7
+  if (cpu_features::GetArmInfo().features.neon == false) {
     context->jni_status_code = kJniStatusNeonNotSupported;
     return reinterpret_cast<jlong>(context);
   }
-#else
+#else   // Unexpected case of an ARMv8 with no NEON support.
   context->jni_status_code = kJniStatusNeonNotSupported;
   return reinterpret_cast<jlong>(context);
-#endif  // CPU_FEATURES_COMPILED_ANY_ARM_NEON
 #endif  // CPU_FEATURES_ARCH_ARM
+#endif  // HAS_COMPILE_TIME_NEON_SUPPORT
+#endif  // CPU_FEATURES_ARCH_ANY_ARM
 
   libgav1::DecoderSettings settings;
   settings.threads = threads;
@@ -613,11 +623,11 @@ DECODER_FUNC(jint, gav1GetFrame, jlong jContext, jobject jOutputBuffer,
         CopyFrameToDataBuffer(decoder_buffer, data);
         break;
       case 10:
-#ifdef CPU_FEATURES_COMPILED_ANY_ARM_NEON
+#ifdef HAS_COMPILE_TIME_NEON_SUPPORT
         Convert10BitFrameTo8BitDataBufferNeon(decoder_buffer, data);
 #else
         Convert10BitFrameTo8BitDataBuffer(decoder_buffer, data);
-#endif  // CPU_FEATURES_COMPILED_ANY_ARM_NEON
+#endif  // HAS_COMPILE_TIME_NEON_SUPPORT
         break;
       default:
         context->jni_status_code = kJniStatusBitDepth12NotSupportedWithYuv;