[0/1] Add v4l2_subdev_format in V4L2SubdeviceFormat
mbox series

Message ID 20241105105445.1468954-1-chenghaoyang@chromium.org
Headers show
Series
  • Add v4l2_subdev_format in V4L2SubdeviceFormat
Related show

Message

Cheng-Hao Yang Nov. 5, 2024, 10:49 a.m. UTC
Hi folks,

This patch adds extra information in V4L2SubdeviceFormat, which will
be needed in mtkisp7:
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/mtkisp7/src/libcamera/pipeline/mtkisp7/camsys/camsys.cpp;l=371

I think we can also choose to just add the format (type:
v4l2_mbus_framefmt) into the struct though. However, in either approach,
we are duplicating the information with other existing member variables.

Please check if this patch makes sense, or if there are other
approaches.

BR,
Harvey


Han-Lin Chen (1):
  libcamera: Add v4l2_subdev_format in V4L2SubdeviceFormat

 include/libcamera/internal/v4l2_subdevice.h |  2 ++
 src/libcamera/pipeline/simple/simple.cpp    |  2 +-
 src/libcamera/sensor/camera_sensor.cpp      |  1 +
 src/libcamera/v4l2_subdevice.cpp            | 10 ++++++++++
 4 files changed, 14 insertions(+), 1 deletion(-)

Comments

Laurent Pinchart Nov. 5, 2024, 4:20 p.m. UTC | #1
Hi Harvey,

On Tue, Nov 05, 2024 at 10:49:20AM +0000, Harvey Yang wrote:
> Hi folks,
> 
> This patch adds extra information in V4L2SubdeviceFormat, which will
> be needed in mtkisp7:
> https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/mtkisp7/src/libcamera/pipeline/mtkisp7/camsys/camsys.cpp;l=371
> 
> I think we can also choose to just add the format (type:
> v4l2_mbus_framefmt) into the struct though. However, in either approach,
> we are duplicating the information with other existing member variables.
> 
> Please check if this patch makes sense, or if there are other
> approaches.

I'll return the question: could you explain why this solution makes the
most sense, and what other approaches you've considered and decided to
reject, if any ? Giving us rationales for the design decisions behind
your patch series is crucial for us to understand what you're doing, and
will help speeding up the discussions and merging of the code.

> Han-Lin Chen (1):
>   libcamera: Add v4l2_subdev_format in V4L2SubdeviceFormat
> 
>  include/libcamera/internal/v4l2_subdevice.h |  2 ++
>  src/libcamera/pipeline/simple/simple.cpp    |  2 +-
>  src/libcamera/sensor/camera_sensor.cpp      |  1 +
>  src/libcamera/v4l2_subdevice.cpp            | 10 ++++++++++
>  4 files changed, 14 insertions(+), 1 deletion(-)
> 
> -- 
> 2.47.0.199.ga7371fff76-goog