admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix potential null pointer access in HardwareVideoEncoder

Browse Source 
There was no check for null in the code that prepends config buffer to key frame buffer. It is not a requirement for codec to deliver config buffer. Some codecs probably do not do that.

Bug: none
Change-Id: Id9c57efc5d1de5f569fa773313da4db3cd8b074c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299900
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Sergey Silkin <ssilkin@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39807}
This commit is contained in:
Sergey Silkin 2023-04-02 06:11:18 +00:00 committed by WebRTC LUCI CQ
parent c2d37895a2
commit 89facfc421
2 changed files with 164 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

134
sdk/android/src/java/org/webrtc/HardwareVideoEncoder.java
View File

@ -579,75 +579,81 @@ class HardwareVideoEncoder implements VideoEncoder {
return;
}
ByteBuffer codecOutputBuffer = codec.getOutputBuffer(index);
codecOutputBuffer.position(info.offset);
codecOutputBuffer.limit(info.offset + info.size);
ByteBuffer outputBuffer = codec.getOutputBuffer(index);
outputBuffer.position(info.offset);
outputBuffer.limit(info.offset + info.size);
if ((info.flags & MediaCodec.BUFFER_FLAG_CODEC_CONFIG) != 0) {
Logging.d(TAG, "Config frame generated. Offset: " + info.offset + ". Size: " + info.size);
configBuffer = ByteBuffer.allocateDirect(info.size);
configBuffer.put(codecOutputBuffer);
} else {
bitrateAdjuster.reportEncodedFrame(info.size);
if (adjustedBitrate != bitrateAdjuster.getAdjustedBitrateBps()) {
updateBitrate();
if (info.size > 0
&& (codecType == VideoCodecMimeType.H264 || codecType == VideoCodecMimeType.H265)) {
// In case of H264 and H265 config buffer contains SPS and PPS headers. Presence of these
// headers makes IDR frame a truly keyframe. Some encoders issue IDR frames without SPS
// and PPS. We save config buffer here to prepend it to all IDR frames encoder delivers.
configBuffer = ByteBuffer.allocateDirect(info.size);
configBuffer.put(outputBuffer);
}
final boolean isKeyFrame = (info.flags & MediaCodec.BUFFER_FLAG_SYNC_FRAME) != 0;
if (isKeyFrame) {
Logging.d(TAG, "Sync frame generated");
}
final ByteBuffer frameBuffer;
if (isKeyFrame && codecType == VideoCodecMimeType.H264) {
Logging.d(TAG,
"Prepending config frame of size " + configBuffer.capacity()
+ " to output buffer with offset " + info.offset + ", size " + info.size);
// For H.264 key frame prepend SPS and PPS NALs at the start.
frameBuffer = ByteBuffer.allocateDirect(info.size + configBuffer.capacity());
configBuffer.rewind();
frameBuffer.put(configBuffer);
frameBuffer.put(codecOutputBuffer);
frameBuffer.rewind();
} else {
frameBuffer = codecOutputBuffer.slice();
}
final EncodedImage.FrameType frameType = isKeyFrame
? EncodedImage.FrameType.VideoFrameKey
: EncodedImage.FrameType.VideoFrameDelta;
outputBuffersBusyCount.increment();
EncodedImage.Builder builder = outputBuilders.poll();
builder
.setBuffer(frameBuffer,
() -> {
// This callback should not throw any exceptions since
// it may be called on an arbitrary thread.
// Check bug webrtc:11230 for more details.
try {
codec.releaseOutputBuffer(index, false);
} catch (Exception e) {
Logging.e(TAG, "releaseOutputBuffer failed", e);
}
outputBuffersBusyCount.decrement();
})
.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);
}
}
EncodedImage encodedImage = builder.createEncodedImage();
// TODO(mellem): Set codec-specific info.
callback.onEncodedFrame(encodedImage, new CodecSpecificInfo());
// Note that the callback may have retained the image.
encodedImage.release();
return;
}
bitrateAdjuster.reportEncodedFrame(info.size);
if (adjustedBitrate != bitrateAdjuster.getAdjustedBitrateBps()) {
updateBitrate();
}
final boolean isKeyFrame = (info.flags & MediaCodec.BUFFER_FLAG_SYNC_FRAME) != 0;
if (isKeyFrame) {
Logging.d(TAG, "Sync frame generated");
}
final ByteBuffer frameBuffer;
final Runnable releaseCallback;
if (isKeyFrame && configBuffer != null) {
Logging.d(TAG,
"Prepending config buffer of size " + configBuffer.capacity()
+ " to output buffer with offset " + info.offset + ", size " + info.size);
frameBuffer = ByteBuffer.allocateDirect(info.size + configBuffer.capacity());
configBuffer.rewind();
frameBuffer.put(configBuffer);
frameBuffer.put(outputBuffer);
frameBuffer.rewind();
codec.releaseOutputBuffer(index, /* render= */ false);
releaseCallback = null;
} else {
frameBuffer = outputBuffer.slice();
outputBuffersBusyCount.increment();
releaseCallback = () -> {
// This callback should not throw any exceptions since
// it may be called on an arbitrary thread.
// Check bug webrtc:11230 for more details.
try {
codec.releaseOutputBuffer(index, /* render= */ false);
} catch (Exception e) {
Logging.e(TAG, "releaseOutputBuffer failed", e);
}
outputBuffersBusyCount.decrement();
};
}
final EncodedImage.FrameType frameType = isKeyFrame ? EncodedImage.FrameType.VideoFrameKey
: 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);
}
}
EncodedImage encodedImage = builder.createEncodedImage();
// TODO(mellem): Set codec-specific info.
callback.onEncodedFrame(encodedImage, new CodecSpecificInfo());
// Note that the callback may have retained the image.
encodedImage.release();
} catch (IllegalStateException e) {
Logging.e(TAG, "deliverOutput failed", e);
}

