From bf2c3f23fab544122c1b16c21f5fc3c1fd9abc6f Mon Sep 17 00:00:00 2001 From: codelogic Date: Fri, 26 Jun 2026 19:03:40 +0000 Subject: [PATCH] Handle closed images in AndroidImageReader Wrap image acquisition and AndroidImage construction in a try-catch block to handle IllegalStateException that can occur if the ImageReader or Image is closed concurrently. Ensure that the acquired image is safely closed if wrapping fails, and invoke the image listener outside the try-catch block to avoid catching listener-specific exceptions. Additionally, ensure that the expected outputs listener is fired if the image timestamp is successfully resolved, even if the subsequent wrapping in AndroidImage fails. Also add corresponding unit tests for both regular and multi-resolution image readers. Bug: 520091791 Change-Id: I517ccc55d876a51d0f02b9a9d9243f691050933f --- .../camera2/pipe/media/AndroidImageReaders.kt | 100 +++--- .../pipe/media/AndroidImageReaderTest.kt | 297 ++++++++++++++++++ 2 files changed, 362 insertions(+), 35 deletions(-) diff --git a/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/media/AndroidImageReaders.kt b/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/media/AndroidImageReaders.kt index 6c074ec0c6b57..1939e45afe07a 100644 --- a/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/media/AndroidImageReaders.kt +++ b/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/media/AndroidImageReaders.kt @@ -19,6 +19,7 @@ package androidx.camera.camera2.pipe.media import android.hardware.camera2.MultiResolutionImageReader import android.hardware.camera2.params.MultiResolutionStreamInfo import android.hardware.camera2.params.OutputConfiguration +import android.media.Image import android.media.ImageReader import android.os.Build import android.os.Handler @@ -59,18 +60,23 @@ internal constructor( atomic(null) override fun onImageAvailable(reader: ImageReader?) { - val image = reader?.acquireNextImage() - if (image != null) { - val imageListener = onImageListener - if (imageListener == null) { - image.close() - return - } + if (reader == null) return + + val imageListener = onImageListener + if (imageListener == null) { + reader.closeNext() + return + } - onExpectedOutputsListener?.onExpectedOutputs(image.timestamp, outputIdSet) + val expectedOutputsListener = onExpectedOutputsListener - imageListener.onImage(streamId, outputId, AndroidImage(image)) + val wrappedImage = reader.acquireNext() ?: return + val timestamp = wrappedImage.timestamp + + if (expectedOutputsListener != null) { + expectedOutputsListener.onExpectedOutputs(timestamp, outputIdSet) } + imageListener.onImage(streamId, outputId, wrappedImage) } override fun close(): Unit = imageReader.close() @@ -230,37 +236,30 @@ public class AndroidMultiResolutionImageReader( atomic(null) override fun onImageAvailable(reader: ImageReader?) { - if (reader != null) imageReaderSet.add(reader) - val image = reader?.acquireNextImage() - if (image != null) { - val imageListener = onImageListener - if (imageListener == null) { - image.close() - return - } + if (reader == null) return + imageReaderSet.add(reader) - // MultiResolutionImageReaders produce images from multiple sub-ImageReaders, in order - // to figure out which output the image is from, we have to first look up the - // StreamInfo from the MultiResolutionImageReader instance, and then use it to look it - // up in the outputMap that was used to create the MultiResolutionImageReader. - val streamInfo = multiResolutionImageReader.getStreamInfoForImageReader(reader) - val outputId = - checkNotNull(streamInfoToOutputIdMap[streamInfo]) { - "$this: Failed to find OutputId for $reader based on streamInfo $streamInfo!" - } + val imageListener = onImageListener + if (imageListener == null) { + reader.closeNext() + return + } + + val expectedOutputsListener = onExpectedOutputsListener + + val wrappedImage = reader.acquireNext() ?: return + val timestamp = wrappedImage.timestamp - if (!concurrentOutputsEnabled) { - // For non-concurrent streams, we cannot rely on onActiveOutputSurfaces. Hence, we - // fire the expected outputs listener here. - onExpectedOutputsListener?.onExpectedOutputs(image.timestamp, setOf(outputId)) + val streamInfo = multiResolutionImageReader.getStreamInfoForImageReader(reader) + val outputId = + checkNotNull(streamInfoToOutputIdMap[streamInfo]) { + "$this: Failed to find OutputId for $reader based on streamInfo $streamInfo!" } - // Note: During camera switches, MultiResolutionImageReaders does not guarantee that - // images will always be in monotonically increasing order. The primary reason for this - // is when a camera switches from one lens to another, which can cause the camera - // to produce overlapping images from each sensor and can be delivered out of order. - imageListener.onImage(streamId, outputId, AndroidImage(image)) + if (expectedOutputsListener != null && !concurrentOutputsEnabled) { + expectedOutputsListener.onExpectedOutputs(timestamp, setOf(outputId)) } + imageListener.onImage(streamId, outputId, wrappedImage) } override fun onActiveOutputSurfaces( @@ -452,3 +451,34 @@ public class AndroidMultiResolutionImageReader( } } } + +private fun ImageReader.acquireNext(): AndroidImage? { + var image: Image? = null + return try { + image = this.acquireNextImage() + if (image == null) return null + val wrapped = AndroidImage(image) + // b/520091791: Eagerly touch the timestamp to cache it while we are still inside the safety + // of the try-catch block. + wrapped.timestamp + wrapped + } catch (e: IllegalStateException) { + // b/520091791: Suppress failures that can occur when an Image or ImageReader is + // concurrently closed while the callback is being invoked. + try { + image?.close() + } catch (_: Exception) { + // Ignore + } + null + } +} + +private fun ImageReader.closeNext() { + try { + this.acquireNextImage()?.close() + } catch (e: IllegalStateException) { + // b/520091791: Suppress failures that can occur when an Image or ImageReader is + // concurrently closed while the callback is being invoked. + } +} diff --git a/camera/camera-camera2-pipe/src/test/java/androidx/camera/camera2/pipe/media/AndroidImageReaderTest.kt b/camera/camera-camera2-pipe/src/test/java/androidx/camera/camera2/pipe/media/AndroidImageReaderTest.kt index b1686620ed511..8abf3a76e4eb3 100644 --- a/camera/camera-camera2-pipe/src/test/java/androidx/camera/camera2/pipe/media/AndroidImageReaderTest.kt +++ b/camera/camera-camera2-pipe/src/test/java/androidx/camera/camera2/pipe/media/AndroidImageReaderTest.kt @@ -16,15 +16,20 @@ package androidx.camera.camera2.pipe.media +import android.hardware.camera2.MultiResolutionImageReader +import android.hardware.camera2.params.MultiResolutionStreamInfo import android.media.Image import android.media.ImageReader import android.view.Surface import androidx.camera.camera2.pipe.OutputId +import androidx.camera.camera2.pipe.StreamFormat import androidx.camera.camera2.pipe.StreamId import com.google.common.truth.Truth.assertThat import org.junit.Test import org.junit.runner.RunWith import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.whenever import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config @@ -40,8 +45,10 @@ class AndroidImageReaderTest { val mockImageReader = mock() val mockImage = mock() val mockSurface = mock() + val timestamp = 12345L whenever(mockImageReader.acquireNextImage()).thenReturn(mockImage) whenever(mockImageReader.surface).thenReturn(mockSurface) + whenever(mockImage.timestamp).thenReturn(timestamp) val androidImageReader = AndroidImageReader( @@ -52,6 +59,48 @@ class AndroidImageReaderTest { outputId = outputId, ) + val mockExpectedOutputsListener = mock() + androidImageReader.onExpectedOutputsListener = mockExpectedOutputsListener + + val testListener = + object : ImageReaderWrapper.OnImageListener { + var imageReceived: ImageWrapper? = null + + override fun onImage(streamId: StreamId, outputId: OutputId, image: ImageWrapper) { + imageReceived = image + } + } + androidImageReader.onImageListener = testListener + + androidImageReader.onImageAvailable(mockImageReader) + + assertThat(testListener.imageReceived).isNotNull() + verify(mockExpectedOutputsListener).onExpectedOutputs(timestamp, setOf(outputId)) + } + + @Test + fun onImageAvailable_handlesClosedImage() { + val mockImageReader = mock() + val mockImage = mock() + val mockSurface = mock() + whenever(mockImageReader.acquireNextImage()).thenReturn(mockImage) + whenever(mockImageReader.surface).thenReturn(mockSurface) + + // Simulate closed image by throwing IllegalStateException on timestamp access + whenever(mockImage.timestamp).thenThrow(IllegalStateException("Image is already closed")) + + val androidImageReader = + AndroidImageReader( + imageReader = mockImageReader, + capacity = 10, + usageFlags = null, + streamId = streamId, + outputId = outputId, + ) + + val mockExpectedOutputsListener = mock() + androidImageReader.onExpectedOutputsListener = mockExpectedOutputsListener + val testListener = object : ImageReaderWrapper.OnImageListener { var imageReceived: ImageWrapper? = null @@ -64,6 +113,254 @@ class AndroidImageReaderTest { androidImageReader.onImageAvailable(mockImageReader) + assertThat(testListener.imageReceived).isNull() + verifyNoInteractions(mockExpectedOutputsListener) + verify(mockImage).close() + } + + @Test + fun onImageAvailable_handlesIllegalStateExceptionOnWrapAndClosesImageAndDoesNotFireExpectedOutputs() { + val mockImageReader = mock() + val mockImage = mock() + val mockSurface = mock() + val timestamp = 12345L + whenever(mockImageReader.acquireNextImage()).thenReturn(mockImage) + whenever(mockImageReader.surface).thenReturn(mockSurface) + whenever(mockImage.timestamp).thenReturn(timestamp) + // Simulate closed image during wrapping (e.g. format access) + whenever(mockImage.format).thenThrow(IllegalStateException("Image is already closed")) + + val androidImageReader = + AndroidImageReader( + imageReader = mockImageReader, + capacity = 10, + usageFlags = null, + streamId = streamId, + outputId = outputId, + ) + + val mockExpectedOutputsListener = mock() + androidImageReader.onExpectedOutputsListener = mockExpectedOutputsListener + + val testListener = mock() + androidImageReader.onImageListener = testListener + + androidImageReader.onImageAvailable(mockImageReader) + + verify(mockImage).close() + verifyNoInteractions(mockExpectedOutputsListener) + verifyNoInteractions(testListener) + } + + @Test + fun onImageAvailable_closesImageWhenListenerIsNull() { + val mockImageReader = mock() + val mockImage = mock() + val mockSurface = mock() + whenever(mockImageReader.acquireNextImage()).thenReturn(mockImage) + whenever(mockImageReader.surface).thenReturn(mockSurface) + + val androidImageReader = + AndroidImageReader( + imageReader = mockImageReader, + capacity = 10, + usageFlags = null, + streamId = streamId, + outputId = outputId, + ) + + androidImageReader.onImageAvailable(mockImageReader) + + verify(mockImage).close() + } + + @Test + fun onImageAvailable_handlesIllegalStateExceptionOnAcquire() { + val mockImageReader = mock() + val mockSurface = mock() + whenever(mockImageReader.surface).thenReturn(mockSurface) + whenever(mockImageReader.acquireNextImage()).thenThrow(IllegalStateException("Closed")) + + val androidImageReader = + AndroidImageReader( + imageReader = mockImageReader, + capacity = 10, + usageFlags = null, + streamId = streamId, + outputId = outputId, + ) + + val testListener = mock() + androidImageReader.onImageListener = testListener + + androidImageReader.onImageAvailable(mockImageReader) + + verifyNoInteractions(testListener) + } + + @Test(expected = RuntimeException::class) + fun onImageAvailable_doesNotCatchListenerExceptions() { + val mockImageReader = mock() + val mockImage = mock() + val mockSurface = mock() + val timestamp = 12345L + whenever(mockImageReader.acquireNextImage()).thenReturn(mockImage) + whenever(mockImageReader.surface).thenReturn(mockSurface) + whenever(mockImage.timestamp).thenReturn(timestamp) + + val androidImageReader = + AndroidImageReader( + imageReader = mockImageReader, + capacity = 10, + usageFlags = null, + streamId = streamId, + outputId = outputId, + ) + + val testListener = + object : ImageReaderWrapper.OnImageListener { + override fun onImage(streamId: StreamId, outputId: OutputId, image: ImageWrapper) { + throw RuntimeException("Listener error") + } + } + androidImageReader.onImageListener = testListener + + androidImageReader.onImageAvailable(mockImageReader) + } + + @Test + @Config(minSdk = 31) + fun multiResolutionOnImageAvailable_handlesClosedImage() { + val mockMultiReader = mock() + val mockImageReader = mock() + val mockImage = mock() + + whenever(mockImageReader.acquireNextImage()).thenReturn(mockImage) + + val mockStreamInfo = mock() + whenever(mockMultiReader.getStreamInfoForImageReader(mockImageReader)) + .thenReturn(mockStreamInfo) + + val streamInfoMap = mapOf(mockStreamInfo to outputId) + + // Simulate closed image by throwing IllegalStateException on timestamp access + whenever(mockImage.timestamp).thenThrow(IllegalStateException("Image is already closed")) + + val multiImageReader = + AndroidMultiResolutionImageReader( + multiResolutionImageReader = mockMultiReader, + streamFormat = StreamFormat(1), + capacity = 10, + usageFlags = null, + streamId = streamId, + outputConfigurations = emptyList(), + streamInfoToOutputIdMap = streamInfoMap, + surfaceToOutputIdMap = emptyMap(), + concurrentOutputsEnabled = false, + ) + + val mockExpectedOutputsListener = mock() + multiImageReader.onExpectedOutputsListener = mockExpectedOutputsListener + + val testListener = + object : ImageReaderWrapper.OnImageListener { + var imageReceived: ImageWrapper? = null + + override fun onImage(streamId: StreamId, outputId: OutputId, image: ImageWrapper) { + imageReceived = image + } + } + multiImageReader.onImageListener = testListener + + multiImageReader.onImageAvailable(mockImageReader) + + assertThat(testListener.imageReceived).isNull() + verifyNoInteractions(mockExpectedOutputsListener) + verify(mockImage).close() + } + + @Test + @Config(minSdk = 31) + fun multiResolutionOnImageAvailable_firesExpectedOutputs_onSuccess() { + val mockMultiReader = mock() + val mockImageReader = mock() + val mockImage = mock() + val timestamp = 12345L + + whenever(mockImageReader.acquireNextImage()).thenReturn(mockImage) + whenever(mockImage.timestamp).thenReturn(timestamp) + + val mockStreamInfo = mock() + whenever(mockMultiReader.getStreamInfoForImageReader(mockImageReader)) + .thenReturn(mockStreamInfo) + + val streamInfoMap = mapOf(mockStreamInfo to outputId) + + val multiImageReader = + AndroidMultiResolutionImageReader( + multiResolutionImageReader = mockMultiReader, + streamFormat = StreamFormat(1), + capacity = 10, + usageFlags = null, + streamId = streamId, + outputConfigurations = emptyList(), + streamInfoToOutputIdMap = streamInfoMap, + surfaceToOutputIdMap = emptyMap(), + concurrentOutputsEnabled = false, + ) + + val mockExpectedOutputsListener = mock() + multiImageReader.onExpectedOutputsListener = mockExpectedOutputsListener + + val testListener = + object : ImageReaderWrapper.OnImageListener { + var imageReceived: ImageWrapper? = null + + override fun onImage(streamId: StreamId, outputId: OutputId, image: ImageWrapper) { + imageReceived = image + } + } + multiImageReader.onImageListener = testListener + + multiImageReader.onImageAvailable(mockImageReader) + assertThat(testListener.imageReceived).isNotNull() + verify(mockExpectedOutputsListener).onExpectedOutputs(timestamp, setOf(outputId)) + } + + @Test(expected = IllegalStateException::class) + @Config(minSdk = 31) + fun multiResolutionOnImageAvailable_throws_ifMappingMissing() { + val mockMultiReader = mock() + val mockImageReader = mock() + val mockImage = mock() + val timestamp = 12345L + + whenever(mockImageReader.acquireNextImage()).thenReturn(mockImage) + whenever(mockImage.timestamp).thenReturn(timestamp) + + val mockStreamInfo = mock() + whenever(mockMultiReader.getStreamInfoForImageReader(mockImageReader)) + .thenReturn(mockStreamInfo) + + val streamInfoMap = emptyMap() + + val multiImageReader = + AndroidMultiResolutionImageReader( + multiResolutionImageReader = mockMultiReader, + streamFormat = StreamFormat(1), + capacity = 10, + usageFlags = null, + streamId = streamId, + outputConfigurations = emptyList(), + streamInfoToOutputIdMap = streamInfoMap, + surfaceToOutputIdMap = emptyMap(), + concurrentOutputsEnabled = false, + ) + + val testListener = mock() + multiImageReader.onImageListener = testListener + + multiImageReader.onImageAvailable(mockImageReader) } }