[v3,5/9] ipa: rkisp1: params: Check for empty parameters
diff mbox series

Message ID 20250707085520.39777-6-stefan.klug@ideasonboard.com
State New
Headers show
Series
  • Wdr preparations
Related show

Commit Message

Stefan Klug July 7, 2025, 8:55 a.m. UTC
Add functions to check for empty parameter blocks. Modify the constructor
so that a parameter block constructed from an empty span stays empty.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

---

Changes in v3:
- Collected tags

Changes in v2:
- Removed unrelated get() functions
---
 src/ipa/rkisp1/params.cpp | 3 +++
 src/ipa/rkisp1/params.h   | 3 +++
 2 files changed, 6 insertions(+)

Comments

Laurent Pinchart July 7, 2025, 1:01 p.m. UTC | #1
Hi Stefan,

Thank you for the patch.

On Mon, Jul 07, 2025 at 10:55:08AM +0200, Stefan Klug wrote:
> Add functions to check for empty parameter blocks. Modify the constructor
> so that a parameter block constructed from an empty span stays empty.

I'd say that a block constructed from an empty span "is invalid", as
there's no "empty block" concept. Also, please explain in the commit
message why/when this can happen.

> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> ---
> 
> Changes in v3:
> - Collected tags
> 
> Changes in v2:
> - Removed unrelated get() functions
> ---
>  src/ipa/rkisp1/params.cpp | 3 +++
>  src/ipa/rkisp1/params.h   | 3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/src/ipa/rkisp1/params.cpp b/src/ipa/rkisp1/params.cpp
> index 4c0b051ce65d..b4a889e415fc 100644
> --- a/src/ipa/rkisp1/params.cpp
> +++ b/src/ipa/rkisp1/params.cpp
> @@ -82,6 +82,9 @@ RkISP1ParamsBlockBase::RkISP1ParamsBlockBase(RkISP1Params *params, BlockType typ
>  					     const Span<uint8_t> &data)
>  	: params_(params), type_(type)
>  {
> +	if (data.empty())
> +		return;
> +
>  	if (params_->format() == V4L2_META_FMT_RK_ISP1_EXT_PARAMS) {
>  		header_ = data.subspan(0, sizeof(rkisp1_ext_params_block_header));
>  		data_ = data.subspan(sizeof(rkisp1_ext_params_block_header));
> diff --git a/src/ipa/rkisp1/params.h b/src/ipa/rkisp1/params.h
> index 40450e34497a..04b06c2a6266 100644
> --- a/src/ipa/rkisp1/params.h
> +++ b/src/ipa/rkisp1/params.h
> @@ -89,6 +89,9 @@ public:
>  
>  	void setEnabled(bool enabled);
>  
> +	bool isValid() const { return !data_.empty(); }

This function doesn't seem to be used, neither in this series nor in the
"[PATCH 0/8] Implement WDR algorithm" series.

> +	explicit operator bool() const { return !data_.empty(); }
> +
>  private:
>  	LIBCAMERA_DISABLE_COPY(RkISP1ParamsBlockBase)
>

Patch
diff mbox series

diff --git a/src/ipa/rkisp1/params.cpp b/src/ipa/rkisp1/params.cpp
index 4c0b051ce65d..b4a889e415fc 100644
--- a/src/ipa/rkisp1/params.cpp
+++ b/src/ipa/rkisp1/params.cpp
@@ -82,6 +82,9 @@  RkISP1ParamsBlockBase::RkISP1ParamsBlockBase(RkISP1Params *params, BlockType typ
 					     const Span<uint8_t> &data)
 	: params_(params), type_(type)
 {
+	if (data.empty())
+		return;
+
 	if (params_->format() == V4L2_META_FMT_RK_ISP1_EXT_PARAMS) {
 		header_ = data.subspan(0, sizeof(rkisp1_ext_params_block_header));
 		data_ = data.subspan(sizeof(rkisp1_ext_params_block_header));
diff --git a/src/ipa/rkisp1/params.h b/src/ipa/rkisp1/params.h
index 40450e34497a..04b06c2a6266 100644
--- a/src/ipa/rkisp1/params.h
+++ b/src/ipa/rkisp1/params.h
@@ -89,6 +89,9 @@  public:
 
 	void setEnabled(bool enabled);
 
+	bool isValid() const { return !data_.empty(); }
+	explicit operator bool() const { return !data_.empty(); }
+
 private:
 	LIBCAMERA_DISABLE_COPY(RkISP1ParamsBlockBase)