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) } }