[libcamera-devel] [PATCH 5/5] android: camera_device: List RAW resolutions

[Kieran Bingham]

On 02/09/2020 14:36, Jacopo Mondi wrote:
> Hi Kieran,
> 
> On Wed, Sep 02, 2020 at 02:15:44PM +0100, Kieran Bingham wrote:
>> Hi Jacopo,
>>
>> On 02/09/2020 11:47, Jacopo Mondi wrote:
>>> The resolutions supported for the RAW formats cannot be tested from
>>> a list of known sizes like the processed ones. This is mainly due to the
>>> fact RAW streams are produced by capturing frames at the CSI-2 receiver
>>> output and their size corresponds to the sensor's native sizes.
>>>
>>> In order to obtain the RAW frame size generate a temporary
>>> CameraConfiguration for the Role::StillCaptureRAW role and inspect the
>>> map of StreamFormats returned by the pipeline handler.
>>
>> Presumably, it will also be heavily dependant upon the actual chosen
>> streams from the other requested streams ... and only a subset might
>> really be available when a full configuration is used?
> 
> Not sure I got you here :/

I was worried about the fact that the pipeline handlers would configure
the sensor in a specific format to handle the other streams, which might
not match what android requests as the raw size ... but I don't think it
would happen.

Asking for a RAW stream should always be the 'size' of the main image I
expect... (or the 'rawest' form thereof...)

>>
>>
>>
>>>
>>> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
>>> ---
>>>  src/android/camera_device.cpp | 21 +++++++++++++++++----
>>>  src/android/camera_device.h   |  2 ++
>>>  2 files changed, 19 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>>> index 765c3292e4f3..181fca83988d 100644
>>> --- a/src/android/camera_device.cpp
>>> +++ b/src/android/camera_device.cpp
>>> @@ -314,6 +314,17 @@ std::vector<Size> CameraDevice::listProcessedResolutions(CameraConfiguration *ca
>>>  	return supportedResolutions;
>>>  }
>>>
>>> +std::vector<Size> CameraDevice::listRawResolutions(const libcamera::PixelFormat &pixelFormat)
>>> +{
>>
>> same comment on the function name, that we're filtering (rather than
>> 'listing' valid resolutions. Though maybe this one is subtly
>> different... but I'd keep both namings consistent either way.
> 
> Well, we're actually listing them here :D
> And anyway, there's not much filtering going on here, maybe a bit more
> in the corresponding non-RAW implementation as we actually test a list
> of sizes and report only the supported ones.
> 
> In any case, I would keep the naming of the two functions consistent
> whatever we chose.
> 
>>
>>> +	std::unique_ptr<CameraConfiguration> cameraConfig =
>>> +		camera_->generateConfiguration({ StillCaptureRaw });
>>> +	StreamConfiguration &cfg = cameraConfig->at(0);
>>> +	const StreamFormats &formats = cfg.formats();
>>> +	std::vector<Size> supportedResolutions = formats.sizes(pixelFormat);
>>
>> I'm a bit weary of this.
>>
>> If you apply this function to a UVC camera for example, it would simply
>> return a list of sizes which represent the available frame sizes...
>>
> 
> Pardon my ignorance, does uvc support RAW ?

probably not...


>> Scratch that, I see it's filtered by pixelFormat ... ok - so I guess
>> that works.

Sorry - I left my working/thoughts in place. That sentence voided the
previous one.


>>
>>
>> I wonder if this couldn't also be handled by the other filter fucntion
>> but I assume not easily, as you've done this ;-)
> 
> Well, they do two different things. I actually considered doing what
> I'm doing here (building on top of StreamFormats) for non-RAW stream,
> but it turned out to be a whack-a-mole game of finding the right Role
> to request to have the sizes for the format we are interested in. And
> that gets pretty pipeline-implementation-dependent as there's no
> strict rule on what formats corresponds to a role. So I might be
> looking for supported NV12 sizes and my best bet is to ask for
> Viewfinder and see what StreamFormats are returned, but it's a bet and
> some pipeline handler might only support RGB for viewfinder and NV12
> for StillCapture (it's an example, not sure it makes any sense) so I
> could end up testing all roles, filtering double results, re-ordering
> etc... Seems like a no-go to me.
> 
> The other way around is possible as well: list RAW sizes by using the
> same method as it's used for non-RAW ones, with the difference that we
> accept adjusted sizes, as the pipeline handler will adjust to the
> closest available sensor frame size for each tested image resolution.
> Again, it seemd sub-optimal and requires filtering doubles and testing
> an un-necessary number of times, so I created two separate functions
> in the end.
> 
> Be aware that in the back of my mind there's also a listJPEGSizes()
> that queries the encoder for the reasons explained in my other reply,
> if that makes any sense :)


Well, we'll see what happens when we get our next encoder ...


> Thanks
>   j
> 
> 
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>
>>
>>> +
>>> +	return supportedResolutions;
>>> +}
>>> +
>>>  /*
>>>   * Initialize the format conversion map to translate from Android format
>>>   * identifier to libcamera pixel formats and fill in the list of supported
>>> @@ -466,13 +477,15 @@ int CameraDevice::initializeStreamConfigurations()
>>>  				<< camera3Format.name << " to: "
>>>  				<< mappedFormat.toString();
>>>
>>> +		std::vector<Size> resolutions;
>>>  		const PixelFormatInfo &info = PixelFormatInfo::info(mappedFormat);
>>>  		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
>>> -			continue;
>>> +			resolutions = listRawResolutions(mappedFormat);
>>> +		else
>>> +			resolutions = listProcessedResolutions(cameraConfig.get(),
>>> +							       mappedFormat,
>>> +							       cameraResolutions);
>>>
>>> -		std::vector<Size> resolutions = listProcessedResolutions(cameraConfig.get(),
>>> -									 mappedFormat,
>>> -									 cameraResolutions);
>>>  		for (const Size &res : resolutions) {
>>>  			streamConfigurations_.push_back({ res, androidFormat });
>>>
>>> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
>>> index 18fd5ff03cde..230e89b011e6 100644
>>> --- a/src/android/camera_device.h
>>> +++ b/src/android/camera_device.h
>>> @@ -97,6 +97,8 @@ private:
>>>  	listProcessedResolutions(libcamera::CameraConfiguration *cameraConfig,
>>>  				 const libcamera::PixelFormat &pixelFormat,
>>>  				 const std::vector<libcamera::Size> &resolutions);
>>> +	std::vector<libcamera::Size>
>>> +	listRawResolutions(const libcamera::PixelFormat &pixelFormat);
>>>
>>>  	std::tuple<uint32_t, uint32_t> calculateStaticMetadataSize();
>>>  	libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);
>>>
>>
>> --
>> Regards
>> --
>> Kieran

-- 
Regards
--
Kieran