From 18f1f0c1f55cb66ff2844673b9cf94db50086b16 Mon Sep 17 00:00:00 2001 From: Niels Moller Date: Wed, 19 Jun 2019 08:50:44 +0000 Subject: [PATCH] Revert "Raise IllegalStateException for calls to retain() or release() on zero ref count" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 8a959bfa88b08e215baf3b38e914c41e483c9ece. Reason for revert: Breaks a downstream test. 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 > Reviewed-by: Sami Kalliomäki > Cr-Commit-Position: refs/heads/master@{#28319} TBR=sakal@webrtc.org,nisse@webrtc.org Change-Id: I522cc5264789d8c7088de6df6e47584622265a94 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: None Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/142806 Reviewed-by: Niels Moller Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#28320} --- .../src/java/org/webrtc/RefCountDelegate.java | 11 +--- .../org/webrtc/AndroidVideoDecoderTest.java | 53 ++++++------------- 2 files changed, 18 insertions(+), 46 deletions(-) diff --git a/sdk/android/src/java/org/webrtc/RefCountDelegate.java b/sdk/android/src/java/org/webrtc/RefCountDelegate.java index 58be7aa0fb..fc8a1910b4 100644 --- a/sdk/android/src/java/org/webrtc/RefCountDelegate.java +++ b/sdk/android/src/java/org/webrtc/RefCountDelegate.java @@ -29,19 +29,12 @@ class RefCountDelegate implements RefCounted { @Override public void retain() { - int updated_count = refCount.incrementAndGet(); - if (updated_count < 2) { - throw new IllegalStateException("retain() called on an object with refcount < 1"); - } + refCount.incrementAndGet(); } @Override public void release() { - 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) { + if (refCount.decrementAndGet() == 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 1b9b56608d..693dc6491f 100644 --- a/sdk/android/tests/src/org/webrtc/AndroidVideoDecoderTest.java +++ b/sdk/android/tests/src/org/webrtc/AndroidVideoDecoderTest.java @@ -29,10 +29,7 @@ import android.media.MediaCodecInfo.CodecCapabilities; 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; @@ -165,25 +162,6 @@ public class AndroidVideoDecoderTest { } } - private static class FakeDecoderCallback implements VideoDecoder.Callback { - public final List 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)) @@ -196,7 +174,6 @@ public class AndroidVideoDecoderTest { @Mock private SurfaceTextureHelper mockSurfaceTextureHelper; @Mock private VideoDecoder.Callback mockDecoderCallback; private FakeMediaCodecWrapper fakeMediaCodecWrapper; - private FakeDecoderCallback fakeDecoderCallback; @Before public void setUp() { @@ -206,12 +183,6 @@ public class AndroidVideoDecoderTest { 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 @@ -297,7 +268,7 @@ public class AndroidVideoDecoderTest { // Set-up. TestDecoder decoder = new TestDecoderBuilder().setUseSurface(/* useSurface = */ false).build(); - decoder.initDecode(TEST_DECODER_SETTINGS, fakeDecoderCallback); + decoder.initDecode(TEST_DECODER_SETTINGS, mockDecoderCallback); decoder.decode(createTestEncodedImage(), new DecodeInfo(/* isMissingFrames= */ false, /* renderTimeMs= */ 0)); fakeMediaCodecWrapper.addOutputData( @@ -307,8 +278,13 @@ public class AndroidVideoDecoderTest { decoder.waitDeliverDecodedFrame(); // Verify. - assertThat(fakeDecoderCallback.decodedFrames).hasSize(1); - VideoFrame videoFrame = fakeDecoderCallback.decodedFrames.get(0); + ArgumentCaptor videoFrameCaptor = ArgumentCaptor.forClass(VideoFrame.class); + verify(mockDecoderCallback) + .onDecodedFrame(videoFrameCaptor.capture(), + /* decodeTimeMs= */ any(Integer.class), + /* qp= */ any()); + + VideoFrame videoFrame = videoFrameCaptor.getValue(); assertThat(videoFrame).isNotNull(); assertThat(videoFrame.getRotatedWidth()).isEqualTo(TEST_DECODER_SETTINGS.width); assertThat(videoFrame.getRotatedHeight()).isEqualTo(TEST_DECODER_SETTINGS.height); @@ -370,7 +346,7 @@ public class AndroidVideoDecoderTest { public void testDeliversRenderedBuffers() throws InterruptedException { // Set-up. TestDecoder decoder = new TestDecoderBuilder().build(); - decoder.initDecode(TEST_DECODER_SETTINGS, fakeDecoderCallback); + decoder.initDecode(TEST_DECODER_SETTINGS, mockDecoderCallback); decoder.decode(createTestEncodedImage(), new DecodeInfo(/* isMissingFrames= */ false, /* renderTimeMs= */ 0)); fakeMediaCodecWrapper.addOutputTexture(/* presentationTimestampUs= */ 0, /* flags= */ 0); @@ -394,13 +370,16 @@ public class AndroidVideoDecoderTest { outputVideoFrame.release(); // Verify. - assertThat(fakeDecoderCallback.decodedFrames).hasSize(1); - VideoFrame videoFrame = fakeDecoderCallback.decodedFrames.get(0); + ArgumentCaptor videoFrameCaptor = ArgumentCaptor.forClass(VideoFrame.class); + verify(mockDecoderCallback) + .onDecodedFrame(videoFrameCaptor.capture(), + /* decodeTimeMs= */ any(Integer.class), + /* qp= */ any()); + + VideoFrame videoFrame = videoFrameCaptor.getValue(); assertThat(videoFrame).isNotNull(); assertThat(videoFrame.getBuffer()).isEqualTo(outputTextureBuffer); - fakeDecoderCallback.release(); - verify(releaseCallback).run(); }