From 22f6dcec7e81c1a747b516377d2e7fc8cb06c508 Mon Sep 17 00:00:00 2001 From: Shuzhen Wang Date: Mon, 20 May 2019 11:04:29 -0700 Subject: [PATCH] Camera: Use original dataspace/format for 3.5 device Starting from CameraDeviceSession 3.5, for IMPLEMENTATION_DEFINED pixel format, configureStreams call uses original format and dataspace instead of the overridden value. This makes sure the HAL interface behavior is consistent between first and subsequent processCaptureRequest() calls. Test: Camera CTS and partner testing Bug: 131864007 Change-Id: Id701141d2c11089ef063fd3f32444212855f84ab --- .../3.4/default/CameraDeviceSession.cpp | 29 ++++++++++++++----- .../device_v3_4_impl/CameraDeviceSession.h | 4 +-- camera/device/3.5/ICameraDeviceSession.hal | 6 ++++ .../3.5/default/CameraDeviceSession.cpp | 2 +- current.txt | 1 + 5 files changed, 31 insertions(+), 11 deletions(-) diff --git a/camera/device/3.4/default/CameraDeviceSession.cpp b/camera/device/3.4/default/CameraDeviceSession.cpp index 03b6050397..b4ebe22701 100644 --- a/camera/device/3.4/default/CameraDeviceSession.cpp +++ b/camera/device/3.4/default/CameraDeviceSession.cpp @@ -75,7 +75,7 @@ Return CameraDeviceSession::configureStreams_3_4( void CameraDeviceSession::configureStreams_3_4_Impl( const StreamConfiguration& requestedConfiguration, ICameraDeviceSession::configureStreams_3_4_cb _hidl_cb, - uint32_t streamConfigCounter) { + uint32_t streamConfigCounter, bool useOverriddenFields) { Status status = initStatus(); HalStreamConfiguration outStreams; @@ -133,7 +133,8 @@ void CameraDeviceSession::configureStreams_3_4_Impl( mStreamConfigCounter = streamConfigCounter; hidl_vec streams; stream_list.session_parameters = paramBuffer; - if (!preProcessConfigurationLocked_3_4(requestedConfiguration, &stream_list, &streams)) { + if (!preProcessConfigurationLocked_3_4(requestedConfiguration, + useOverriddenFields, &stream_list, &streams)) { _hidl_cb(Status::INTERNAL_ERROR, outStreams); return; } @@ -164,7 +165,7 @@ void CameraDeviceSession::configureStreams_3_4_Impl( } bool CameraDeviceSession::preProcessConfigurationLocked_3_4( - const StreamConfiguration& requestedConfiguration, + const StreamConfiguration& requestedConfiguration, bool useOverriddenFields, camera3_stream_configuration_t *stream_list /*out*/, hidl_vec *streams /*out*/) { @@ -189,19 +190,31 @@ bool CameraDeviceSession::preProcessConfigurationLocked_3_4( mStreamMap[id].data_space); mCirculatingBuffers.emplace(stream.mId, CirculatingBuffers{}); } else { - // width/height/format must not change, but usage/rotation might need to change + // width/height/format must not change, but usage/rotation might need to change. + // format and data_space may change. if (mStreamMap[id].stream_type != (int) requestedConfiguration.streams[i].v3_2.streamType || mStreamMap[id].width != requestedConfiguration.streams[i].v3_2.width || mStreamMap[id].height != requestedConfiguration.streams[i].v3_2.height || - mStreamMap[id].format != (int) requestedConfiguration.streams[i].v3_2.format || - mStreamMap[id].data_space != - mapToLegacyDataspace( static_cast ( - requestedConfiguration.streams[i].v3_2.dataSpace)) || mPhysicalCameraIdMap[id] != requestedConfiguration.streams[i].physicalCameraId) { ALOGE("%s: stream %d configuration changed!", __FUNCTION__, id); return false; } + if (useOverriddenFields) { + android_dataspace_t requestedDataSpace = + mapToLegacyDataspace(static_cast( + requestedConfiguration.streams[i].v3_2.dataSpace)); + if (mStreamMap[id].format != (int) requestedConfiguration.streams[i].v3_2.format || + mStreamMap[id].data_space != requestedDataSpace) { + ALOGE("%s: stream %d configuration changed!", __FUNCTION__, id); + return false; + } + } else { + mStreamMap[id].format = + (int) requestedConfiguration.streams[i].v3_2.format; + mStreamMap[id].data_space = (android_dataspace_t) + requestedConfiguration.streams[i].v3_2.dataSpace; + } mStreamMap[id].rotation = (int) requestedConfiguration.streams[i].v3_2.rotation; mStreamMap[id].usage = (uint32_t) requestedConfiguration.streams[i].v3_2.usage; } diff --git a/camera/device/3.4/default/include/device_v3_4_impl/CameraDeviceSession.h b/camera/device/3.4/default/include/device_v3_4_impl/CameraDeviceSession.h index 1db7b41dec..280c4bec46 100644 --- a/camera/device/3.4/default/include/device_v3_4_impl/CameraDeviceSession.h +++ b/camera/device/3.4/default/include/device_v3_4_impl/CameraDeviceSession.h @@ -80,7 +80,7 @@ protected: ICameraDeviceSession::configureStreams_3_4_cb _hidl_cb); bool preProcessConfigurationLocked_3_4( - const StreamConfiguration& requestedConfiguration, + const StreamConfiguration& requestedConfiguration, bool useOverriddenFields, camera3_stream_configuration_t *stream_list /*out*/, hidl_vec *streams /*out*/); void postProcessConfigurationLocked_3_4(const StreamConfiguration& requestedConfiguration); @@ -91,7 +91,7 @@ protected: const StreamConfiguration& requestedConfiguration, ICameraDeviceSession::configureStreams_3_4_cb _hidl_cb, // Optional argument for ICameraDeviceSession@3.5 impl - uint32_t streamConfigCounter = 0); + uint32_t streamConfigCounter = 0, bool useOverriddenFields = true); Return processCaptureRequest_3_4( const hidl_vec& requests, diff --git a/camera/device/3.5/ICameraDeviceSession.hal b/camera/device/3.5/ICameraDeviceSession.hal index d0cfe392ae..c868e1e96c 100644 --- a/camera/device/3.5/ICameraDeviceSession.hal +++ b/camera/device/3.5/ICameraDeviceSession.hal @@ -36,6 +36,12 @@ interface ICameraDeviceSession extends @3.4::ICameraDeviceSession { * * - a streamConfigCounter counter is provided to check for race condition * between configureStreams_3_5 and signalStreamFlush call. + * - In case the HAL overrides dataspace or format for + * IMPLEMENTATION_DEFINED pixel format, camera framework must use original + * dataspace and format in subsequent configureStreams_3_5 calls for the same + * stream. HAL is allowed to change the overriding behavior of format or + * dataspace for reconfiguration of the same stream depending on the stream + * combination. * * @return status Status code for the operation, one of: * OK: diff --git a/camera/device/3.5/default/CameraDeviceSession.cpp b/camera/device/3.5/default/CameraDeviceSession.cpp index e812e50f5f..44d067d475 100644 --- a/camera/device/3.5/default/CameraDeviceSession.cpp +++ b/camera/device/3.5/default/CameraDeviceSession.cpp @@ -66,7 +66,7 @@ Return CameraDeviceSession::configureStreams_3_5( const StreamConfiguration& requestedConfiguration, ICameraDeviceSession::configureStreams_3_5_cb _hidl_cb) { configureStreams_3_4_Impl(requestedConfiguration.v3_4, _hidl_cb, - requestedConfiguration.streamConfigCounter); + requestedConfiguration.streamConfigCounter, false /*useOverriddenFields*/); return Void(); } diff --git a/current.txt b/current.txt index 5742d667d0..6510134204 100644 --- a/current.txt +++ b/current.txt @@ -455,6 +455,7 @@ f7431f3e3e4e3387fc6f27a6cf423eddcd824a395dc4349d302c995ab44a9895 android.hardwar 09ab9b24994429d9bb32a3fb420b6f6be3e47eb655139a2c08c4e80d3f33ff95 android.hardware.camera.device@3.5::ICameraDevice 06237de53c42890029e3f8fe7d1480d078469c0d07608e51c37b4d485d342992 android.hardware.camera.device@3.5::ICameraDeviceCallback 08c68b196e2fc4e5ba67ba0d0917bde828a87cbe2cffec19d04733972da9eb49 android.hardware.camera.device@3.5::ICameraDeviceSession +feabf0b7caa947757bf74375aceb4919a5aa99dd6a36216843553b6adec7eb5d android.hardware.camera.device@3.5::ICameraDeviceSession # b/131864007 f9b8b388c0c76669e4b9189e4943efd2982f9bda5c10e276f96cc91bc8e818d6 android.hardware.camera.device@3.5::types f727d5f350f55a6d3354aad2feb64e43200de77c10d9d642465566bc260bb8ec android.hardware.camera.metadata@3.4::types 0fb39a7809ad1c52b3efbbed5ef4749b06c2a4f1f19cdc3efa2e3d9b28f1205c android.hardware.camera.provider@2.5::ICameraProvider