Hi Hans,
On Mon, Jan 20, 2014 at 01:46:01PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>
> Since complex controls can have non-standard types we need to be able to do
> type-specific checks etc. In order to make that easy type operations are added.
> There are four operations:
>
> - equal: check if two values are equal
> - init: initialize a value
> - log: log the value
> - validate: validate a new value
>
> This patch uses the v4l2_ctrl_ptr union for the first time.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> ---
> drivers/media/v4l2-core/v4l2-ctrls.c | 267 ++++++++++++++++++++++-------------
> include/media/v4l2-ctrls.h | 21 +++
> 2 files changed, 190 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 98e940f..9f97af4 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -1123,6 +1123,149 @@ static void send_event(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl, u32 changes)
> v4l2_event_queue_fh(sev->fh, &ev);
> }
>
> +static bool std_equal(const struct v4l2_ctrl *ctrl,
> + union v4l2_ctrl_ptr ptr1,
> + union v4l2_ctrl_ptr ptr2)
> +{
> + switch (ctrl->type) {
> + case V4L2_CTRL_TYPE_BUTTON:
> + return false;
> + case V4L2_CTRL_TYPE_STRING:
> + /* strings are always 0-terminated */
> + return !strcmp(ptr1.p_char, ptr2.p_char);
> + case V4L2_CTRL_TYPE_INTEGER64:
> + return *ptr1.p_s64 == *ptr2.p_s64;
The above two lines seem redundant to me.
> + default:
> + if (ctrl->is_ptr)
> + return !memcmp(ptr1.p, ptr2.p, ctrl->elem_size);
> + return *ptr1.p_s32 == *ptr2.p_s32;
> + }
> +}
> +
> +static void std_init(const struct v4l2_ctrl *ctrl,
> + union v4l2_ctrl_ptr ptr)
> +{
> + switch (ctrl->type) {
> + case V4L2_CTRL_TYPE_STRING:
> + memset(ptr.p_char, ' ', ctrl->minimum);
> + ptr.p_char[ctrl->minimum] = '\0';
> + break;
> + case V4L2_CTRL_TYPE_INTEGER64:
> + *ptr.p_s64 = ctrl->default_value;
> + break;
> + case V4L2_CTRL_TYPE_INTEGER:
> + case V4L2_CTRL_TYPE_INTEGER_MENU:
> + case V4L2_CTRL_TYPE_MENU:
> + case V4L2_CTRL_TYPE_BITMASK:
> + case V4L2_CTRL_TYPE_BOOLEAN:
> + *ptr.p_s32 = ctrl->default_value;
> + break;
> + default:
> + break;
> + }
> +}
> +
> +static void std_log(const struct v4l2_ctrl *ctrl)
> +{
> + union v4l2_ctrl_ptr ptr = ctrl->stores[0];
> +
> + switch (ctrl->type) {
> + case V4L2_CTRL_TYPE_INTEGER:
> + pr_cont("%d", *ptr.p_s32);
> + break;
> + case V4L2_CTRL_TYPE_BOOLEAN:
> + pr_cont("%s", *ptr.p_s32 ? "true" : "false");
> + break;
> + case V4L2_CTRL_TYPE_MENU:
> + pr_cont("%s", ctrl->qmenu[*ptr.p_s32]);
> + break;
> + case V4L2_CTRL_TYPE_INTEGER_MENU:
> + pr_cont("%lld", ctrl->qmenu_int[*ptr.p_s32]);
> + break;
> + case V4L2_CTRL_TYPE_BITMASK:
> + pr_cont("0x%08x", *ptr.p_s32);
> + break;
> + case V4L2_CTRL_TYPE_INTEGER64:
> + pr_cont("%lld", *ptr.p_s64);
> + break;
> + case V4L2_CTRL_TYPE_STRING:
> + pr_cont("%s", ptr.p_char);
> + break;
> + default:
> + pr_cont("unknown type %d", ctrl->type);
> + break;
> + }
> +}
> +
> +/* Round towards the closest legal value */
> +#define ROUND_TO_RANGE(val, offset_type, ctrl) \
> +({ \
> + offset_type offset; \
> + val += (ctrl)->step / 2; \
> + val = clamp_t(typeof(val), val, \
> + (ctrl)->minimum, (ctrl)->maximum); \
> + offset = (val) - (ctrl)->minimum; \
> + offset = (ctrl)->step * (offset / (ctrl)->step); \
> + val = (ctrl)->minimum + offset; \
> + 0; \
> +})
Could you use an inline function instead? This doesn't really need to be a
macro, albeit I admit that it's always cool to express one's ability to
write GCC-only macros. :-D
(I just noticed that this patch just moves the macro to a different place,
but I think it was added by an earlier patch in the set.)
--
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
