Hi Hans,
On Fri, Jan 24, 2014 at 12:19:30PM +0100, Hans Verkuil wrote:
> On 01/24/2014 11:35 AM, Sakari Ailus wrote:
> > Hi Hans,
> >
> > Thanks for the patchset!
> >
> > On Mon, Jan 20, 2014 at 01:45:55PM +0100, Hans Verkuil wrote:
> >> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> >> index 0b347e8..3998049 100644
> >> --- a/include/media/v4l2-ctrls.h
> >> +++ b/include/media/v4l2-ctrls.h
> >> @@ -85,6 +85,7 @@ typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl *ctrl, void *priv);
> >> * @ops: The control ops.
> >> * @id: The control ID.
> >> * @name: The control name.
> >> + * @unit: The control's unit. May be NULL.
> >> * @type: The control type.
> >> * @minimum: The control's minimum value.
> >> * @maximum: The control's maximum value.
> >> @@ -130,6 +131,7 @@ struct v4l2_ctrl {
> >> const struct v4l2_ctrl_ops *ops;
> >> u32 id;
> >> const char *name;
> >> + const char *unit;
> >
> > What would you think of using a numeric value (with the standardised units
> > #defined)? I think using a string begs for unmanaged unit usage. Code that
> > deals with units might work with one driver but not with another since it
> > uses a slightly different string for unit x.
>
> First of all, you always need a string. You don't want GUIs like qv4l2 to have
> to switch on a unit in order to generate the unit strings. That's impossible to
> keep up to date.
That's true when when you want to show that to the user, yes. But when you
have an application which tries to figure out which value to put to the
control, a numeric value is more convenient.
Kernel interfaces seldom use strings and that's for a good reason.
> In addition, private controls can have really strange custom units, so you want
> to have a string there as well.
Good point as well.
> Standard controls can have their unit string set in v4l2-ctrls.c, just as their
> name is set there these days, thus ensuring consistency.
>
> What I had in mind is that videodev2.h defines a list of standardized unit strings,
> e.g.:
>
> #define V4L2_CTRL_UNIT_USECS "usecs"
> #define V4L2_CTRL_UNIT_MSECS "msecs"
That's possible as well, but requires the user to e.g. use if (strcmp())
... instead of just plain switch (unit) { ... }.
I'd also very much prefer to stick to SI units and prefixes where applicable
if we end up using strings. Combining the unit and prefix could make sense.
> and apps can do strcmp(qc->unit, V4L2_CTRL_UNIT_USECS) to see what the unit is.
> If a driver doesn't use one of those standardized unit strings, then it is a
> driver bug.
>
> > A prefix could be potentially nice, too, so ms and µs would still have the
> > same unit but a different prefix.
>
> Can you give an example of a prefix? I don't really follow what you want to
> achieve.
You use them in your own example above. :-)
<URL:http://en.wikipedia.org/wiki/SI_prefix>
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
