Reland "Raise IllegalStateException for calls to retain() or release() on zero ref count"
This is a reland of 8a959bfa88b08e215baf3b38e914c41e483c9ece
Original change's description:
> Raise IllegalStateException for calls to retain() or release() on zero ref count
>
> Bug: None
> Change-Id: I3205e77b5adfdc4f5dbd7509d1ca0e8b08af62f2
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/142175
> Commit-Queue: Niels Moller <nisse@webrtc.org>
> Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#28319}
Bug: None
Change-Id: If8fb02ca149257dd29b0c3352347369168a5cef3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/142807
Reviewed-by: Magnus Jedvert <magjed@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28372}
diff --git a/sdk/android/src/java/org/webrtc/RefCountDelegate.java b/sdk/android/src/java/org/webrtc/RefCountDelegate.java
index fc8a191..58be7aa 100644
--- a/sdk/android/src/java/org/webrtc/RefCountDelegate.java
+++ b/sdk/android/src/java/org/webrtc/RefCountDelegate.java
@@ -29,12 +29,19 @@
@Override
public void retain() {
- refCount.incrementAndGet();
+ int updated_count = refCount.incrementAndGet();
+ if (updated_count < 2) {
+ throw new IllegalStateException("retain() called on an object with refcount < 1");
+ }
}
@Override
public void release() {
- if (refCount.decrementAndGet() == 0 && releaseCallback != null) {
+ int updated_count = refCount.decrementAndGet();
+ if (updated_count < 0) {
+ throw new IllegalStateException("release() called on an object with refcount < 1");
+ }
+ if (updated_count == 0 && releaseCallback != null) {
releaseCallback.run();
}
}
diff --git a/sdk/android/tests/src/org/webrtc/AndroidVideoDecoderTest.java b/sdk/android/tests/src/org/webrtc/AndroidVideoDecoderTest.java
index 693dc64..1b9b566 100644
--- a/sdk/android/tests/src/org/webrtc/AndroidVideoDecoderTest.java
+++ b/sdk/android/tests/src/org/webrtc/AndroidVideoDecoderTest.java
@@ -29,7 +29,10 @@
import android.media.MediaFormat;
import android.os.Handler;
import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.List;
import org.chromium.testing.local.LocalRobolectricTestRunner;
+import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -162,6 +165,25 @@
}
}
+ private static class FakeDecoderCallback implements VideoDecoder.Callback {
+ public final List<VideoFrame> decodedFrames;
+
+ public FakeDecoderCallback() {
+ decodedFrames = new ArrayList<>();
+ }
+
+ @Override
+ public void onDecodedFrame(VideoFrame frame, Integer decodeTimeMs, Integer qp) {
+ frame.retain();
+ decodedFrames.add(frame);
+ }
+
+ public void release() {
+ for (VideoFrame frame : decodedFrames) frame.release();
+ decodedFrames.clear();
+ }
+ }
+
private EncodedImage createTestEncodedImage() {
return EncodedImage.builder()
.setBuffer(ByteBuffer.wrap(ENCODED_TEST_DATA))
@@ -174,6 +196,7 @@
@Mock private SurfaceTextureHelper mockSurfaceTextureHelper;
@Mock private VideoDecoder.Callback mockDecoderCallback;
private FakeMediaCodecWrapper fakeMediaCodecWrapper;
+ private FakeDecoderCallback fakeDecoderCallback;
@Before
public void setUp() {
@@ -183,6 +206,12 @@
MediaFormat outputFormat = new MediaFormat();
// TODO(sakal): Add more details to output format as needed.
fakeMediaCodecWrapper = spy(new FakeMediaCodecWrapper(outputFormat));
+ fakeDecoderCallback = new FakeDecoderCallback();
+ }
+
+ @After
+ public void cleanUp() {
+ fakeDecoderCallback.release();
}
@Test
@@ -268,7 +297,7 @@
// Set-up.
TestDecoder decoder = new TestDecoderBuilder().setUseSurface(/* useSurface = */ false).build();
- decoder.initDecode(TEST_DECODER_SETTINGS, mockDecoderCallback);
+ decoder.initDecode(TEST_DECODER_SETTINGS, fakeDecoderCallback);
decoder.decode(createTestEncodedImage(),
new DecodeInfo(/* isMissingFrames= */ false, /* renderTimeMs= */ 0));
fakeMediaCodecWrapper.addOutputData(
@@ -278,13 +307,8 @@
decoder.waitDeliverDecodedFrame();
// Verify.
- ArgumentCaptor<VideoFrame> videoFrameCaptor = ArgumentCaptor.forClass(VideoFrame.class);
- verify(mockDecoderCallback)
- .onDecodedFrame(videoFrameCaptor.capture(),
- /* decodeTimeMs= */ any(Integer.class),
- /* qp= */ any());
-
- VideoFrame videoFrame = videoFrameCaptor.getValue();
+ assertThat(fakeDecoderCallback.decodedFrames).hasSize(1);
+ VideoFrame videoFrame = fakeDecoderCallback.decodedFrames.get(0);
assertThat(videoFrame).isNotNull();
assertThat(videoFrame.getRotatedWidth()).isEqualTo(TEST_DECODER_SETTINGS.width);
assertThat(videoFrame.getRotatedHeight()).isEqualTo(TEST_DECODER_SETTINGS.height);
@@ -346,7 +370,7 @@
public void testDeliversRenderedBuffers() throws InterruptedException {
// Set-up.
TestDecoder decoder = new TestDecoderBuilder().build();
- decoder.initDecode(TEST_DECODER_SETTINGS, mockDecoderCallback);
+ decoder.initDecode(TEST_DECODER_SETTINGS, fakeDecoderCallback);
decoder.decode(createTestEncodedImage(),
new DecodeInfo(/* isMissingFrames= */ false, /* renderTimeMs= */ 0));
fakeMediaCodecWrapper.addOutputTexture(/* presentationTimestampUs= */ 0, /* flags= */ 0);
@@ -370,16 +394,13 @@
outputVideoFrame.release();
// Verify.
- ArgumentCaptor<VideoFrame> videoFrameCaptor = ArgumentCaptor.forClass(VideoFrame.class);
- verify(mockDecoderCallback)
- .onDecodedFrame(videoFrameCaptor.capture(),
- /* decodeTimeMs= */ any(Integer.class),
- /* qp= */ any());
-
- VideoFrame videoFrame = videoFrameCaptor.getValue();
+ assertThat(fakeDecoderCallback.decodedFrames).hasSize(1);
+ VideoFrame videoFrame = fakeDecoderCallback.decodedFrames.get(0);
assertThat(videoFrame).isNotNull();
assertThat(videoFrame.getBuffer()).isEqualTo(outputTextureBuffer);
+ fakeDecoderCallback.release();
+
verify(releaseCallback).run();
}
