From 6cf12bbe327f70652f00c6ac1b0af34085a82719 Mon Sep 17 00:00:00 2001
From: Sergey Silkin <ssilkin@webrtc.org>
Date: Sat, 15 Apr 2023 19:35:32 +0000
Subject: [PATCH] Fetch encoded QP before releasing output buffer

Before this change we first released output frame buffer in the code path which prepends config buffer to a keyframe and then called getOutputFormat(index). getOutputFormat(index) throws an exception if index points to a released buffer. This change rearranges calls such that getOutputFormat(index) always executed before releaseOutputBuffer(index).

Bug: webrtc:15015
Change-Id: Ia64f5d8e7483aeeb316d1a71c0cb79233f4bbee9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/301364
Commit-Queue: Sergey Silkin <ssilkin@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39874}
---
 .../java/org/webrtc/HardwareVideoEncoder.java | 21 ++++++++------
 .../org/webrtc/HardwareVideoEncoderTest.java  | 28 +++++++++++++++++++
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java b/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java
index 0a6e344455..773f2b9dd2 100644
--- a/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java
+++ b/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java
@@ -606,6 +606,15 @@ class HardwareVideoEncoder implements VideoEncoder {
         Logging.d(TAG, "Sync frame generated");
       }
 
+      // Extract QP before releasing output buffer.
+      Integer qp = null;
+      if (isEncodingStatisticsEnabled) {
+        MediaFormat format = codec.getOutputFormat(index);
+        if (format != null && format.containsKey(MediaFormat.KEY_VIDEO_QP_AVERAGE)) {
+          qp = format.getInteger(MediaFormat.KEY_VIDEO_QP_AVERAGE);
+        }
+      }
+
       final ByteBuffer frameBuffer;
       final Runnable releaseCallback;
       if (isKeyFrame && configBuffer != null) {
@@ -639,15 +648,9 @@ class HardwareVideoEncoder implements VideoEncoder {
                                                           : EncodedImage.FrameType.VideoFrameDelta;
 
       EncodedImage.Builder builder = outputBuilders.poll();
-      builder.setBuffer(frameBuffer, releaseCallback).setFrameType(frameType);
-
-      if (isEncodingStatisticsEnabled) {
-        MediaFormat format = codec.getOutputFormat(index);
-        if (format != null && format.containsKey(MediaFormat.KEY_VIDEO_QP_AVERAGE)) {
-          int qp = format.getInteger(MediaFormat.KEY_VIDEO_QP_AVERAGE);
-          builder.setQp(qp);
-        }
-      }
+      builder.setBuffer(frameBuffer, releaseCallback);
+      builder.setFrameType(frameType);
+      builder.setQp(qp);
 
       EncodedImage encodedImage = builder.createEncodedImage();
       // TODO(mellem):  Set codec-specific info.
diff --git a/sdk/android/tests/src/org/webrtc/HardwareVideoEncoderTest.java b/sdk/android/tests/src/org/webrtc/HardwareVideoEncoderTest.java
index 919975d46d..6c3af8a155 100644
--- a/sdk/android/tests/src/org/webrtc/HardwareVideoEncoderTest.java
+++ b/sdk/android/tests/src/org/webrtc/HardwareVideoEncoderTest.java
@@ -18,8 +18,10 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.argThat;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.ArgumentMatchers.nullable;
 import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -43,6 +45,7 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
+import org.mockito.InOrder;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.robolectric.annotation.Config;
@@ -288,6 +291,31 @@ public class HardwareVideoEncoderTest {
     assertThat(videoFrame.qp).isEqualTo(123);
   }
 
+  @Test
+  public void encodingStatistics_fetchedBeforeFrameBufferIsReleased() throws InterruptedException {
+    TestEncoder encoder =
+        new TestEncoderBuilder().setCodecType(H264).SetIsEncodingStatisticsSupported(true).build();
+    assertThat(encoder.initEncode(TEST_ENCODER_SETTINGS, mockEncoderCallback))
+        .isEqualTo(VideoCodecStatus.OK);
+
+    encoder.encode(createTestVideoFrame(/* timestampNs= */ 42), ENCODE_INFO_KEY_FRAME);
+    fakeMediaCodecWrapper.addOutputData(CodecTestHelper.generateRandomData(/* length= */ 100),
+        /* presentationTimestampUs= */ 0,
+        /* flags= */ MediaCodec.BUFFER_FLAG_CODEC_CONFIG);
+    encoder.waitDeliverEncodedImage();
+
+    int frameBufferIndex =
+        fakeMediaCodecWrapper.addOutputData(CodecTestHelper.generateRandomData(/* length= */ 100),
+            /* presentationTimestampUs= */ 0,
+            /* flags= */ MediaCodec.BUFFER_FLAG_SYNC_FRAME);
+    encoder.waitDeliverEncodedImage();
+
+    InOrder inOrder = inOrder(fakeMediaCodecWrapper);
+    inOrder.verify(fakeMediaCodecWrapper).getOutputFormat(eq(frameBufferIndex));
+    inOrder.verify(fakeMediaCodecWrapper)
+        .releaseOutputBuffer(eq(frameBufferIndex), /* render= */ eq(false));
+  }
+
   @Test
   public void testEncodeByteBuffer() {
     // Set-up.