Hi Hans,
On Thu, Jan 24, 2019 at 09:44:13AM +0100, Hans Verkuil wrote:
> Note: the subject it wrong, it's v4l-utils, not yavta.
Oops, my bad. I must have been thinking of yavta when writing that. :-)
And thanks for the review.
>
> On 1/22/19 1:05 PM, Sakari Ailus wrote:
> > Add support for META_OUTPUT buffer type to v4l2-ctl.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > ---
> > Hi Hans,
> >
> > Here's an update for the meta output buffer type in v4l2-ctl.
> >
> > Since v1:
> >
> > - Merge help text for meta and meta out buffer types
> >
> > - Unify implementation for meta format handling
> >
> > utils/v4l2-ctl/v4l2-ctl-meta.cpp | 71 ++++++++++++++++++++++++++++++++++++----
> > utils/v4l2-ctl/v4l2-ctl.cpp | 7 ++++
> > utils/v4l2-ctl/v4l2-ctl.h | 5 +++
> > 3 files changed, 76 insertions(+), 7 deletions(-)
> >
> > diff --git a/utils/v4l2-ctl/v4l2-ctl-meta.cpp b/utils/v4l2-ctl/v4l2-ctl-meta.cpp
> > index 37c91940a8..f4aa434937 100644
> > --- a/utils/v4l2-ctl/v4l2-ctl-meta.cpp
> > +++ b/utils/v4l2-ctl/v4l2-ctl-meta.cpp
> > @@ -21,14 +21,22 @@ static struct v4l2_format vfmt; /* set_format/get_format */
> > void meta_usage(void)
> > {
> > printf("\nMetadata Formats options:\n"
> > - " --list-formats-meta display supported metadata formats [VIDIOC_ENUM_FMT]\n"
> > - " --get-fmt-meta query the metadata format [VIDIOC_G_FMT]\n"
> > - " --set-fmt-meta <f> set the metadata format [VIDIOC_S_FMT]\n"
> > + " --list-formats-meta display supported metadata capture formats [VIDIOC_ENUM_FMT]\n"
> > + " --get-fmt-meta query the metadata capture format [VIDIOC_G_FMT]\n"
> > + " --set-fmt-meta <f> set the metadata capture format [VIDIOC_S_FMT]\n"
> > " parameter is either the format index as reported by\n"
> > " --list-formats-meta, or the fourcc value as a string\n"
> > - " --try-fmt-meta <f> try the metadata format [VIDIOC_TRY_FMT]\n"
> > + " --try-fmt-meta <f> try the metadata capture format [VIDIOC_TRY_FMT]\n"
> > " parameter is either the format index as reported by\n"
> > " --list-formats-meta, or the fourcc value as a string\n"
> > + " --list-formats-meta-out display supported metadata output formats [VIDIOC_ENUM_FMT]\n"
> > + " --get-fmt-meta-out query the metadata output format [VIDIOC_G_FMT]\n"
> > + " --set-fmt-meta-out <f> set the metadata output format [VIDIOC_S_FMT]\n"
> > + " parameter is either the format index as reported by\n"
> > + " --list-formats-meta-out, or the fourcc value as a string\n"
> > + " --try-fmt-meta-out <f> try the metadata output format [VIDIOC_TRY_FMT]\n"
> > + " parameter is either the format index as reported by\n"
> > + " --list-formats-meta-out, or the fourcc value as a string\n"
> > );
> > }
> >
> > @@ -37,6 +45,8 @@ void meta_cmd(int ch, char *optarg)
> > switch (ch) {
> > case OptSetMetaFormat:
> > case OptTryMetaFormat:
> > + case OptSetMetaOutFormat:
> > + case OptTryMetaOutFormat:
> > if (strlen(optarg) == 0) {
> > meta_usage();
> > exit(1);
> > @@ -55,8 +65,38 @@ void meta_set(cv4l_fd &_fd)
> > int fd = _fd.g_fd();
> > int ret;
> >
> > + if (!v4l_type_is_meta(_fd.g_type()))
> > + return;
> > +
> > if ((options[OptSetMetaFormat] || options[OptTryMetaFormat]) &&
> > - v4l_type_is_meta(_fd.g_type())) {
> > + v4l_type_is_capture(_fd.g_type())) {
> > + struct v4l2_format in_vfmt;
> > +
> > + in_vfmt.type = _fd.g_type();
> > + in_vfmt.fmt.meta.dataformat = vfmt.fmt.meta.dataformat;
> > +
> > + if (in_vfmt.fmt.meta.dataformat < 256) {
> > + struct v4l2_fmtdesc fmt;
> > +
> > + fmt.index = in_vfmt.fmt.meta.dataformat;
> > + fmt.type = in_vfmt.type;
> > +
> > + if (doioctl(fd, VIDIOC_ENUM_FMT, &fmt))
> > + fmt.pixelformat = 0;
> > +
> > + in_vfmt.fmt.meta.dataformat = fmt.pixelformat;
> > + }
> > +
> > + if (options[OptSetMetaFormat])
> > + ret = doioctl(fd, VIDIOC_S_FMT, &in_vfmt);
> > + else
> > + ret = doioctl(fd, VIDIOC_TRY_FMT, &in_vfmt);
> > + if (ret == 0 && (verbose || options[OptTryMetaFormat]))
> > + printfmt(fd, in_vfmt);
>
> This is exactly the same code for meta output. Why not just do:
>
> if ((options[OptSetMetaFormat] || options[OptTryMetaFormat] ||
> options[OptSetMetaOutFormat] || options[OptTryMetaOutFormat]) &&
> v4l_type_is_meta(_fd.g_type())
>
> You can add tests for capture/output if you like, but it is not really necessary.
Ack, I'll fix that.
>
>
> > + }
> > +
> > + if ((options[OptSetMetaOutFormat] || options[OptTryMetaOutFormat]) &&
> > + v4l_type_is_output(_fd.g_type())) {
> > struct v4l2_format in_vfmt;
> >
> > in_vfmt.type = _fd.g_type();
> > @@ -85,7 +125,16 @@ void meta_set(cv4l_fd &_fd)
> >
> > void meta_get(cv4l_fd &fd)
> > {
> > - if (options[OptGetMetaFormat] && v4l_type_is_meta(fd.g_type())) {
> > + if (!v4l_type_is_meta(fd.g_type()))
> > + return;
> > +
> > + if (options[OptGetMetaFormat] && v4l_type_is_capture(fd.g_type())) {
> > + vfmt.type = fd.g_type();
> > + if (doioctl(fd.g_fd(), VIDIOC_G_FMT, &vfmt) == 0)
> > + printfmt(fd.g_fd(), vfmt);
> > + }
> > +
> > + if (options[OptGetMetaOutFormat] && v4l_type_is_output(fd.g_type())) {
> > vfmt.type = fd.g_type();
> > if (doioctl(fd.g_fd(), VIDIOC_G_FMT, &vfmt) == 0)
> > printfmt(fd.g_fd(), vfmt);
> > @@ -94,7 +143,15 @@ void meta_get(cv4l_fd &fd)
> >
> > void meta_list(cv4l_fd &fd)
> > {
> > - if (options[OptListMetaFormats] && v4l_type_is_meta(fd.g_type())) {
> > + if (!v4l_type_is_meta(fd.g_type()))
> > + return;
> > +
> > + if (options[OptListMetaFormats] && v4l_type_is_capture(fd.g_type())) {
> > + printf("ioctl: VIDIOC_ENUM_FMT\n");
> > + print_video_formats(fd, fd.g_type());
> > + }
> > +
> > + if (options[OptListMetaOutFormats] && v4l_type_is_output(fd.g_type())) {
> > printf("ioctl: VIDIOC_ENUM_FMT\n");
> > print_video_formats(fd, fd.g_type());
> > }
>
> Same for these two functions: the same code works for both meta capture and output.
>
> To be honest, I would really like to completely rework the way v4l2-ctl handles this.
> There are way too many options to list, get, set, try formats. But you really don't
> need to specify the type (video, meta, sdr, vbi, etc) as that can be determined
> automatically.
As long as a video node supports a single type only. Aren't there cases
that need multiple types? Vbi?
>
> But let's get this done first, and then I'll take another look at this.
>
> Regards,
>
> Hans
>
>
> > diff --git a/utils/v4l2-ctl/v4l2-ctl.cpp b/utils/v4l2-ctl/v4l2-ctl.cpp
> > index 1783979d76..ffac8716d0 100644
> > --- a/utils/v4l2-ctl/v4l2-ctl.cpp
> > +++ b/utils/v4l2-ctl/v4l2-ctl.cpp
> > @@ -122,6 +122,7 @@ static struct option long_options[] = {
> > {"list-formats-out", no_argument, 0, OptListOutFormats},
> > {"list-formats-out-ext", no_argument, 0, OptListOutFormatsExt},
> > {"list-formats-meta", no_argument, 0, OptListMetaFormats},
> > + {"list-formats-meta-out", no_argument, 0, OptListMetaOutFormats},
> > {"list-subdev-mbus-codes", optional_argument, 0, OptListSubDevMBusCodes},
> > {"list-subdev-framesizes", required_argument, 0, OptListSubDevFrameSizes},
> > {"list-subdev-frameintervals", required_argument, 0, OptListSubDevFrameIntervals},
> > @@ -174,6 +175,9 @@ static struct option long_options[] = {
> > {"get-fmt-meta", no_argument, 0, OptGetMetaFormat},
> > {"set-fmt-meta", required_argument, 0, OptSetMetaFormat},
> > {"try-fmt-meta", required_argument, 0, OptTryMetaFormat},
> > + {"get-fmt-meta-out", no_argument, 0, OptGetMetaOutFormat},
> > + {"set-fmt-meta-out", required_argument, 0, OptSetMetaOutFormat},
> > + {"try-fmt-meta-out", required_argument, 0, OptTryMetaOutFormat},
> > {"get-subdev-fmt", optional_argument, 0, OptGetSubDevFormat},
> > {"set-subdev-fmt", required_argument, 0, OptSetSubDevFormat},
> > {"try-subdev-fmt", required_argument, 0, OptTrySubDevFormat},
> > @@ -238,6 +242,7 @@ static struct option long_options[] = {
> > {"list-buffers-sdr", no_argument, 0, OptListBuffersSdr},
> > {"list-buffers-sdr-out", no_argument, 0, OptListBuffersSdrOut},
> > {"list-buffers-meta", no_argument, 0, OptListBuffersMeta},
> > + {"list-buffers-meta-out", no_argument, 0, OptListBuffersMetaOut},
> > {"stream-count", required_argument, 0, OptStreamCount},
> > {"stream-skip", required_argument, 0, OptStreamSkip},
> > {"stream-loop", no_argument, 0, OptStreamLoop},
> > @@ -507,6 +512,7 @@ void printfmt(int fd, const struct v4l2_format &vfmt)
> > printf("\tBuffer Size : %u\n", vfmt.fmt.sdr.buffersize);
> > break;
> > case V4L2_BUF_TYPE_META_CAPTURE:
> > + case V4L2_BUF_TYPE_META_OUTPUT:
> > printf("\tSample Format : '%s'%s\n", fcc2s(vfmt.fmt.meta.dataformat).c_str(),
> > printfmtname(fd, vfmt.type, vfmt.fmt.meta.dataformat).c_str());
> > printf("\tBuffer Size : %u\n", vfmt.fmt.meta.buffersize);
> > @@ -1247,6 +1253,7 @@ int main(int argc, char **argv)
> > options[OptGetSdrFormat] = 1;
> > options[OptGetSdrOutFormat] = 1;
> > options[OptGetMetaFormat] = 1;
> > + options[OptGetMetaOutFormat] = 1;
> > options[OptGetFBuf] = 1;
> > options[OptGetCropCap] = 1;
> > options[OptGetOutputCropCap] = 1;
> > diff --git a/utils/v4l2-ctl/v4l2-ctl.h b/utils/v4l2-ctl/v4l2-ctl.h
> > index 5a52a0a48f..fc51cd1b97 100644
> > --- a/utils/v4l2-ctl/v4l2-ctl.h
> > +++ b/utils/v4l2-ctl/v4l2-ctl.h
> > @@ -89,6 +89,7 @@ enum Option {
> > OptGetSdrFormat,
> > OptGetSdrOutFormat,
> > OptGetMetaFormat,
> > + OptGetMetaOutFormat,
> > OptGetSubDevFormat,
> > OptSetSlicedVbiOutFormat,
> > OptSetOverlayFormat,
> > @@ -97,6 +98,7 @@ enum Option {
> > OptSetSdrFormat,
> > OptSetSdrOutFormat,
> > OptSetMetaFormat,
> > + OptSetMetaOutFormat,
> > OptSetSubDevFormat,
> > OptTryVideoOutFormat,
> > OptTrySlicedVbiOutFormat,
> > @@ -108,6 +110,7 @@ enum Option {
> > OptTrySdrFormat,
> > OptTrySdrOutFormat,
> > OptTryMetaFormat,
> > + OptTryMetaOutFormat,
> > OptTrySubDevFormat,
> > OptAll,
> > OptListStandards,
> > @@ -122,6 +125,7 @@ enum Option {
> > OptListOutFormats,
> > OptListOutFormatsExt,
> > OptListMetaFormats,
> > + OptListMetaOutFormats,
> > OptListSubDevMBusCodes,
> > OptListSubDevFrameSizes,
> > OptListSubDevFrameIntervals,
> > @@ -205,6 +209,7 @@ enum Option {
> > OptListBuffersSdr,
> > OptListBuffersSdrOut,
> > OptListBuffersMeta,
> > + OptListBuffersMetaOut,
> > OptStreamCount,
> > OptStreamSkip,
> > OptStreamLoop,
> >
>
--
Sakari Ailus
sakari.ailus@xxxxxxxxxxxxxxx
