Hello Helen, Just a few drive-by comments. On Fri, 17 Jul 2020 08:54:29 -0300 Helen Koike <helen.koike@xxxxxxxxxxxxx> wrote: > Hi, > > I'm sorry for taking too long to submit v4. > > It is not perfect, not all v4l2-compliance tests passes, but I'd like a review, > specially on the API and potential problems, so I can focus on improving implementation > and maybe drop the RFC tag for next version. > > Follow below what changed in v4 and some items I'd like to discuss: > > > * Ioctl to replace v4l2_pix_format > --------------------------------------------------------------------------------- > During last media summit, we agreed to create ioctls that replace the v4l2_pix_format > struct and leave the other structs in the v4l2_format union alone. > Thus I refactored the code to receive struct v4l2_ext_pix_format, and I renamed the > ioctls, so now we have: > > int ioctl(int fd, VIDIOC_G_EXT_FMT, struct v4l2_ext_pix_format *argp); Maybe use the EXT_PIX_FMT suffix here since the struct is really only about pixel formats. > int ioctl(int fd, VIDIOC_S_EXT_FMT, struct v4l2_ext_pix_format *argp); > int ioctl(int fd, VIDIOC_TRY_EXT_FMT, struct v4l2_ext_pix_format *argp); > > The only valid types are V4L2_BUF_TYPE_VIDEO_CAPTURE and V4L2_BUF_TYPE_VIDEO_OUTPUT, > all the other types are invalid with this API. > [...] > > > Boris Brezillon (5): > media: v4l2: Extend pixel formats to unify single/multi-planar > handling (and more) > media: videobuf2: Expose helpers to implement the _ext_fmt and > _ext_buf hooks > media: mediabus: Add helpers to convert a ext_pix format to/from a > mbus_fmt > media: vivid: Convert the capture and output drivers to > EXT_FMT/EXT_BUF > media: vimc: Implement the ext_fmt and ext_buf hooks I think you should take ownership of these patches. The end result is likely to be completely different from what I initially posted, and you're the one doing the hard work here. > > Hans Verkuil (1): > media: v4l2: Add extended buffer operations > > .../media/common/videobuf2/videobuf2-core.c | 2 + > .../media/common/videobuf2/videobuf2-v4l2.c | 549 +++++----- > .../media/test-drivers/vimc/vimc-capture.c | 61 +- > drivers/media/test-drivers/vimc/vimc-common.c | 6 +- > drivers/media/test-drivers/vimc/vimc-common.h | 2 +- > drivers/media/test-drivers/vivid/vivid-core.c | 70 +- > .../test-drivers/vivid/vivid-touch-cap.c | 26 +- > .../test-drivers/vivid/vivid-touch-cap.h | 3 +- > .../media/test-drivers/vivid/vivid-vid-cap.c | 169 +--- > .../media/test-drivers/vivid/vivid-vid-cap.h | 15 +- > .../media/test-drivers/vivid/vivid-vid-out.c | 193 ++-- > .../media/test-drivers/vivid/vivid-vid-out.h | 15 +- > drivers/media/v4l2-core/v4l2-dev.c | 50 +- > drivers/media/v4l2-core/v4l2-ioctl.c | 934 ++++++++++++++++-- > include/media/v4l2-ioctl.h | 60 ++ > include/media/v4l2-mediabus.h | 42 + > include/media/videobuf2-core.h | 6 +- > include/media/videobuf2-v4l2.h | 21 +- > include/uapi/linux/videodev2.h | 144 +++ > 19 files changed, 1650 insertions(+), 718 deletions(-) >