Re: [raw2rgbpnm PATCH v4 04/11] Support long options and improve help text

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Tue, Mar 17, 2026 at 11:49:14AM +0200, Sakari Ailus wrote:
> Moi,
> 
> On Mon, Mar 16, 2026 at 04:41:51PM +0200, Laurent Pinchart wrote:
> > On Tue, Mar 10, 2026 at 10:46:10AM +0200, Sakari Ailus wrote:
> > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > ---
> > >  raw2rgbpnm.c | 34 +++++++++++++++++++++++++---------
> > >  1 file changed, 25 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/raw2rgbpnm.c b/raw2rgbpnm.c
> > > index 14cfa7e7a46c..16ef64e984b9 100644
> > > --- a/raw2rgbpnm.c
> > > +++ b/raw2rgbpnm.c
> > > @@ -24,6 +24,7 @@
> > >   */
> > >  
> > >  #include <ctype.h>
> > > +#include <getopt.h>
> > >  #include <stdio.h>
> > >  #include <stdint.h>
> > >  #include <stdlib.h>
> > > @@ -836,6 +837,17 @@ static int parse_format(const char *p, int *w, int *h)
> > >  	return 0;
> > >  }
> > >  
> > > +static struct option options[] = {
> > > +	{ "algo", required_argument, NULL, 'a', },
> > > +	{ "brightness", required_argument, NULL, 'b', },
> > > +	{ "format", required_argument, NULL, 'f', },
> > > +	{ "help", no_argument, NULL, 'h', },
> > > +	{ "high-bits", no_argument, NULL, 'g', },
> > > +	{ "size", required_argument, NULL, 's', },
> > > +	{ "swap-rb", no_argument, NULL, 'w', },
> > > +	{ 0 },
> > > +};
> > > +
> > >  int main(int argc, char *argv[])
> > >  {
> > >  	FILE *f;
> > > @@ -849,7 +861,7 @@ int main(int argc, char *argv[])
> > >  	int width = -1;
> > >  
> > >  	for (;;) {
> > > -		int c = getopt(argc, argv, "a:b:f:ghs:w");
> > > +		int c = getopt_long(argc, argv, "a:b:f:ghs:w", options, NULL);
> > >  		if (c==-1) break;
> > >  		switch (c) {
> > >  		case 'a':
> > > @@ -889,14 +901,18 @@ int main(int argc, char *argv[])
> > >  			break;
> > >  		case 'h':
> > >  			printf("%s - Convert headerless raw image to RGB file (PNM)\n"
> > > -			       "Usage: %s [-h] [-w] [-s XxY] <inputfile> <outputfile>\n"
> > > -			       "-a <algo>     Select algorithm, use \"-a ?\" for a list\n"
> > > -			       "-b <bright>   Set brightness (multiplier) to output image (float, default 1.0)\n"
> > > -			       "-f <format>   Specify input file format format (-f ? for list, default UYVY)\n"
> > > -			       "-g            Use high bits for Bayer RAW 10 data\n"
> > > -			       "-h            Show this help\n"
> > > -			       "-s <XxY>      Specify image size\n"
> > > -			       "-w            Swap R and B channels\n", progname, argv[0]);
> > > +			       "Usage: %s [--algo|-a <algo>] [--brightness|-b <brightness>]\n"
> > > +			       "       [--format|-f <format>] [--help|-h] [--high-bits|-g] [--size|-s XxY]\n"
> > > +			       "       [--swap-rb|-w] <inputfile> <outputfile>\n\n"
> > 
> > That won't scale nicely, but I don't want to implement an options
> > handling framework in C right now :-)
> > 
> > > +			       "--algo, -a <algo>         Select algorithm, use \"-a ?\" for a list\n"
> > > +			       "--brightness, -b <bright> Set brightness (multiplier) to output image\n"
> > > +			       "                          (float, default 1.0)\n"
> > 
> > Do you know if this option is used by anyone ? It seems a bit out of
> > place. Not an issue with this patch of course.
> 
> I have no idea. But keeping it won't do any harm, will it?

It's just an opportunity for cleaning up an unused feature :-) It looks
out of place, as nothing else in raw2rgbpnm performs post-processing on
the converted image (and rightfully so).

> > > +			       "--format, -f <format>     Specify input file format format\n"
> > > +			       "                          (-f ? for list, default UYVY)\n"
> > > +			       "--help, -h                Show this help\n"
> > > +			       "--high-bits, -g           Use high bits for Bayer RAW 10 data\n"
> > > +			       "--size, -s <XxY>          Specify image size\n"
> > > +			       "--swap-rb, -w             Swap R and B channels\n", progname, argv[0]);
> > 
> > I'd add a line break before progname.
> 
> Sounds good.
> 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > >  			exit(0);
> > >  		case 's':
> > >  			if (parse_format(optarg, &width, &height) < 0) {

-- 
Regards,

Laurent Pinchart