[libcamera-devel] [PATCH v5 7/7] qcam: dng_writer: Record camera model

[Umang Jain]

Hi Niklas,

Thank you for this work.

On 9/25/20 8:37 PM, Niklas Söderlund wrote:
> Record the model property of the camera if available.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>   src/qcam/dng_writer.cpp | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp
> index 030d1387118d91e9..f6d004d0407f8641 100644
> --- a/src/qcam/dng_writer.cpp
> +++ b/src/qcam/dng_writer.cpp
> @@ -15,6 +15,7 @@
>   
>   #include <libcamera/control_ids.h>
>   #include <libcamera/formats.h>
> +#include <libcamera/property_ids.h>
>   
>   using namespace libcamera;
>   
> @@ -353,6 +354,8 @@ int DNGWriter::write(const char *filename, const Camera *camera,
>   		     [[maybe_unused]] const FrameBuffer *buffer,
>   		     const void *data)
>   {
> +	const ControlList &cameraProperties = camera->properties();
> +
>   	const auto it = formatInfo.find(config.pixelFormat);
>   	if (it == formatInfo.cend()) {
>   		std::cerr << "Unsupported pixel format" << std::endl;
> @@ -387,9 +390,13 @@ int DNGWriter::write(const char *filename, const Camera *camera,
>   	TIFFSetField(tif, TIFFTAG_DNGBACKWARDVERSION, version);
>   	TIFFSetField(tif, TIFFTAG_FILLORDER, FILLORDER_MSB2LSB);
>   	TIFFSetField(tif, TIFFTAG_MAKE, "libcamera");
> -	/* \todo Report a real model string instead of id. */
> -	TIFFSetField(tif, TIFFTAG_MODEL, camera->id().c_str());
> -	TIFFSetField(tif, TIFFTAG_UNIQUECAMERAMODEL, camera->id().c_str());
> +
> +	if (cameraProperties.contains(properties::Model)) {
> +		std::string model = cameraProperties.get(properties::Model);
> +		TIFFSetField(tif, TIFFTAG_MODEL, model.c_str());
> +		TIFFSetField(tif, TIFFTAG_UNIQUECAMERAMODEL, model.c_str());
Looking at TIFFTAG_UNIQUECAMERAMODEL description in the spec, it states 
that:

 > UniqueCameraModel defines a unique, non-localized name for the camera 
model that created the image in the raw file. This name should include 
the manufacturer's name to avoid conflicts ...
E.g. mentions - "Canon EOS 300D" , "Nikon DIX" , "Sony F828" etc.

So, I think we can skip setting the UNIQUECAMERAMODEL for now if we 
can't get the mfg.'s name (I have similiar issue for setting related 
EXIF tag "setMake()" in src/android/cameraDevice.cpp).  OR atleast 
introduce a \todo to follow up on the work.

Apart for that:
Reviewed-by: Umang Jain <email at uajain.com>
> +	}
> +
>   	TIFFSetField(tif, TIFFTAG_SOFTWARE, "qcam");
>   	TIFFSetField(tif, TIFFTAG_ORIENTATION, ORIENTATION_TOPLEFT);
>