From d73c0b3cc038162d70deace8e0fe44a42fe4f4a5 Mon Sep 17 00:00:00 2001
From: huangdarwin <huangdarwin@google.com>
Date: Tue, 21 Feb 2023 17:32:12 +0000
Subject: [PATCH] Effect: Implement isNoOp on GlEffect interfaces.

Also, allow isNoOp to default to false without the TODO, so that implementations
of isNoOp must opt-in to implementing the override in order to be considered for
skipping the effect (ex. for transcoding in Transformer).

PiperOrigin-RevId: 511223540
---
 .../src/main/java/androidx/media3/effect/Brightness.java | 9 ++++++++-
 .../src/main/java/androidx/media3/effect/Contrast.java   | 5 +++++
 .../src/main/java/androidx/media3/effect/GlEffect.java   | 3 ---
 .../main/java/androidx/media3/effect/HslAdjustment.java  | 5 +++++
 .../main/java/androidx/media3/effect/RgbAdjustment.java  | 6 ++++++
 5 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/libraries/effect/src/main/java/androidx/media3/effect/Brightness.java b/libraries/effect/src/main/java/androidx/media3/effect/Brightness.java
index 80c161e24c..f1691148e0 100644
--- a/libraries/effect/src/main/java/androidx/media3/effect/Brightness.java
+++ b/libraries/effect/src/main/java/androidx/media3/effect/Brightness.java
@@ -22,6 +22,7 @@ import android.opengl.Matrix;
 import androidx.annotation.FloatRange;
 import androidx.media3.common.util.GlUtil;
 import androidx.media3.common.util.UnstableApi;
+import java.util.Arrays;
 
 /** Modifies brightness of an input frame. */
 @UnstableApi
@@ -33,7 +34,8 @@ public class Brightness implements RgbMatrix {
    * Modifies brightness by adding a constant value to red, green, and blue values.
    *
    * @param brightness The constant value to add to red, green, and blue values. Should be greater
-   *     than or equal to -1f, and less than or equal to 1f.
+   *     than or equal to {@code -1f}, and less than or equal to {@code 1f}. {@code 0} means to
+   *     leave brightness unchanged.
    */
   public Brightness(@FloatRange(from = -1, to = 1) float brightness) {
     checkArgument(
@@ -53,4 +55,9 @@ public class Brightness implements RgbMatrix {
     checkArgument(!useHdr, "HDR is not supported.");
     return rgbMatrix;
   }
+
+  @Override
+  public boolean isNoOp(int inputWidth, int inputHeight) {
+    return Arrays.equals(rgbMatrix, GlUtil.create4x4IdentityMatrix());
+  }
 }
diff --git a/libraries/effect/src/main/java/androidx/media3/effect/Contrast.java b/libraries/effect/src/main/java/androidx/media3/effect/Contrast.java
index 48300cac67..8755eae90b 100644
--- a/libraries/effect/src/main/java/androidx/media3/effect/Contrast.java
+++ b/libraries/effect/src/main/java/androidx/media3/effect/Contrast.java
@@ -46,4 +46,9 @@ public class Contrast implements GlEffect {
       throws VideoFrameProcessingException {
     return new ContrastShaderProgram(context, this, useHdr);
   }
+
+  @Override
+  public boolean isNoOp(int inputWidth, int inputHeight) {
+    return contrast == 0f;
+  }
 }
diff --git a/libraries/effect/src/main/java/androidx/media3/effect/GlEffect.java b/libraries/effect/src/main/java/androidx/media3/effect/GlEffect.java
index b891e2be74..86b4af59a4 100644
--- a/libraries/effect/src/main/java/androidx/media3/effect/GlEffect.java
+++ b/libraries/effect/src/main/java/androidx/media3/effect/GlEffect.java
@@ -51,9 +51,6 @@ public interface GlEffect extends Effect {
    * @param inputHeight The input frame height, in pixels.
    */
   default boolean isNoOp(int inputWidth, int inputHeight) {
-    // TODO(b/265927935): Generalize this logic by implementing this method on all
-    //  subclasses, and deleting the default implementation here. Otherwise, some no-op effects may
-    //  not be properly detected or handled.
     return false;
   }
 }
diff --git a/libraries/effect/src/main/java/androidx/media3/effect/HslAdjustment.java b/libraries/effect/src/main/java/androidx/media3/effect/HslAdjustment.java
index bc2a44d707..786129a8d0 100644
--- a/libraries/effect/src/main/java/androidx/media3/effect/HslAdjustment.java
+++ b/libraries/effect/src/main/java/androidx/media3/effect/HslAdjustment.java
@@ -118,4 +118,9 @@ public class HslAdjustment implements GlEffect {
       throws VideoFrameProcessingException {
     return new HslShaderProgram(context, /* hslAdjustment= */ this, useHdr);
   }
+
+  @Override
+  public boolean isNoOp(int inputWidth, int inputHeight) {
+    return hueAdjustmentDegrees == 0f && saturationAdjustment == 0f && lightnessAdjustment == 0f;
+  }
 }
diff --git a/libraries/effect/src/main/java/androidx/media3/effect/RgbAdjustment.java b/libraries/effect/src/main/java/androidx/media3/effect/RgbAdjustment.java
index 57d2e970ff..e1eadd6187 100644
--- a/libraries/effect/src/main/java/androidx/media3/effect/RgbAdjustment.java
+++ b/libraries/effect/src/main/java/androidx/media3/effect/RgbAdjustment.java
@@ -23,6 +23,7 @@ import androidx.annotation.FloatRange;
 import androidx.media3.common.util.GlUtil;
 import androidx.media3.common.util.UnstableApi;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
+import java.util.Arrays;
 
 /** Scales the red, green, and blue color channels of a frame. */
 @UnstableApi
@@ -100,4 +101,9 @@ public final class RgbAdjustment implements RgbMatrix {
   public float[] getMatrix(long presentationTimeUs, boolean useHdr) {
     return rgbMatrix;
   }
+
+  @Override
+  public boolean isNoOp(int inputWidth, int inputHeight) {
+    return Arrays.equals(rgbMatrix, GlUtil.create4x4IdentityMatrix());
+  }
 }