[libcamera-devel] [PATCH 1/4] libcamera: camera_sensor: Transform CameraSensor::sizes()

[Umang Jain]

Hi Jacopo

On 8/9/21 9:19 PM, Jacopo Mondi wrote:
> Hi Umang,
>
> On Tue, Aug 03, 2021 at 07:02:02PM +0530, Umang Jain wrote:
>> In CameraSensor, the mbusCodes() and sizes() accessor functions
>> retrieves all the supported media bus codes and the supported sizes
>> respectively. However, this is quite limiting since the caller
>> probably isn't in a position to match which range of sizes are
>> supported for a particular mbusCode.
>>
>> Hence, the caller is most likely interested to know about the sizes
>> supported for a particular media bus code. This patch transforms the
>> existing CameraSensor::sizes() to CameraSensor::sizes(mbuscode) to
>> achieve that goal.
>>
>> To know all the frame sizes of the CameraSensor as required in IPU3
>> case Cio2Device::sizes(), one would now require to enumerate all the
>> media bus codes (can be retrieved by CameraSensor::mbusCodes()) with
>> CameraSensor::size(mbusCode). The result can be inserted in a
>> std::set<> to avoid duplicates.
>>
>> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
>> ---
>>   include/libcamera/internal/camera_sensor.h |  2 +-
>>   src/libcamera/camera_sensor.cpp            | 36 ++++++++++++++++------
>>   src/libcamera/pipeline/ipu3/cio2.cpp       | 10 +++---
>>   test/camera-sensor.cpp                     |  2 +-
>>   4 files changed, 34 insertions(+), 16 deletions(-)
>>
<snip>

>> +			allSizes.insert(sz);
>> +	}
>>
>> -	return sizes;
>> +	return std::vector<SizeRange>(allSizes.begin(), allSizes.end());
> Looking at how CIO2::sizes() is used in IPU3::generateConfiguration()
> I wonder if we shouldn't pass the desired PixelFormat to

I think you meant s/shouldn't/should here? So essentially do you want

    std::vector<SizeRange> CIO2Device::sizes(const PixelFormat &format)
    const

    ?

(which looks okay to me)

> CIO2::sizes() so that we can actually enumerate the per-format sizes
> for real (and avoid going through the set->vector conversion).
>
>>   }
>>
>>   /**
>> diff --git a/test/camera-sensor.cpp b/test/camera-sensor.cpp
>> index a8dcad82..372ee4af 100644
>> --- a/test/camera-sensor.cpp
>> +++ b/test/camera-sensor.cpp
>> @@ -76,7 +76,7 @@ protected:
>>   			return TestFail;
>>   		}
>>
>> -		const std::vector<Size> &sizes = sensor_->sizes();
>> +		const std::vector<Size> &sizes = sensor_->sizes(*iter);
> This changes the semantic of the test from "let's check if a format
> supports 4096x2160" to "let's check if ARGB8888_1X32 supports
> 4096x2160". I think it's better now, and as long as the test passes,
> since it uses VIMC, we should be good!
I'll check!
>
> Thanks
>     j
>
>>   		auto iter2 = std::find(sizes.begin(), sizes.end(),
>>   				       Size(4096, 2160));
>>   		if (iter2 == sizes.end()) {
>> --
>> 2.31.0
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210810/9bba300a/attachment.htm>