[libcamera-devel] [PATCH 1/3] controls: Promote NoiseReductionMode to non-draft
Naushir Patuck
naush at raspberrypi.com
Mon Sep 13 12:31:39 CEST 2021
Hi Paul,
Thank you for your patch.
On Mon, 13 Sept 2021 at 11:20, Paul Elder <paul.elder at ideasonboard.com>
wrote:
> Promote NoiseReductionMode to a non-draft control. Upgrade the value
> descriptions. Update the raspberrypi IPA accordingly, as it is the only
> current user of this control.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
> include/libcamera/ipa/raspberrypi.h | 2 +-
> src/android/camera_capabilities.cpp | 2 +-
> src/ipa/raspberrypi/raspberrypi.cpp | 12 ++---
> src/libcamera/control_ids.yaml | 73 +++++++++++++++++------------
> 4 files changed, 52 insertions(+), 37 deletions(-)
>
> diff --git a/include/libcamera/ipa/raspberrypi.h
> b/include/libcamera/ipa/raspberrypi.h
> index 521eaecd..e0dc6f5e 100644
> --- a/include/libcamera/ipa/raspberrypi.h
> +++ b/include/libcamera/ipa/raspberrypi.h
> @@ -45,7 +45,7 @@ static const ControlInfoMap Controls({
> { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f,
> 16.0f) },
> { &controls::ScalerCrop, ControlInfo(Rectangle{},
> Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },
> { &controls::FrameDurationLimits,
> ControlInfo(INT64_C(1000), INT64_C(1000000000)) },
> - { &controls::draft::NoiseReductionMode,
> ControlInfo(controls::draft::NoiseReductionModeValues) }
> + { &controls::NoiseReductionMode,
> ControlInfo(controls::NoiseReductionModeValues) }
> }, controls::controls);
>
> } /* namespace RPi */
> diff --git a/src/android/camera_capabilities.cpp
> b/src/android/camera_capabilities.cpp
> index e92bca42..08e44a1a 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -1175,7 +1175,7 @@ int CameraCapabilities::initializeStaticMetadata()
> {
> std::vector<uint8_t> data;
> data.reserve(5);
> - const auto &infoMap =
> controlsInfo.find(&controls::draft::NoiseReductionMode);
> + const auto &infoMap =
> controlsInfo.find(&controls::NoiseReductionMode);
> if (infoMap != controlsInfo.end()) {
> for (const auto &value : infoMap->second.values())
> data.push_back(value.get<int32_t>());
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> b/src/ipa/raspberrypi/raspberrypi.cpp
> index 047123ab..8d44ab0a 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -605,11 +605,11 @@ static const std::map<int32_t, std::string>
> AwbModeTable = {
> };
>
> static const std::map<int32_t, RPiController::DenoiseMode>
> DenoiseModeTable = {
> - { controls::draft::NoiseReductionModeOff,
> RPiController::DenoiseMode::Off },
> - { controls::draft::NoiseReductionModeFast,
> RPiController::DenoiseMode::ColourFast },
> - { controls::draft::NoiseReductionModeHighQuality,
> RPiController::DenoiseMode::ColourHighQuality },
> - { controls::draft::NoiseReductionModeMinimal,
> RPiController::DenoiseMode::ColourOff },
> - { controls::draft::NoiseReductionModeZSL,
> RPiController::DenoiseMode::ColourHighQuality },
> + { controls::NoiseReductionModeOff, RPiController::DenoiseMode::Off
> },
> + { controls::NoiseReductionModeFast,
> RPiController::DenoiseMode::ColourFast },
> + { controls::NoiseReductionModeHighQuality,
> RPiController::DenoiseMode::ColourHighQuality },
> + { controls::NoiseReductionModeMinimal,
> RPiController::DenoiseMode::ColourOff },
> + { controls::NoiseReductionModeZSL,
> RPiController::DenoiseMode::ColourHighQuality },
> };
>
> void IPARPi::queueRequest(const ControlList &controls)
> @@ -898,7 +898,7 @@ void IPARPi::queueRequest(const ControlList &controls)
> * analysis image resolution or format
> mismatch, we should
> * report the status correctly in the
> metadata.
> */
> -
> libcameraMetadata_.set(controls::draft::NoiseReductionMode, idx);
> +
> libcameraMetadata_.set(controls::NoiseReductionMode, idx);
> } else {
> LOG(IPARPI, Error) << "Noise reduction
> mode " << idx
> << " not recognised";
> diff --git a/src/libcamera/control_ids.yaml
> b/src/libcamera/control_ids.yaml
> index 9d4638ae..fffcca2d 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -381,6 +381,50 @@ controls:
> \todo Define how the sensor timestamp has to be used in the
> reprocessing
> use case.
>
> + - NoiseReductionMode:
> + type: int32_t
> + description: |
> + Mode of operation for the noise reduction algorithm.
> +
> + The noise reduction algorithm attempts to improve image quality by
> + removing excessive noise added by the capture process, especially
> in
> + dark conditions.
> + enum:
> + - name: NoiseReductionModeOff
> + value: 0
> + description: |
> + No noise reduction will be applied by the camera device, for
> + both the raw and YUV domains.
> + - name: NoiseReductionModeFast
> + value: 1
> + description: |
> + Noise reduction is applied without reducing the frame rate.
> + This may be the same as Raw if it is listed, or the same as
> Off if
> + any noise reduction will slow down the frame rate.
>
I wonder if the stated requirement of "without reducing the framerate" is a
bit strict here?
There are certainly going to be cases, e.g. 60/120fps recording, where our
implementation will
drop frames and be unable to keep up. Perhaps this should be worded as:
"Noise reduction is applied with minimal possible impact on the frame rate."
> + - name: NoiseReductionModeHighQuality
> + value: 2
> + description: |
> + High quality noise reduction at the expense of frame rate.
> + - name: NoiseReductionModeRaw
> + value: 3
> + description: |
> + Only sensor raw domain basic noise reduction is enabled, to
> remove
> + demosaicing or other processing artifacts. Frame rate will
> not be
> + reduced.
>
Similar to the above comment, perhaps there may be an impact on the
framerate as well.
> + - name: NoiseReductionModeZSL
> + value: 4
> + description: |
> + Noise reduction is applied at different levels to different
> streams.
> +
> + ZSL is meant to be used by applications that maintain a
> continuous
> + circular buffer of high-resolution images during preview and
> + reprocess image(s) from that buffer into a final capture when
> + triggered by the user. In this mode, the camera device applies
> + noise reduction to low-resolution streams (below maximum
> recording
> + resolution) to maximize preview quality, but does not apply
> noise
> + reduction to high-resolution streams, since those will be
> + reprocessed later if necessary.
>
I know that this is following the Android API, but given that libcamera
does not
have reprocessing capabilities just yet, should this mode be left out for
now?
Regards,
Naush
> +
> #
> ----------------------------------------------------------------------------
> # Draft controls section
>
> @@ -427,35 +471,6 @@ controls:
> The camera will cancel any active trigger and the AF routine
> is
> reset to its initial state.
>
> - - NoiseReductionMode:
> - type: int32_t
> - draft: true
> - description: |
> - Control to select the noise reduction algorithm mode. Currently
> - identical to ANDROID_NOISE_REDUCTION_MODE.
> -
> - Mode of operation for the noise reduction algorithm.
> - enum:
> - - name: NoiseReductionModeOff
> - value: 0
> - description: No noise reduction is applied
> - - name: NoiseReductionModeFast
> - value: 1
> - description: |
> - Noise reduction is applied without reducing the frame rate.
> - - name: NoiseReductionModeHighQuality
> - value: 2
> - description: |
> - High quality noise reduction at the expense of frame rate.
> - - name: NoiseReductionModeMinimal
> - value: 3
> - description: |
> - Minimal noise reduction is applied without reducing the frame
> rate.
> - - name: NoiseReductionModeZSL
> - value: 4
> - description: |
> - Noise reduction is applied at different levels to different
> streams.
> -
> - ColorCorrectionAberrationMode:
> type: int32_t
> draft: true
> --
> 2.27.0
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210913/6c434946/attachment-0001.htm>
More information about the libcamera-devel
mailing list