commit 6cf12bbe327f70652f00c6ac1b0af34085a82719
author Sergey Silkin <ssilkin@webrtc.org> Sat Apr 15 19:35:32 2023
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Apr 17 12:31:32 2023
tree 85909094bee4d23f6c66243706a1d61c5084e748
parent ea1502accbd03d678ff820f522f1e8788625b3e6

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}

sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java

diff --git a/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java b/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java
index 0a6e344..773f2b9 100644
--- a/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java
+++ b/sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java
@@ -606,6 +606,15 @@
         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 @@
                                                           : 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.

sdk/android/tests/src/org/webrtc/HardwareVideoEncoderTest.java

diff --git a/sdk/android/tests/src/org/webrtc/HardwareVideoEncoderTest.java b/sdk/android/tests/src/org/webrtc/HardwareVideoEncoderTest.java
index 919975d..6c3af8a 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.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.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;
@@ -289,6 +292,31 @@
   }
 
   @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.
     HardwareVideoEncoder encoder = new TestEncoderBuilder().build();