Message ID | 20250127064745.5740-1-laurent.pinchart@ideasonboard.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Quoting Laurent Pinchart (2025-01-27 06:47:45) > Add a data() function to the Matrix class to access the internal data. > This is useful for code that needs to use the matrix contents as a > linear array, as shows by the RkISP1::Ccm::process() function that needs s/shows/shown/ > to copy the matrix data to a local variable. Simplify that function by > using the new accessor. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/internal/matrix.h | 2 ++ > src/ipa/rkisp1/algorithms/ccm.cpp | 7 +------ > src/libcamera/matrix.cpp | 6 ++++++ > 3 files changed, 9 insertions(+), 6 deletions(-) > > diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h > index 7a71028c473a..a055e6926c94 100644 > --- a/include/libcamera/internal/matrix.h > +++ b/include/libcamera/internal/matrix.h > @@ -66,6 +66,8 @@ public: > return out.str(); > } > > + Span<const T, Rows * Cols> data() const { return data_; } > + > Span<const T, Cols> operator[](size_t i) const > { > return Span<const T, Cols>{ &data_.data()[i * Cols], Cols }; > diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp > index e2b5cf4d313e..eb8ca39e56a8 100644 > --- a/src/ipa/rkisp1/algorithms/ccm.cpp > +++ b/src/ipa/rkisp1/algorithms/ccm.cpp > @@ -120,12 +120,7 @@ void Ccm::process([[maybe_unused]] IPAContext &context, > [[maybe_unused]] const rkisp1_stat_buffer *stats, > ControlList &metadata) > { > - float m[9]; > - for (unsigned int i = 0; i < 3; i++) { > - for (unsigned int j = 0; j < 3; j++) > - m[i * 3 + j] = frameContext.ccm.ccm[i][j]; > - } > - metadata.set(controls::ColourCorrectionMatrix, m); > + metadata.set(controls::ColourCorrectionMatrix, frameContext.ccm.ccm.data()); That's better than iterating 3x3. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > } > > REGISTER_IPA_ALGORITHM(Ccm, "Ccm") > diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp > index 4d95a19bfbb9..d9d34867b0a3 100644 > --- a/src/libcamera/matrix.cpp > +++ b/src/libcamera/matrix.cpp > @@ -52,6 +52,12 @@ LOG_DEFINE_CATEGORY(Matrix) > * \return A string describing the matrix > */ > > +/** > + * \fn Matrix::data() > + * \brief Access the internal data as a linear array > + * \return A span referencing the internal data as a linear array > + */ > + > /** > * \fn Span<const T, Cols> Matrix::operator[](size_t i) const > * \brief Index to a row in the matrix > > base-commit: 8aef7b4dfb12f2f9bf0513625231b85a5b8f5440 > -- > Regards, > > Laurent Pinchart >
Hi Laurent, thank you for the patch. Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > Add a data() function to the Matrix class to access the internal data. > This is useful for code that needs to use the matrix contents as a > linear array, as shows by the RkISP1::Ccm::process() function that needs > to copy the matrix data to a local variable. Simplify that function by > using the new accessor. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/internal/matrix.h | 2 ++ > src/ipa/rkisp1/algorithms/ccm.cpp | 7 +------ > src/libcamera/matrix.cpp | 6 ++++++ > 3 files changed, 9 insertions(+), 6 deletions(-) > > diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h > index 7a71028c473a..a055e6926c94 100644 > --- a/include/libcamera/internal/matrix.h > +++ b/include/libcamera/internal/matrix.h > @@ -66,6 +66,8 @@ public: > return out.str(); > } > > + Span<const T, Rows * Cols> data() const { return data_; } > + > Span<const T, Cols> operator[](size_t i) const > { > return Span<const T, Cols>{ &data_.data()[i * Cols], Cols }; > diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp > index e2b5cf4d313e..eb8ca39e56a8 100644 > --- a/src/ipa/rkisp1/algorithms/ccm.cpp > +++ b/src/ipa/rkisp1/algorithms/ccm.cpp > @@ -120,12 +120,7 @@ void Ccm::process([[maybe_unused]] IPAContext &context, > [[maybe_unused]] const rkisp1_stat_buffer *stats, > ControlList &metadata) > { > - float m[9]; > - for (unsigned int i = 0; i < 3; i++) { > - for (unsigned int j = 0; j < 3; j++) > - m[i * 3 + j] = frameContext.ccm.ccm[i][j]; > - } > - metadata.set(controls::ColourCorrectionMatrix, m); > + metadata.set(controls::ColourCorrectionMatrix, frameContext.ccm.ccm.data()); > } > > REGISTER_IPA_ALGORITHM(Ccm, "Ccm") > diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp > index 4d95a19bfbb9..d9d34867b0a3 100644 > --- a/src/libcamera/matrix.cpp > +++ b/src/libcamera/matrix.cpp > @@ -52,6 +52,12 @@ LOG_DEFINE_CATEGORY(Matrix) > * \return A string describing the matrix > */ > > +/** > + * \fn Matrix::data() > + * \brief Access the internal data as a linear array > + * \return A span referencing the internal data as a linear array "Internal data" can be anything and if it is supposed to be used in places like metadata or for any "real" purpose, it should be better specified. Something like the documentation of `data' argument in the constructor? > + */ > + > /** > * \fn Span<const T, Cols> Matrix::operator[](size_t i) const > * \brief Index to a row in the matrix > > base-commit: 8aef7b4dfb12f2f9bf0513625231b85a5b8f5440
diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h index 7a71028c473a..a055e6926c94 100644 --- a/include/libcamera/internal/matrix.h +++ b/include/libcamera/internal/matrix.h @@ -66,6 +66,8 @@ public: return out.str(); } + Span<const T, Rows * Cols> data() const { return data_; } + Span<const T, Cols> operator[](size_t i) const { return Span<const T, Cols>{ &data_.data()[i * Cols], Cols }; diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp index e2b5cf4d313e..eb8ca39e56a8 100644 --- a/src/ipa/rkisp1/algorithms/ccm.cpp +++ b/src/ipa/rkisp1/algorithms/ccm.cpp @@ -120,12 +120,7 @@ void Ccm::process([[maybe_unused]] IPAContext &context, [[maybe_unused]] const rkisp1_stat_buffer *stats, ControlList &metadata) { - float m[9]; - for (unsigned int i = 0; i < 3; i++) { - for (unsigned int j = 0; j < 3; j++) - m[i * 3 + j] = frameContext.ccm.ccm[i][j]; - } - metadata.set(controls::ColourCorrectionMatrix, m); + metadata.set(controls::ColourCorrectionMatrix, frameContext.ccm.ccm.data()); } REGISTER_IPA_ALGORITHM(Ccm, "Ccm") diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp index 4d95a19bfbb9..d9d34867b0a3 100644 --- a/src/libcamera/matrix.cpp +++ b/src/libcamera/matrix.cpp @@ -52,6 +52,12 @@ LOG_DEFINE_CATEGORY(Matrix) * \return A string describing the matrix */ +/** + * \fn Matrix::data() + * \brief Access the internal data as a linear array + * \return A span referencing the internal data as a linear array + */ + /** * \fn Span<const T, Cols> Matrix::operator[](size_t i) const * \brief Index to a row in the matrix
Add a data() function to the Matrix class to access the internal data. This is useful for code that needs to use the matrix contents as a linear array, as shows by the RkISP1::Ccm::process() function that needs to copy the matrix data to a local variable. Simplify that function by using the new accessor. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- include/libcamera/internal/matrix.h | 2 ++ src/ipa/rkisp1/algorithms/ccm.cpp | 7 +------ src/libcamera/matrix.cpp | 6 ++++++ 3 files changed, 9 insertions(+), 6 deletions(-) base-commit: 8aef7b4dfb12f2f9bf0513625231b85a5b8f5440