| Message ID | 1456090187-1191-5-git-send-email-sakari.ailus@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/21/16 22:29, Sakari Ailus wrote: > Add a new topic option for -h to allow listing supported media bus codes > in conversion functions. This is useful in figuring out which media bus > codes are actually supported by the library. The numeric values of the > codes are listed as well. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 38 insertions(+), 4 deletions(-) > > diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c > index 0afc9c2..55cdd29 100644 > --- a/utils/media-ctl/options.c > +++ b/utils/media-ctl/options.c > @@ -22,7 +22,9 @@ > #include <getopt.h> > #include <stdio.h> > #include <stdlib.h> > +#include <string.h> > #include <unistd.h> > +#include <v4l2subdev.h> > > #include <linux/videodev2.h> > > @@ -45,7 +47,8 @@ static void usage(const char *argv0) > printf("-V, --set-v4l2 v4l2 Comma-separated list of formats to setup\n"); > printf(" --get-v4l2 pad Print the active format on a given pad\n"); > printf(" --set-dv pad Configure DV timings on a given pad\n"); > - printf("-h, --help Show verbose help and exit\n"); > + printf("-h, --help[=topic] Show verbose help and exit\n"); > + printf(" topics: mbus-fmt: List supported media bus pixel codes\n"); OK, this is ugly. It has nothing to do with usage help. Just make a new option --list-mbus-fmts to list supported media bus pixel codes. That would make much more sense. Regards, Hans > printf("-i, --interactive Modify links interactively\n"); > printf("-l, --links links Comma-separated list of link descriptors to setup\n"); > printf("-p, --print-topology Print the device topology\n"); > @@ -100,7 +103,7 @@ static struct option opts[] = { > {"get-format", 1, 0, OPT_GET_FORMAT}, > {"get-v4l2", 1, 0, OPT_GET_FORMAT}, > {"set-dv", 1, 0, OPT_SET_DV}, > - {"help", 0, 0, 'h'}, > + {"help", 2, 0, 'h'}, > {"interactive", 0, 0, 'i'}, > {"links", 1, 0, 'l'}, > {"print-dot", 0, 0, OPT_PRINT_DOT}, > @@ -110,6 +113,27 @@ static struct option opts[] = { > { }, > }; > > +void list_mbus_formats(void) > +{ > + unsigned int ncodes; > + const unsigned int *code = v4l2_subdev_pixelcode_list(&ncodes); > + > + printf("Supported media bus pixel codes\n"); > + > + for (ncodes++; ncodes; ncodes--, code++) { > + const char *str = v4l2_subdev_pixelcode_to_string(*code); > + int spaces = 30 - (int)strlen(str); > + > + if (*code == 0) > + break; > + > + if (spaces < 0) > + spaces = 0; > + > + printf("\t%s %*c (0x%8.8x)\n", str, spaces, ' ', *code); > + } > +} > + > int parse_cmdline(int argc, char **argv) > { > int opt; > @@ -120,7 +144,8 @@ int parse_cmdline(int argc, char **argv) > } > > /* parse options */ > - while ((opt = getopt_long(argc, argv, "d:e:f:hil:prvV:", opts, NULL)) != -1) { > + while ((opt = getopt_long(argc, argv, "d:e:f:h::il:prvV:", > + opts, NULL)) != -1) { > switch (opt) { > case 'd': > media_opts.devname = optarg; > @@ -142,7 +167,16 @@ int parse_cmdline(int argc, char **argv) > break; > > case 'h': > - usage(argv[0]); > + if (optarg) { > + if (!strcmp(optarg, "mbus-fmt")) > + list_mbus_formats(); > + else > + fprintf(stderr, > + "Unknown topic \"%s\"\n", > + optarg); > + } else { > + usage(argv[0]); > + } > exit(0); > > case 'i': > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/21/16 22:29, Sakari Ailus wrote: > Add a new topic option for -h to allow listing supported media bus codes > in conversion functions. This is useful in figuring out which media bus > codes are actually supported by the library. The numeric values of the > codes are listed as well. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 38 insertions(+), 4 deletions(-) > > diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c > index 0afc9c2..55cdd29 100644 > --- a/utils/media-ctl/options.c > +++ b/utils/media-ctl/options.c > @@ -22,7 +22,9 @@ > #include <getopt.h> > #include <stdio.h> > #include <stdlib.h> > +#include <string.h> > #include <unistd.h> > +#include <v4l2subdev.h> > > #include <linux/videodev2.h> > > @@ -45,7 +47,8 @@ static void usage(const char *argv0) > printf("-V, --set-v4l2 v4l2 Comma-separated list of formats to setup\n"); > printf(" --get-v4l2 pad Print the active format on a given pad\n"); > printf(" --set-dv pad Configure DV timings on a given pad\n"); > - printf("-h, --help Show verbose help and exit\n"); > + printf("-h, --help[=topic] Show verbose help and exit\n"); > + printf(" topics: mbus-fmt: List supported media bus pixel codes\n"); > printf("-i, --interactive Modify links interactively\n"); > printf("-l, --links links Comma-separated list of link descriptors to setup\n"); > printf("-p, --print-topology Print the device topology\n"); > @@ -100,7 +103,7 @@ static struct option opts[] = { > {"get-format", 1, 0, OPT_GET_FORMAT}, > {"get-v4l2", 1, 0, OPT_GET_FORMAT}, > {"set-dv", 1, 0, OPT_SET_DV}, > - {"help", 0, 0, 'h'}, > + {"help", 2, 0, 'h'}, > {"interactive", 0, 0, 'i'}, > {"links", 1, 0, 'l'}, > {"print-dot", 0, 0, OPT_PRINT_DOT}, > @@ -110,6 +113,27 @@ static struct option opts[] = { > { }, > }; > > +void list_mbus_formats(void) > +{ > + unsigned int ncodes; > + const unsigned int *code = v4l2_subdev_pixelcode_list(&ncodes); > + > + printf("Supported media bus pixel codes\n"); > + > + for (ncodes++; ncodes; ncodes--, code++) { Huh? Just do: while (ncodes--) { .... code++; } That for loop is weird. Hans > + const char *str = v4l2_subdev_pixelcode_to_string(*code); > + int spaces = 30 - (int)strlen(str); > + > + if (*code == 0) > + break; > + > + if (spaces < 0) > + spaces = 0; > + > + printf("\t%s %*c (0x%8.8x)\n", str, spaces, ' ', *code); > + } > +} > + > int parse_cmdline(int argc, char **argv) > { > int opt; > @@ -120,7 +144,8 @@ int parse_cmdline(int argc, char **argv) > } > > /* parse options */ > - while ((opt = getopt_long(argc, argv, "d:e:f:hil:prvV:", opts, NULL)) != -1) { > + while ((opt = getopt_long(argc, argv, "d:e:f:h::il:prvV:", > + opts, NULL)) != -1) { > switch (opt) { > case 'd': > media_opts.devname = optarg; > @@ -142,7 +167,16 @@ int parse_cmdline(int argc, char **argv) > break; > > case 'h': > - usage(argv[0]); > + if (optarg) { > + if (!strcmp(optarg, "mbus-fmt")) > + list_mbus_formats(); > + else > + fprintf(stderr, > + "Unknown topic \"%s\"\n", > + optarg); > + } else { > + usage(argv[0]); > + } > exit(0); > > case 'i': > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Hans, On Tue, Feb 23, 2016 at 01:18:53PM +0100, Hans Verkuil wrote: > On 02/21/16 22:29, Sakari Ailus wrote: > > Add a new topic option for -h to allow listing supported media bus codes > > in conversion functions. This is useful in figuring out which media bus > > codes are actually supported by the library. The numeric values of the > > codes are listed as well. > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > --- > > utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++++++++---- > > 1 file changed, 38 insertions(+), 4 deletions(-) > > > > diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c > > index 0afc9c2..55cdd29 100644 > > --- a/utils/media-ctl/options.c > > +++ b/utils/media-ctl/options.c > > @@ -22,7 +22,9 @@ > > #include <getopt.h> > > #include <stdio.h> > > #include <stdlib.h> > > +#include <string.h> > > #include <unistd.h> > > +#include <v4l2subdev.h> > > > > #include <linux/videodev2.h> > > > > @@ -45,7 +47,8 @@ static void usage(const char *argv0) > > printf("-V, --set-v4l2 v4l2 Comma-separated list of formats to setup\n"); > > printf(" --get-v4l2 pad Print the active format on a given pad\n"); > > printf(" --set-dv pad Configure DV timings on a given pad\n"); > > - printf("-h, --help Show verbose help and exit\n"); > > + printf("-h, --help[=topic] Show verbose help and exit\n"); > > + printf(" topics: mbus-fmt: List supported media bus pixel codes\n"); > > OK, this is ugly. It has nothing to do with usage help. > > Just make a new option --list-mbus-fmts to list supported media bus pixel > codes. > > That would make much more sense. I added it as a --help option argument in order to imply it's a part of the program's usage instructions, which is what it indeed is. It's not a list of media bus formats supported by a device. A separate option is fine, but it should be clear that it's about just listing supported formats. E.g. --list-supported-mbus-fmts. But that's a long one. Long options are loooong.
On 02/23/2016 05:11 PM, Sakari Ailus wrote: > Hi Hans, > > On Tue, Feb 23, 2016 at 01:18:53PM +0100, Hans Verkuil wrote: >> On 02/21/16 22:29, Sakari Ailus wrote: >>> Add a new topic option for -h to allow listing supported media bus codes >>> in conversion functions. This is useful in figuring out which media bus >>> codes are actually supported by the library. The numeric values of the >>> codes are listed as well. >>> >>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> >>> --- >>> utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++++++++---- >>> 1 file changed, 38 insertions(+), 4 deletions(-) >>> >>> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c >>> index 0afc9c2..55cdd29 100644 >>> --- a/utils/media-ctl/options.c >>> +++ b/utils/media-ctl/options.c >>> @@ -22,7 +22,9 @@ >>> #include <getopt.h> >>> #include <stdio.h> >>> #include <stdlib.h> >>> +#include <string.h> >>> #include <unistd.h> >>> +#include <v4l2subdev.h> >>> >>> #include <linux/videodev2.h> >>> >>> @@ -45,7 +47,8 @@ static void usage(const char *argv0) >>> printf("-V, --set-v4l2 v4l2 Comma-separated list of formats to setup\n"); >>> printf(" --get-v4l2 pad Print the active format on a given pad\n"); >>> printf(" --set-dv pad Configure DV timings on a given pad\n"); >>> - printf("-h, --help Show verbose help and exit\n"); >>> + printf("-h, --help[=topic] Show verbose help and exit\n"); >>> + printf(" topics: mbus-fmt: List supported media bus pixel codes\n"); >> >> OK, this is ugly. It has nothing to do with usage help. >> >> Just make a new option --list-mbus-fmts to list supported media bus pixel >> codes. >> >> That would make much more sense. > > I added it as a --help option argument in order to imply it's a part of the > program's usage instructions, which is what it indeed is. It's not a list of > media bus formats supported by a device. > > A separate option is fine, but it should be clear that it's about just > listing supported formats. E.g. --list-supported-mbus-fmts. But that's a > long one. Long options are loooong. --list-known-mbus-fmts will do the trick. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Hans, On Tuesday 23 February 2016 17:15:15 Hans Verkuil wrote: > On 02/23/2016 05:11 PM, Sakari Ailus wrote: > > On Tue, Feb 23, 2016 at 01:18:53PM +0100, Hans Verkuil wrote: > >> On 02/21/16 22:29, Sakari Ailus wrote: > >>> Add a new topic option for -h to allow listing supported media bus codes > >>> in conversion functions. This is useful in figuring out which media bus > >>> codes are actually supported by the library. The numeric values of the > >>> codes are listed as well. > >>> > >>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > >>> --- > >>> > >>> utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++++++---- > >>> 1 file changed, 38 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c > >>> index 0afc9c2..55cdd29 100644 > >>> --- a/utils/media-ctl/options.c > >>> +++ b/utils/media-ctl/options.c > >>> @@ -22,7 +22,9 @@ > >>> #include <getopt.h> > >>> #include <stdio.h> > >>> #include <stdlib.h> > >>> +#include <string.h> > >>> #include <unistd.h> > >>> +#include <v4l2subdev.h> > >>> > >>> #include <linux/videodev2.h> > >>> > >>> @@ -45,7 +47,8 @@ static void usage(const char *argv0) > >>> printf("-V, --set-v4l2 v4l2 Comma-separated list of formats to > >>> setup\n"); > >>> printf(" --get-v4l2 pad Print the active format on a given pad\n"); > >>> printf(" --set-dv pad Configure DV timings on a given pad\n"); > >>> - printf("-h, --help Show verbose help and exit\n"); > >>> + printf("-h, --help[=topic] Show verbose help and exit\n"); > >>> + printf(" topics: mbus-fmt: List supported media bus pixel codes\n"); > >> > >> OK, this is ugly. It has nothing to do with usage help. > >> > >> Just make a new option --list-mbus-fmts to list supported media bus pixel > >> codes. > >> > >> That would make much more sense. > > > > I added it as a --help option argument in order to imply it's a part of > > the program's usage instructions, which is what it indeed is. It's not a > > list of media bus formats supported by a device. > > > > A separate option is fine, but it should be clear that it's about just > > listing supported formats. E.g. --list-supported-mbus-fmts. But that's a > > long one. Long options are loooong. > > --list-known-mbus-fmts will do the trick. That doesn't feel right. Isn't it a help option, really, given that it lists the formats you can use as command line arguments ? Another option would actually be to always print the formats when the -h switch is given. We could print them in a comma-separated list with multiple formats per line, possibly dropping the numerical value, it should hopefully not be horrible.
Hi Laurent, On Tue, Feb 23, 2016 at 10:15:46PM +0200, Laurent Pinchart wrote: > Hi Hans, > > On Tuesday 23 February 2016 17:15:15 Hans Verkuil wrote: > > On 02/23/2016 05:11 PM, Sakari Ailus wrote: > > > On Tue, Feb 23, 2016 at 01:18:53PM +0100, Hans Verkuil wrote: > > >> On 02/21/16 22:29, Sakari Ailus wrote: > > >>> Add a new topic option for -h to allow listing supported media bus codes > > >>> in conversion functions. This is useful in figuring out which media bus > > >>> codes are actually supported by the library. The numeric values of the > > >>> codes are listed as well. > > >>> > > >>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > >>> --- > > >>> > > >>> utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++++++---- > > >>> 1 file changed, 38 insertions(+), 4 deletions(-) > > >>> > > >>> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c > > >>> index 0afc9c2..55cdd29 100644 > > >>> --- a/utils/media-ctl/options.c > > >>> +++ b/utils/media-ctl/options.c > > >>> @@ -22,7 +22,9 @@ > > >>> #include <getopt.h> > > >>> #include <stdio.h> > > >>> #include <stdlib.h> > > >>> +#include <string.h> > > >>> #include <unistd.h> > > >>> +#include <v4l2subdev.h> > > >>> > > >>> #include <linux/videodev2.h> > > >>> > > >>> @@ -45,7 +47,8 @@ static void usage(const char *argv0) > > >>> printf("-V, --set-v4l2 v4l2 Comma-separated list of formats to > > >>> setup\n"); > > >>> printf(" --get-v4l2 pad Print the active format on a given > pad\n"); > > >>> printf(" --set-dv pad Configure DV timings on a given pad\n"); > > >>> - printf("-h, --help Show verbose help and exit\n"); > > >>> + printf("-h, --help[=topic] Show verbose help and exit\n"); > > >>> + printf(" topics: mbus-fmt: List supported media bus pixel > codes\n"); > > >> > > >> OK, this is ugly. It has nothing to do with usage help. > > >> > > >> Just make a new option --list-mbus-fmts to list supported media bus pixel > > >> codes. > > >> > > >> That would make much more sense. > > > > > > I added it as a --help option argument in order to imply it's a part of > > > the program's usage instructions, which is what it indeed is. It's not a > > > list of media bus formats supported by a device. > > > > > > A separate option is fine, but it should be clear that it's about just > > > listing supported formats. E.g. --list-supported-mbus-fmts. But that's a > > > long one. Long options are loooong. > > > > --list-known-mbus-fmts will do the trick. > > That doesn't feel right. Isn't it a help option, really, given that it lists > the formats you can use as command line arguments ? > > Another option would actually be to always print the formats when the -h > switch is given. We could print them in a comma-separated list with multiple > formats per line, possibly dropping the numerical value, it should hopefully > not be horrible. I'd prefer to keep the numerical value as well; the link validation code in drivers may print the media bus code at each end in case they do not match. To debug that, it's easy to grep that from the list media-ctl prints.
Hi Sakari, On Tuesday 23 February 2016 22:24:00 Sakari Ailus wrote: > On Tue, Feb 23, 2016 at 10:15:46PM +0200, Laurent Pinchart wrote: > > On Tuesday 23 February 2016 17:15:15 Hans Verkuil wrote: > >> On 02/23/2016 05:11 PM, Sakari Ailus wrote: > >>> On Tue, Feb 23, 2016 at 01:18:53PM +0100, Hans Verkuil wrote: > >>>> On 02/21/16 22:29, Sakari Ailus wrote: > >>>>> Add a new topic option for -h to allow listing supported media bus > >>>>> codes in conversion functions. This is useful in figuring out which > >>>>> media bus codes are actually supported by the library. The numeric > >>>>> values of the codes are listed as well. > >>>>> > >>>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > >>>>> --- > >>>>> > >>>>> utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++---- > >>>>> 1 file changed, 38 insertions(+), 4 deletions(-) > >>>>> > >>>>> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c > >>>>> index 0afc9c2..55cdd29 100644 > >>>>> --- a/utils/media-ctl/options.c > >>>>> +++ b/utils/media-ctl/options.c [snip] > >>>>> @@ -45,7 +47,8 @@ static void usage(const char *argv0) > >>>>> > >>>>> printf("-V, --set-v4l2 v4l2 Comma-separated list of formats to > >>>>> setup\n"); > >>>>> printf(" --get-v4l2 pad Print the active format on a given > >>>>> pad\n"); > >>>>> printf(" --set-dv pad Configure DV timings on a given pad\n"); > >>>>> > >>>>> - printf("-h, --help Show verbose help and exit\n"); > >>>>> + printf("-h, --help[=topic] Show verbose help and exit\n"); > >>>>> + printf(" topics: mbus-fmt: List supported media bus pixel > >>>>> codes\n"); > >>>> > >>>> OK, this is ugly. It has nothing to do with usage help. > >>>> > >>>> Just make a new option --list-mbus-fmts to list supported media bus > >>>> pixel codes. > >>>> > >>>> That would make much more sense. > >>> > >>> I added it as a --help option argument in order to imply it's a part > >>> of the program's usage instructions, which is what it indeed is. It's > >>> not a list of media bus formats supported by a device. > >>> > >>> A separate option is fine, but it should be clear that it's about just > >>> listing supported formats. E.g. --list-supported-mbus-fmts. But that's > >>> a long one. Long options are loooong. > >> > >> --list-known-mbus-fmts will do the trick. > > > > That doesn't feel right. Isn't it a help option, really, given that it > > lists the formats you can use as command line arguments ? > > > > Another option would actually be to always print the formats when the -h > > switch is given. We could print them in a comma-separated list with > > multiple formats per line, possibly dropping the numerical value, it > > should hopefully not be horrible. > > I'd prefer to keep the numerical value as well; the link validation code in > drivers may print the media bus code at each end in case they do not match. > To debug that, it's easy to grep that from the list media-ctl prints. Grepping media-bus-formats.h shouldn't be difficult ;-) To shorten the output, how about printing the numerical values as 0x%04x or %04x ?
Hi Laurent, On Tue, Feb 23, 2016 at 10:30:35PM +0200, Laurent Pinchart wrote: > Hi Sakari, > > On Tuesday 23 February 2016 22:24:00 Sakari Ailus wrote: > > On Tue, Feb 23, 2016 at 10:15:46PM +0200, Laurent Pinchart wrote: > > > On Tuesday 23 February 2016 17:15:15 Hans Verkuil wrote: > > >> On 02/23/2016 05:11 PM, Sakari Ailus wrote: > > >>> On Tue, Feb 23, 2016 at 01:18:53PM +0100, Hans Verkuil wrote: > > >>>> On 02/21/16 22:29, Sakari Ailus wrote: > > >>>>> Add a new topic option for -h to allow listing supported media bus > > >>>>> codes in conversion functions. This is useful in figuring out which > > >>>>> media bus codes are actually supported by the library. The numeric > > >>>>> values of the codes are listed as well. > > >>>>> > > >>>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > >>>>> --- > > >>>>> > > >>>>> utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++---- > > >>>>> 1 file changed, 38 insertions(+), 4 deletions(-) > > >>>>> > > >>>>> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c > > >>>>> index 0afc9c2..55cdd29 100644 > > >>>>> --- a/utils/media-ctl/options.c > > >>>>> +++ b/utils/media-ctl/options.c > > [snip] > > > >>>>> @@ -45,7 +47,8 @@ static void usage(const char *argv0) > > >>>>> > > >>>>> printf("-V, --set-v4l2 v4l2 Comma-separated list of formats to > > >>>>> setup\n"); > > >>>>> printf(" --get-v4l2 pad Print the active format on a given > > >>>>> pad\n"); > > >>>>> printf(" --set-dv pad Configure DV timings on a given pad\n"); > > >>>>> > > >>>>> - printf("-h, --help Show verbose help and exit\n"); > > >>>>> + printf("-h, --help[=topic] Show verbose help and exit\n"); > > >>>>> + printf(" topics: mbus-fmt: List supported media bus pixel > > >>>>> codes\n"); > > >>>> > > >>>> OK, this is ugly. It has nothing to do with usage help. > > >>>> > > >>>> Just make a new option --list-mbus-fmts to list supported media bus > > >>>> pixel codes. > > >>>> > > >>>> That would make much more sense. > > >>> > > >>> I added it as a --help option argument in order to imply it's a part > > >>> of the program's usage instructions, which is what it indeed is. It's > > >>> not a list of media bus formats supported by a device. > > >>> > > >>> A separate option is fine, but it should be clear that it's about just > > >>> listing supported formats. E.g. --list-supported-mbus-fmts. But that's > > >>> a long one. Long options are loooong. > > >> > > >> --list-known-mbus-fmts will do the trick. > > > > > > That doesn't feel right. Isn't it a help option, really, given that it > > > lists the formats you can use as command line arguments ? > > > > > > Another option would actually be to always print the formats when the -h > > > switch is given. We could print them in a comma-separated list with > > > multiple formats per line, possibly dropping the numerical value, it > > > should hopefully not be horrible. > > > > I'd prefer to keep the numerical value as well; the link validation code in > > drivers may print the media bus code at each end in case they do not match. > > To debug that, it's easy to grep that from the list media-ctl prints. > > Grepping media-bus-formats.h shouldn't be difficult ;-) > > To shorten the output, how about printing the numerical values as 0x%04x or > %04x ? Well, yes, you can do that. But you have to have the headers available; with the numerical values printed by media-ctl you don't need any source code. I presume user space developers would appreciate that. We're not using more than 16 bits currently, I can sure change to print just four digits.
On 02/23/16 21:30, Laurent Pinchart wrote: > Hi Sakari, > > On Tuesday 23 February 2016 22:24:00 Sakari Ailus wrote: >> On Tue, Feb 23, 2016 at 10:15:46PM +0200, Laurent Pinchart wrote: >>> On Tuesday 23 February 2016 17:15:15 Hans Verkuil wrote: >>>> On 02/23/2016 05:11 PM, Sakari Ailus wrote: >>>>> On Tue, Feb 23, 2016 at 01:18:53PM +0100, Hans Verkuil wrote: >>>>>> On 02/21/16 22:29, Sakari Ailus wrote: >>>>>>> Add a new topic option for -h to allow listing supported media bus >>>>>>> codes in conversion functions. This is useful in figuring out which >>>>>>> media bus codes are actually supported by the library. The numeric >>>>>>> values of the codes are listed as well. >>>>>>> >>>>>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> >>>>>>> --- >>>>>>> >>>>>>> utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++---- >>>>>>> 1 file changed, 38 insertions(+), 4 deletions(-) >>>>>>> >>>>>>> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c >>>>>>> index 0afc9c2..55cdd29 100644 >>>>>>> --- a/utils/media-ctl/options.c >>>>>>> +++ b/utils/media-ctl/options.c > > [snip] > >>>>>>> @@ -45,7 +47,8 @@ static void usage(const char *argv0) >>>>>>> >>>>>>> printf("-V, --set-v4l2 v4l2 Comma-separated list of formats to >>>>>>> setup\n"); >>>>>>> printf(" --get-v4l2 pad Print the active format on a given >>>>>>> pad\n"); >>>>>>> printf(" --set-dv pad Configure DV timings on a given pad\n"); >>>>>>> >>>>>>> - printf("-h, --help Show verbose help and exit\n"); >>>>>>> + printf("-h, --help[=topic] Show verbose help and exit\n"); >>>>>>> + printf(" topics: mbus-fmt: List supported media bus pixel >>>>>>> codes\n"); >>>>>> >>>>>> OK, this is ugly. It has nothing to do with usage help. >>>>>> >>>>>> Just make a new option --list-mbus-fmts to list supported media bus >>>>>> pixel codes. >>>>>> >>>>>> That would make much more sense. >>>>> >>>>> I added it as a --help option argument in order to imply it's a part >>>>> of the program's usage instructions, which is what it indeed is. It's >>>>> not a list of media bus formats supported by a device. >>>>> >>>>> A separate option is fine, but it should be clear that it's about just >>>>> listing supported formats. E.g. --list-supported-mbus-fmts. But that's >>>>> a long one. Long options are loooong. >>>> >>>> --list-known-mbus-fmts will do the trick. >>> >>> That doesn't feel right. Isn't it a help option, really, given that it >>> lists the formats you can use as command line arguments ? The help arguments don't have 'options'. You could provide a --help-list-mbus-fmts, though. >>> Another option would actually be to always print the formats when the -h >>> switch is given. We could print them in a comma-separated list with >>> multiple formats per line, possibly dropping the numerical value, it >>> should hopefully not be horrible. Just always printing the list when -h is given works for me too. Regards, Hans >> >> I'd prefer to keep the numerical value as well; the link validation code in >> drivers may print the media bus code at each end in case they do not match. >> To debug that, it's easy to grep that from the list media-ctl prints. > > Grepping media-bus-formats.h shouldn't be difficult ;-) > > To shorten the output, how about printing the numerical values as 0x%04x or > %04x ? > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hans Verkuil wrote: > On 02/23/16 21:30, Laurent Pinchart wrote: >> Hi Sakari, >> >> On Tuesday 23 February 2016 22:24:00 Sakari Ailus wrote: >>> On Tue, Feb 23, 2016 at 10:15:46PM +0200, Laurent Pinchart wrote: >>>> On Tuesday 23 February 2016 17:15:15 Hans Verkuil wrote: >>>>> On 02/23/2016 05:11 PM, Sakari Ailus wrote: >>>>>> On Tue, Feb 23, 2016 at 01:18:53PM +0100, Hans Verkuil wrote: >>>>>>> On 02/21/16 22:29, Sakari Ailus wrote: >>>>>>>> Add a new topic option for -h to allow listing supported media bus >>>>>>>> codes in conversion functions. This is useful in figuring out which >>>>>>>> media bus codes are actually supported by the library. The numeric >>>>>>>> values of the codes are listed as well. >>>>>>>> >>>>>>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> >>>>>>>> --- >>>>>>>> >>>>>>>> utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++---- >>>>>>>> 1 file changed, 38 insertions(+), 4 deletions(-) >>>>>>>> >>>>>>>> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c >>>>>>>> index 0afc9c2..55cdd29 100644 >>>>>>>> --- a/utils/media-ctl/options.c >>>>>>>> +++ b/utils/media-ctl/options.c >> >> [snip] >> >>>>>>>> @@ -45,7 +47,8 @@ static void usage(const char *argv0) >>>>>>>> >>>>>>>> printf("-V, --set-v4l2 v4l2 Comma-separated list of formats to >>>>>>>> setup\n"); >>>>>>>> printf(" --get-v4l2 pad Print the active format on a given >>>>>>>> pad\n"); >>>>>>>> printf(" --set-dv pad Configure DV timings on a given pad\n"); >>>>>>>> >>>>>>>> - printf("-h, --help Show verbose help and exit\n"); >>>>>>>> + printf("-h, --help[=topic] Show verbose help and exit\n"); >>>>>>>> + printf(" topics: mbus-fmt: List supported media bus pixel >>>>>>>> codes\n"); >>>>>>> >>>>>>> OK, this is ugly. It has nothing to do with usage help. >>>>>>> >>>>>>> Just make a new option --list-mbus-fmts to list supported media bus >>>>>>> pixel codes. >>>>>>> >>>>>>> That would make much more sense. >>>>>> >>>>>> I added it as a --help option argument in order to imply it's a part >>>>>> of the program's usage instructions, which is what it indeed is. It's >>>>>> not a list of media bus formats supported by a device. >>>>>> >>>>>> A separate option is fine, but it should be clear that it's about just >>>>>> listing supported formats. E.g. --list-supported-mbus-fmts. But that's >>>>>> a long one. Long options are loooong. >>>>> >>>>> --list-known-mbus-fmts will do the trick. >>>> >>>> That doesn't feel right. Isn't it a help option, really, given that it >>>> lists the formats you can use as command line arguments ? > > The help arguments don't have 'options'. You could provide a --help-list-mbus-fmts, > though. > >>>> Another option would actually be to always print the formats when the -h >>>> switch is given. We could print them in a comma-separated list with >>>> multiple formats per line, possibly dropping the numerical value, it >>>> should hopefully not be horrible. > > Just always printing the list when -h is given works for me too. > > Regards, > > Hans > >>> >>> I'd prefer to keep the numerical value as well; the link validation code in >>> drivers may print the media bus code at each end in case they do not match. >>> To debug that, it's easy to grep that from the list media-ctl prints. >> >> Grepping media-bus-formats.h shouldn't be difficult ;-) >> >> To shorten the output, how about printing the numerical values as 0x%04x or >> %04x ? >>
Hi Hans, On Wed, Feb 24, 2016 at 04:38:58PM +0100, Hans Verkuil wrote: > On 02/23/16 21:30, Laurent Pinchart wrote: > > Hi Sakari, > > > > On Tuesday 23 February 2016 22:24:00 Sakari Ailus wrote: > >> On Tue, Feb 23, 2016 at 10:15:46PM +0200, Laurent Pinchart wrote: > >>> On Tuesday 23 February 2016 17:15:15 Hans Verkuil wrote: > >>>> On 02/23/2016 05:11 PM, Sakari Ailus wrote: > >>>>> On Tue, Feb 23, 2016 at 01:18:53PM +0100, Hans Verkuil wrote: > >>>>>> On 02/21/16 22:29, Sakari Ailus wrote: > >>>>>>> Add a new topic option for -h to allow listing supported media bus > >>>>>>> codes in conversion functions. This is useful in figuring out which > >>>>>>> media bus codes are actually supported by the library. The numeric > >>>>>>> values of the codes are listed as well. > >>>>>>> > >>>>>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > >>>>>>> --- > >>>>>>> > >>>>>>> utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++---- > >>>>>>> 1 file changed, 38 insertions(+), 4 deletions(-) > >>>>>>> > >>>>>>> diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c > >>>>>>> index 0afc9c2..55cdd29 100644 > >>>>>>> --- a/utils/media-ctl/options.c > >>>>>>> +++ b/utils/media-ctl/options.c > > > > [snip] > > > >>>>>>> @@ -45,7 +47,8 @@ static void usage(const char *argv0) > >>>>>>> > >>>>>>> printf("-V, --set-v4l2 v4l2 Comma-separated list of formats to > >>>>>>> setup\n"); > >>>>>>> printf(" --get-v4l2 pad Print the active format on a given > >>>>>>> pad\n"); > >>>>>>> printf(" --set-dv pad Configure DV timings on a given pad\n"); > >>>>>>> > >>>>>>> - printf("-h, --help Show verbose help and exit\n"); > >>>>>>> + printf("-h, --help[=topic] Show verbose help and exit\n"); > >>>>>>> + printf(" topics: mbus-fmt: List supported media bus pixel > >>>>>>> codes\n"); > >>>>>> > >>>>>> OK, this is ugly. It has nothing to do with usage help. > >>>>>> > >>>>>> Just make a new option --list-mbus-fmts to list supported media bus > >>>>>> pixel codes. > >>>>>> > >>>>>> That would make much more sense. > >>>>> > >>>>> I added it as a --help option argument in order to imply it's a part > >>>>> of the program's usage instructions, which is what it indeed is. It's > >>>>> not a list of media bus formats supported by a device. > >>>>> > >>>>> A separate option is fine, but it should be clear that it's about just > >>>>> listing supported formats. E.g. --list-supported-mbus-fmts. But that's > >>>>> a long one. Long options are loooong. > >>>> > >>>> --list-known-mbus-fmts will do the trick. > >>> > >>> That doesn't feel right. Isn't it a help option, really, given that it > >>> lists the formats you can use as command line arguments ? > > The help arguments don't have 'options'. You could provide a --help-list-mbus-fmts, > though. You could ask that from GCC. > > >>> Another option would actually be to always print the formats when the -h > >>> switch is given. We could print them in a comma-separated list with > >>> multiple formats per line, possibly dropping the numerical value, it > >>> should hopefully not be horrible. > > Just always printing the list when -h is given works for me too. Well, I'm fine with that as well.
diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c index 0afc9c2..55cdd29 100644 --- a/utils/media-ctl/options.c +++ b/utils/media-ctl/options.c @@ -22,7 +22,9 @@ #include <getopt.h> #include <stdio.h> #include <stdlib.h> +#include <string.h> #include <unistd.h> +#include <v4l2subdev.h> #include <linux/videodev2.h> @@ -45,7 +47,8 @@ static void usage(const char *argv0) printf("-V, --set-v4l2 v4l2 Comma-separated list of formats to setup\n"); printf(" --get-v4l2 pad Print the active format on a given pad\n"); printf(" --set-dv pad Configure DV timings on a given pad\n"); - printf("-h, --help Show verbose help and exit\n"); + printf("-h, --help[=topic] Show verbose help and exit\n"); + printf(" topics: mbus-fmt: List supported media bus pixel codes\n"); printf("-i, --interactive Modify links interactively\n"); printf("-l, --links links Comma-separated list of link descriptors to setup\n"); printf("-p, --print-topology Print the device topology\n"); @@ -100,7 +103,7 @@ static struct option opts[] = { {"get-format", 1, 0, OPT_GET_FORMAT}, {"get-v4l2", 1, 0, OPT_GET_FORMAT}, {"set-dv", 1, 0, OPT_SET_DV}, - {"help", 0, 0, 'h'}, + {"help", 2, 0, 'h'}, {"interactive", 0, 0, 'i'}, {"links", 1, 0, 'l'}, {"print-dot", 0, 0, OPT_PRINT_DOT}, @@ -110,6 +113,27 @@ static struct option opts[] = { { }, }; +void list_mbus_formats(void) +{ + unsigned int ncodes; + const unsigned int *code = v4l2_subdev_pixelcode_list(&ncodes); + + printf("Supported media bus pixel codes\n"); + + for (ncodes++; ncodes; ncodes--, code++) { + const char *str = v4l2_subdev_pixelcode_to_string(*code); + int spaces = 30 - (int)strlen(str); + + if (*code == 0) + break; + + if (spaces < 0) + spaces = 0; + + printf("\t%s %*c (0x%8.8x)\n", str, spaces, ' ', *code); + } +} + int parse_cmdline(int argc, char **argv) { int opt; @@ -120,7 +144,8 @@ int parse_cmdline(int argc, char **argv) } /* parse options */ - while ((opt = getopt_long(argc, argv, "d:e:f:hil:prvV:", opts, NULL)) != -1) { + while ((opt = getopt_long(argc, argv, "d:e:f:h::il:prvV:", + opts, NULL)) != -1) { switch (opt) { case 'd': media_opts.devname = optarg; @@ -142,7 +167,16 @@ int parse_cmdline(int argc, char **argv) break; case 'h': - usage(argv[0]); + if (optarg) { + if (!strcmp(optarg, "mbus-fmt")) + list_mbus_formats(); + else + fprintf(stderr, + "Unknown topic \"%s\"\n", + optarg); + } else { + usage(argv[0]); + } exit(0); case 'i':
Add a new topic option for -h to allow listing supported media bus codes in conversion functions. This is useful in figuring out which media bus codes are actually supported by the library. The numeric values of the codes are listed as well. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- utils/media-ctl/options.c | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-)