[libcamera-devel] [PATCH 2/4] android: camera_device: Use HAL_PIXEL_FORMAT_* defines for formats

[Niklas Söderlund]

Hi Jacopo,

Thanks for your feedback.

On 2020-07-24 10:43:23 +0200, Jacopo Mondi wrote:
> Hi Niklas,
> 
> On Thu, Jul 23, 2020 at 07:39:40PM +0200, Niklas Söderlund wrote:
> > The documentation says the HAL_PIXEL_FORMAT_* defines shall be used for
> > formats instead of ANDROID_SCALER_AVAILABLE_FORMATS_* for the
> > ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS and
> > ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS tags.
> >
> > This have worked in the past as the numerical value of the two sets are
> > the same for the formats supported.
> 
> I wonder if in the future we would need scaler formats for other
> properties, but as the numerical values are the same, we could use the
> HAL_PIXEL_FORMAT_* definitions.
> 
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Was this comment and tag indented for 3/4 ?

> 
> Thanks
>   j
> 
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> >  src/android/camera_device.cpp | 8 ++++----
> >  src/android/camera_device.h   | 2 +-
> >  2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 6e7673583f6a8f85..96dd8d5a99082966 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -317,14 +317,14 @@ int CameraDevice::initializeStreamConfigurations()
> >  			    status != CameraConfiguration::Valid)
> >  				continue;
> >
> > -			streamConfigurations_.push_back({ res, camera3Format.scalerFormat });
> > +			streamConfigurations_.push_back({ res, androidFormat });
> >  		}
> >  	}
> >
> >  	LOG(HAL, Debug) << "Collected stream configuration map: ";
> >  	for (const auto &entry : streamConfigurations_)
> >  		LOG(HAL, Debug) << "{ " << entry.resolution.toString() << " - "
> > -				<< utils::hex(entry.androidScalerCode) << " }";
> > +				<< utils::hex(entry.androidFormat) << " }";
> >
> >  	return 0;
> >  }
> > @@ -658,7 +658,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >  	std::vector<uint32_t> availableStreamConfigurations;
> >  	availableStreamConfigurations.reserve(streamConfigurations_.size() * 4);
> >  	for (const auto &entry : streamConfigurations_) {
> > -		availableStreamConfigurations.push_back(entry.androidScalerCode);
> > +		availableStreamConfigurations.push_back(entry.androidFormat);
> >  		availableStreamConfigurations.push_back(entry.resolution.width);
> >  		availableStreamConfigurations.push_back(entry.resolution.height);
> >  		availableStreamConfigurations.push_back(
> > @@ -679,7 +679,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >  	std::vector<int64_t> minFrameDurations;
> >  	minFrameDurations.reserve(streamConfigurations_.size() * 4);
> >  	for (const auto &entry : streamConfigurations_) {
> > -		minFrameDurations.push_back(entry.androidScalerCode);
> > +		minFrameDurations.push_back(entry.androidFormat);
> >  		minFrameDurations.push_back(entry.resolution.width);
> >  		minFrameDurations.push_back(entry.resolution.height);
> >  		minFrameDurations.push_back(33333333);
> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> > index 5b8b9c3e26e2871e..00472c21938871a1 100644
> > --- a/src/android/camera_device.h
> > +++ b/src/android/camera_device.h
> > @@ -75,7 +75,7 @@ private:
> >
> >  	struct Camera3StreamConfiguration {
> >  		libcamera::Size resolution;
> > -		int androidScalerCode;
> > +		int androidFormat;
> >  	};
> >
> >  	int initializeStreamConfigurations();
> > --
> > 2.27.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund