On 10/07/2020 15:51, Laurent Pinchart wrote:
> Hi Hans,
>
> On Fri, Jul 10, 2020 at 03:33:25PM +0200, Hans Verkuil wrote:
>> On 10/07/2020 15:25, Laurent Pinchart wrote:
>>> 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@xxxxxxxxxxxxxxxx>
>>>> ---
>>>> 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.
>>
>> 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.
>
> The issue with the SHA is that, while it identifies the exact commit, it
> is useless to compare versions. We are using v4l2-compliance to test the
> libcamera V4L2 compatibility layer, and this depends on recent features
> merged in the master branch but not available in a release yet. We would
> like the test to be skipped if the v4l2-compliance is too old. Printing
> the package version is a good step forward, but would require waiting
> for the next release before the test can be enabled. That's probably OK
> overall, but it's a bit annoying during development. That's why I was
> wondering if a commit count (as output by git rev-list --count HEAD)
> would be useful too. In our case, the fact that v4l2-compliance supports
> the --version option will be enough to know it's recent enough, but I'm
> thinking about the future (for libcamera and other users).
That would work, then you would get this:
v4l2-compliance 1.21.0-4606
v4l2-compliance SHA: 3b22ab02b960e4d1e90618e9fce9b7c8a80d814a, 64 bits, 64-bit time_t
cec-compliance 1.21.0-4604
cec-compliance SHA: 3b22ab02b960e4d1e90618e9fce9b7c8a80d814a
I still want the SHA, though :-)
The problem with the commit count is that someone can fork v4l-utils and apply
its own patches. So the count does not sufficiently identify the version.
It's no doubt fine for libcamera, but for getting a new driver into mainline
I must know the exact version that is used for the compliance test.
Regards,
Hans
>
>> 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).
>>
>>>> +}
>>>> +
>>>> 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]);
>
