[1/2] apps: Replace HAVE_DNG with HAVE_TIFF

21370 diff mbox series

Support for DNG capture is conditioned by the availability of libtiff,
which is indicated by the HAVE_TIFF macro set by meson. The dng_writer.h
header then defines HAVE_DNG, which is used is a couple of places to
conditionally compile DNG-related code. Most of the other locations
where conditional compilation is required use HAVE_TIFF.

Using both HAVE_TIFF and HAVE_DNG is confusing. HAVE_DNG would be a
better name, but as the macro is defined in dng_writer.h, it would
require all files that need to test for DNG support to include that
header. Failure to include it (directly or indirectly) would result in
the code covered by the macro to be silently disabled.

To avoid the confusion, standardize on using HAVE_TIFF everywhere and
drop HAVE_DNG.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/apps/common/dng_writer.h  | 1 -
 src/apps/qcam/main_window.cpp | 4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:

> Support for DNG capture is conditioned by the availability of libtiff,
> which is indicated by the HAVE_TIFF macro set by meson. The dng_writer.h
> header then defines HAVE_DNG, which is used is a couple of places to

... *in* a couple ...

> conditionally compile DNG-related code. Most of the other locations
> where conditional compilation is required use HAVE_TIFF.
>
> Using both HAVE_TIFF and HAVE_DNG is confusing. HAVE_DNG would be a
> better name, but as the macro is defined in dng_writer.h, it would
> require all files that need to test for DNG support to include that
> header. Failure to include it (directly or indirectly) would result in
> the code covered by the macro to be silently disabled.
>
> To avoid the confusion, standardize on using HAVE_TIFF everywhere and
> drop HAVE_DNG.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Milan Zamazal <mzamazal@redhat.com>

> ---
>  src/apps/common/dng_writer.h  | 1 -
>  src/apps/qcam/main_window.cpp | 4 ++--
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/src/apps/common/dng_writer.h b/src/apps/common/dng_writer.h
> index 917713e61001..aaa8a852b307 100644
> --- a/src/apps/common/dng_writer.h
> +++ b/src/apps/common/dng_writer.h
> @@ -8,7 +8,6 @@
>  #pragma once
>  
>  #ifdef HAVE_TIFF
> -#define HAVE_DNG
>  
>  #include <libcamera/camera.h>
>  #include <libcamera/controls.h>
> diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp
> index dd2aa19618a1..5144c6b3eb30 100644
> --- a/src/apps/qcam/main_window.cpp
> +++ b/src/apps/qcam/main_window.cpp
> @@ -211,7 +211,7 @@ int MainWindow::createToolbars()
>  	action->setShortcut(QKeySequence::SaveAs);
>  	connect(action, &QAction::triggered, this, &MainWindow::saveImageAs);
>  
> -#ifdef HAVE_DNG
> +#ifdef HAVE_TIFF
>  	/* Save Raw action. */
>  	action = toolbar_->addAction(QIcon::fromTheme("camera-photo",
>  						      QIcon(":aperture.svg")),
> @@ -646,7 +646,7 @@ void MainWindow::captureRaw()
>  void MainWindow::processRaw(FrameBuffer *buffer,
>  			    [[maybe_unused]] const ControlList &metadata)
>  {
> -#ifdef HAVE_DNG
> +#ifdef HAVE_TIFF
>  	QString defaultPath = QStandardPaths::writableLocation(QStandardPaths::PicturesLocation);
>  	QString filename = QFileDialog::getSaveFileName(this, "Save DNG", defaultPath,
>  							"DNG Files (*.dng)");

Hi Laurent

On Wed, Sep 25, 2024 at 06:21:33PM GMT, Laurent Pinchart wrote:
> Support for DNG capture is conditioned by the availability of libtiff,
> which is indicated by the HAVE_TIFF macro set by meson. The dng_writer.h
> header then defines HAVE_DNG, which is used is a couple of places to
> conditionally compile DNG-related code. Most of the other locations
> where conditional compilation is required use HAVE_TIFF.
>
> Using both HAVE_TIFF and HAVE_DNG is confusing. HAVE_DNG would be a
> better name, but as the macro is defined in dng_writer.h, it would
> require all files that need to test for DNG support to include that
> header. Failure to include it (directly or indirectly) would result in
> the code covered by the macro to be silently disabled.
>
> To avoid the confusion, standardize on using HAVE_TIFF everywhere and
> drop HAVE_DNG.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

> ---
>  src/apps/common/dng_writer.h  | 1 -
>  src/apps/qcam/main_window.cpp | 4 ++--
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/src/apps/common/dng_writer.h b/src/apps/common/dng_writer.h
> index 917713e61001..aaa8a852b307 100644
> --- a/src/apps/common/dng_writer.h
> +++ b/src/apps/common/dng_writer.h
> @@ -8,7 +8,6 @@
>  #pragma once
>
>  #ifdef HAVE_TIFF
> -#define HAVE_DNG
>
>  #include <libcamera/camera.h>
>  #include <libcamera/controls.h>
> diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp
> index dd2aa19618a1..5144c6b3eb30 100644
> --- a/src/apps/qcam/main_window.cpp
> +++ b/src/apps/qcam/main_window.cpp
> @@ -211,7 +211,7 @@ int MainWindow::createToolbars()
>  	action->setShortcut(QKeySequence::SaveAs);
>  	connect(action, &QAction::triggered, this, &MainWindow::saveImageAs);
>
> -#ifdef HAVE_DNG
> +#ifdef HAVE_TIFF
>  	/* Save Raw action. */
>  	action = toolbar_->addAction(QIcon::fromTheme("camera-photo",
>  						      QIcon(":aperture.svg")),
> @@ -646,7 +646,7 @@ void MainWindow::captureRaw()
>  void MainWindow::processRaw(FrameBuffer *buffer,
>  			    [[maybe_unused]] const ControlList &metadata)
>  {
> -#ifdef HAVE_DNG
> +#ifdef HAVE_TIFF
>  	QString defaultPath = QStandardPaths::writableLocation(QStandardPaths::PicturesLocation);
>  	QString filename = QFileDialog::getSaveFileName(this, "Save DNG", defaultPath,
>  							"DNG Files (*.dng)");
> --
> Regards,
>
> Laurent Pinchart
>