101
sdk/android/tests/src/org/webrtc/HardwareVideoEncoderTest.java
View File

@ -10,15 +10,24 @@
package org.webrtc;
import static android.media.MediaCodec.BUFFER_FLAG_CODEC_CONFIG;
import static android.media.MediaCodec.BUFFER_FLAG_SYNC_FRAME;
import static com.google.common.truth.Truth.assertThat;
import static java.util.concurrent.TimeUnit.SECONDS;
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.nullable;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.webrtc.VideoCodecMimeType.AV1;
import static org.webrtc.VideoCodecMimeType.H264;
import static org.webrtc.VideoCodecMimeType.H265;
import static org.webrtc.VideoCodecMimeType.VP8;
import static org.webrtc.VideoCodecMimeType.VP9;
import android.media.MediaCodec;
import android.media.MediaCodecInfo;
@ -27,6 +36,7 @@ import android.os.Build.VERSION_CODES;
import android.os.Bundle;
import androidx.test.runner.AndroidJUnit4;
import java.nio.ByteBuffer;
import java.nio.charset.Charset;
import java.util.HashMap;
import java.util.Map;
import org.junit.Before;
@ -131,7 +141,7 @@ public class HardwareVideoEncoderTest {
}
private class TestEncoderBuilder {
private VideoCodecMimeType codecType = VideoCodecMimeType.VP8;
private VideoCodecMimeType codecType = VP8;
private BitrateAdjuster bitrateAdjuster = new BaseBitrateAdjuster();
private boolean isEncodingStatisticsSupported;
@ -187,8 +197,7 @@ public class HardwareVideoEncoderTest {
@Test
public void testInit() {
// Set-up.
HardwareVideoEncoder encoder =
new TestEncoderBuilder().setCodecType(VideoCodecMimeType.VP8).build();
HardwareVideoEncoder encoder = new TestEncoderBuilder().setCodecType(VP8).build();
// Test.
assertThat(encoder.initEncode(TEST_ENCODER_SETTINGS, mockEncoderCallback))
@ -203,8 +212,7 @@ public class HardwareVideoEncoderTest {
.isEqualTo(TEST_ENCODER_SETTINGS.width);
assertThat(mediaFormat.getInteger(MediaFormat.KEY_HEIGHT))
.isEqualTo(TEST_ENCODER_SETTINGS.height);
assertThat(mediaFormat.getString(MediaFormat.KEY_MIME))
.isEqualTo(VideoCodecMimeType.VP8.mimeType());
assertThat(mediaFormat.getString(MediaFormat.KEY_MIME)).isEqualTo(VP8.mimeType());
assertThat(fakeMediaCodecWrapper.getConfiguredFlags())
.isEqualTo(MediaCodec.CONFIGURE_FLAG_ENCODE);
@ -231,7 +239,7 @@ public class HardwareVideoEncoderTest {
fakeMediaCodecWrapper.addOutputData(CodecTestHelper.generateRandomData(100),
/* presentationTimestampUs= */ 0,
/* flags= */ MediaCodec.BUFFER_FLAG_SYNC_FRAME);
/* flags= */ BUFFER_FLAG_SYNC_FRAME);
encoder.waitDeliverEncodedImage();
@ -267,7 +275,7 @@ public class HardwareVideoEncoderTest {
fakeMediaCodecWrapper.addOutputData(CodecTestHelper.generateRandomData(100),
/* presentationTimestampUs= */ 0,
/* flags= */ MediaCodec.BUFFER_FLAG_SYNC_FRAME);
/* flags= */ BUFFER_FLAG_SYNC_FRAME);
encoder.waitDeliverEncodedImage();
@ -454,4 +462,83 @@ public class HardwareVideoEncoderTest {
assertThat(timestampCaptor.getAllValues())
.containsExactly(0L, frameDurationMs, 2 * frameDurationMs);
}
private void encodeWithConfigBuffer(VideoCodecMimeType codecType, boolean keyFrame,
boolean emptyConfig, String expected) throws InterruptedException {
String configData = emptyConfig ? "" : "config";
byte[] configBytes = configData.getBytes(Charset.defaultCharset());
byte[] frameBytes = "frame".getBytes(Charset.defaultCharset());
byte[] expectedBytes = expected.getBytes(Charset.defaultCharset());
TestEncoder encoder = new TestEncoderBuilder().setCodecType(codecType).build();
encoder.initEncode(TEST_ENCODER_SETTINGS, mockEncoderCallback);
encoder.encode(createTestVideoFrame(/* timestampNs= */ 0), ENCODE_INFO_DELTA_FRAME);
fakeMediaCodecWrapper.addOutputData(
configBytes, /* presentationTimestampUs= */ 0, /* flags= */ BUFFER_FLAG_CODEC_CONFIG);
encoder.waitDeliverEncodedImage();
fakeMediaCodecWrapper.addOutputData(frameBytes, /* presentationTimestampUs= */ 0,
/* flags= */ keyFrame ? BUFFER_FLAG_SYNC_FRAME : 0);
encoder.waitDeliverEncodedImage();
verify(mockEncoderCallback)
.onEncodedFrame(
argThat(
(EncodedImage encoded) -> encoded.buffer.equals(ByteBuffer.wrap(expectedBytes))),
nullable(CodecSpecificInfo.class));
assertThat(encoder.release()).isEqualTo(VideoCodecStatus.OK);
}
@Test
public void encode_vp8KeyFrame_nonEmptyConfig_configNotPrepended() throws InterruptedException {
encodeWithConfigBuffer(VP8, /*keyFrame=*/true, /* emptyConfig= */ false, "frame");
}
@Test
public void encode_vp9KeyFrame_nonEmptyConfig_configNotPrepended() throws InterruptedException {
encodeWithConfigBuffer(VP9, /*keyFrame=*/true, /* emptyConfig= */ false, "frame");
}
@Test
public void encode_av1KeyFrame_nonEmptyConfig_configNotPrepended() throws InterruptedException {
encodeWithConfigBuffer(AV1, /*keyFrame=*/true, /* emptyConfig= */ false, "frame");
}
@Test
public void encode_h264KeyFrame_nonEmptyConfig_configPrepended() throws InterruptedException {
encodeWithConfigBuffer(H264, /*keyFrame=*/true, /* emptyConfig= */ false, "configframe");
}
@Test
public void encode_h265KeyFrame_nonEmptyConfig_configPrepended() throws InterruptedException {
encodeWithConfigBuffer(H265, /*keyFrame=*/true, /* emptyConfig= */ false, "configframe");
}
@Test
public void encode_vp8DeltaFrame_nonEmptyConfig_configNotPrepended() throws InterruptedException {
encodeWithConfigBuffer(VP8, /*keyFrame=*/false, /* emptyConfig= */ false, "frame");
}
@Test
public void encode_vp9DeltaFrame_nonEmptyConfig_configNotPrepended() throws InterruptedException {
encodeWithConfigBuffer(VP9, /*keyFrame=*/false, /* emptyConfig= */ false, "frame");
}
@Test
public void encode_av1DeltaFrame_nonEmptyConfig_configNotPrepended() throws InterruptedException {
encodeWithConfigBuffer(AV1, /*keyFrame=*/false, /* emptyConfig= */ false, "frame");
}
@Test
public void encode_h264KeyFrame_emptyConfig_configNotPrepended() throws InterruptedException {
encodeWithConfigBuffer(H264, /*keyFrame=*/true, /* emptyConfig= */ true, "frame");
}
@Test
public void encode_h265KeyFrame_emptyConfig_configNotPrepended() throws InterruptedException {
encodeWithConfigBuffer(H265, /*keyFrame=*/true, /* emptyConfig= */ true, "frame");
}
}