Re: [PATCH 1/2] v4l2-compliance: Add version command

[paul.elder@ideasonboard.com]

On Fri, Jul 10, 2020 at 03:33:25PM +0200, Hans Verkuil wrote:
> On 10/07/2020 15:25, Laurent Pinchart wrote:
> > Hi Paul,
> > 
> > Thank you for the patch.
> > 
> > On Fri, Jul 10, 2020 at 10:18:12PM +0900, Paul Elder wrote:
> >> Add a --version option to v4l2-compliance to retrieve the version of
> >> v4l2-compliance.
> >>
> >> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> >> ---
> >>  utils/v4l2-compliance/v4l2-compliance.cpp | 11 +++++++++++
> >>  1 file changed, 11 insertions(+)
> >>
> >> diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp
> >> index 4b45f110..72b9768f 100644
> >> --- a/utils/v4l2-compliance/v4l2-compliance.cpp
> >> +++ b/utils/v4l2-compliance/v4l2-compliance.cpp
> >> @@ -79,6 +79,7 @@ enum Option {
> >>  	OptMediaBusInfo = 'z',
> >>  	OptStreamFrom = 128,
> >>  	OptStreamFromHdr,
> >> +	OptVersion,
> >>  	OptLast = 256
> >>  };
> >>  
> >> @@ -153,9 +154,15 @@ static struct option long_options[] = {
> >>  	{"stream-all-formats", optional_argument, 0, OptStreamAllFormats},
> >>  	{"stream-all-io", no_argument, 0, OptStreamAllIO},
> >>  	{"stream-all-color", required_argument, 0, OptStreamAllColorTest},
> >> +	{"version", no_argument, 0, OptVersion},
> >>  	{0, 0, 0, 0}
> >>  };
> >>  
> >> +static void version()
> >> +{
> >> +	printf("v4l2-compliance " PACKAGE_VERSION "\n");
> > 
> > Is it enough to rely on the v4l-utils package version, or should we add
> > a git commit count as well ? The traditional version number will make it
> > difficult to test for features added between two released versions.

Yeah, it might be useful.

> If you add a version option, then v4l2-compliance should also show the SHA.
> It's already available (grep for SHA), so easy enough to add here.

Oh yeah we could use that.

> Also, if you add --version here, then it really should be added to most
> other utils as well (certainly media-ctl and cec-follower/ctl/compliance).

Okay, I can add that.

For v4l2-ctl and the other tools, would it be better like:

v4l2-ctl 1.21.0-deadbeef

Or like what v4l2-compliance has:

v4l2-compliance SHA: 3b22ab02b960e4d1e90618e9fce9b7c8a80d814a, 64 bits, 64-bit time_t

v4l2-compliance 1.21.0





Thanks,

Paul

> >> +}
> >> +
> >>  static void usage()
> >>  {
> >>  	printf("Usage:\n");
> >> @@ -244,6 +251,7 @@ static void usage()
> >>  	printf("  -P, --no-progress  Turn off progress messages.\n");
> >>  	printf("  -T, --trace        Trace all called ioctls.\n");
> >>  	printf("  -v, --verbose      Turn on verbose reporting.\n");
> >> +	printf("  --version          Show version information.\n");
> >>  #ifndef NO_LIBV4L2
> >>  	printf("  -w, --wrapper      Use the libv4l2 wrapper library.\n");
> >>  #endif
> >> @@ -1664,6 +1672,9 @@ int main(int argc, char **argv)
> >>  		case OptNoProgress:
> >>  			no_progress = true;
> >>  			break;
> >> +		case OptVersion:
> >> +			version();
> >> +			std::exit(EXIT_SUCCESS);
> >>  		case ':':
> >>  			fprintf(stderr, "Option `%s' requires a value\n",
> >>  				argv[optind]);
> > 
>