[libcamera-devel] [PATCH 07/10] android: camera_device: Handle PIPELINE_MAX_DEPTH

[Kieran Bingham]

Hi Jacopo,

On 09/10/2020 14:19, Jacopo Mondi wrote:
> Hi Kieran,
> 
> On Fri, Oct 09, 2020 at 01:44:19PM +0100, Kieran Bingham wrote:
>> Hi Jacopo,
>>
>> On 09/10/2020 13:20, Jacopo Mondi wrote:
>>> Register the ANDROID_REQUEST_PIPELINE_MAX_DEPTH static property
>>> inspecting what the value returned by the pipeline handler
>>> through the properties::draft::PipelineMaxDepth libcamera property.
>>>
>>> Report the result metadata ANDROID_REQUEST_PIPELINE_DEPTH that
>>> reports the processing stage a frame went through by inspecting the
>>> controls::draft::PipelineDepth libcamera control reported in the
>>> completed Request.
>>>
>>> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
>>> ---
>>>  src/android/camera_device.cpp | 28 ++++++++++++++++++++++------
>>>  src/android/camera_device.h   |  3 ++-
>>>  2 files changed, 24 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>>> index 0a94c1ae17ac..bccec358ea13 100644
>>> --- a/src/android/camera_device.cpp
>>> +++ b/src/android/camera_device.cpp
>>> @@ -12,6 +12,7 @@
>>>  #include <tuple>
>>>  #include <vector>
>>>
>>> +#include <libcamera/control_ids.h>
>>>  #include <libcamera/controls.h>
>>>  #include <libcamera/formats.h>
>>>  #include <libcamera/property_ids.h>
>>> @@ -576,6 +577,8 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>>>  		return nullptr;
>>>  	}
>>>
>>> +	const ControlList &cameraProperties = camera_->properties();
>>> +
>>>  	/* Color correction static metadata. */
>>>  	std::vector<uint8_t> aberrationModes = {
>>>  		ANDROID_COLOR_CORRECTION_ABERRATION_MODE_OFF,
>>> @@ -867,9 +870,12 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>>>  	staticMetadata_->addEntry(ANDROID_REQUEST_PARTIAL_RESULT_COUNT,
>>>  				  &partialResultCount, 1);
>>>
>>> -	uint8_t maxPipelineDepth = 2;
>>> -	staticMetadata_->addEntry(ANDROID_REQUEST_PIPELINE_MAX_DEPTH,
>>> -				  &maxPipelineDepth, 1);
>>> +	if (cameraProperties.contains(properties::draft::PipelineMaxDepth)) {
>>> +		uint8_t maxPipelineDepth =
>>> +			cameraProperties.get<int32_t>(properties::draft::PipelineMaxDepth);
>>> +		staticMetadata_->addEntry(ANDROID_REQUEST_PIPELINE_MAX_DEPTH,
>>> +					  &maxPipelineDepth, 1);
>>> +	}
>>
>> I assume if the pipeline handler doesn't specify a max depth, and one
>> doesn't get added here, that's not a problem for android.
> 
> That's the right question to ask!
> 
> By points:
> 1) Is this is problem for Android: it depends, Do you want to be
> compliant with the Android requirements for the reported Android HW
> level ? Yes, that might be a problem, depending on which
> property/control the pipeline handler does not report.
> 
> 2) What policy do we want ? It boils down to the concept of
> "android-aware" pipeline handler. The Android HAL cannot, in my
> opinion, be fully Android-compliant without support in the pipeline
> handler. It cannot produce 'default' values for properties, controls
> and metadata without breaking compliance with CTS and other tests.
> The pipeline handler needs to report and handle the values required
> by Android, I don't see many ways around it :)

To me, I see that we would handle this with the
 "Pipeline Validation Tool" *1

We can simply add tests there for any android specific requirements.

then a pipeline can be 'passed' as android capabable if it implements
and passes those tests, and - simply not otherwise.


*1 - ah yes the fabled tool that we really should start ...
- Lets discuss next week, and see if we can get at least a small one
started!, then it's easier to continue if it exists ;-)

--
Kieran


> 
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>
>>>
>>>  	/* LIMITED does not support reprocessing. */
>>>  	uint32_t maxNumInputStreams = 0;
>>> @@ -1480,7 +1486,8 @@ void CameraDevice::requestComplete(Request *request)
>>>  	 * pipeline handlers) timestamp in the Request itself.
>>>  	 */
>>>  	FrameBuffer *buffer = buffers.begin()->second;
>>> -	resultMetadata = getResultMetadata(descriptor->frameNumber,
>>> +	resultMetadata = getResultMetadata(request->metadata(),
>>> +					   descriptor->frameNumber,
>>>  					   buffer->metadata().timestamp);
>>>
>>>  	/* Handle any JPEG compression. */
>>> @@ -1600,7 +1607,8 @@ void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream)
>>>   * Produce a set of fixed result metadata.
>>>   */
>>>  std::unique_ptr<CameraMetadata>
>>> -CameraDevice::getResultMetadata([[maybe_unused]] int frame_number,
>>> +CameraDevice::getResultMetadata(ControlList &metadata,
>>> +				[[maybe_unused]] int frame_number,
>>>  				int64_t timestamp)
>>>  {
>>>  	/*
>>> @@ -1608,7 +1616,7 @@ CameraDevice::getResultMetadata([[maybe_unused]] int frame_number,
>>>  	 * Currently: 18 entries, 62 bytes
>>>  	 */
>>>  	std::unique_ptr<CameraMetadata> resultMetadata =
>>> -		std::make_unique<CameraMetadata>(18, 62);
>>> +		std::make_unique<CameraMetadata>(19, 63);
>>>  	if (!resultMetadata->isValid()) {
>>>  		LOG(HAL, Error) << "Failed to allocate static metadata";
>>>  		return nullptr;
>>> @@ -1658,6 +1666,14 @@ CameraDevice::getResultMetadata([[maybe_unused]] int frame_number,
>>>  	resultMetadata->addEntry(ANDROID_STATISTICS_SCENE_FLICKER,
>>>  				 &scene_flicker, 1);
>>>
>>> +	/* Add metadata tags reported by libcamera. */
>>> +	if (metadata.contains(controls::draft::PipelineDepth)) {
>>> +		uint8_t pipeline_depth =
>>> +			metadata.get<int32_t>(controls::draft::PipelineDepth);
>>> +		resultMetadata->addEntry(ANDROID_REQUEST_PIPELINE_DEPTH,
>>> +					 &pipeline_depth, 1);
>>> +	}
>>> +
>>>  	/*
>>>  	 * Return the result metadata pack even is not valid: get() will return
>>>  	 * nullptr.
>>> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
>>> index 777d1a35e801..8cf1006c0840 100644
>>> --- a/src/android/camera_device.h
>>> +++ b/src/android/camera_device.h
>>> @@ -102,7 +102,8 @@ private:
>>>  	void notifyError(uint32_t frameNumber, camera3_stream_t *stream);
>>>  	CameraMetadata *requestTemplatePreview();
>>>  	libcamera::PixelFormat toPixelFormat(int format);
>>> -	std::unique_ptr<CameraMetadata> getResultMetadata(int frame_number,
>>> +	std::unique_ptr<CameraMetadata> getResultMetadata(libcamera::ControlList &metadata,
>>> +							  int frame_number,
>>>  							  int64_t timestamp);
>>>
>>>  	unsigned int id_;
>>>
>>
>> --
>> Regards
>> --
>> Kieran

-- 
Regards
--
Kieran