Hi Laurent,
Laurent Pinchart wrote:
> Hi Sakari,
>
> Thanks for the patch.
And thank you for the review!!!
> On Monday 20 February 2012 03:56:43 Sakari Ailus wrote:
>> Add support for VIDIOC_SUBDEV_S_SELECTION and VIDIOC_SUBDEV_G_SELECTION
>> IOCTLs. They replace functionality provided by VIDIOC_SUBDEV_S_CROP and
>> VIDIOC_SUBDEV_G_CROP IOCTLs and also add new functionality (composing).
>>
>> VIDIOC_SUBDEV_G_CROP and VIDIOC_SUBDEV_S_CROP continue to be supported.
>>
>> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxx>
>> ---
>> drivers/media/video/v4l2-subdev.c | 34 +++++++++++++++++++++---------
>> include/linux/v4l2-subdev.h | 41
>> +++++++++++++++++++++++++++++++++++++ include/media/v4l2-subdev.h |
>> 21 +++++++++++++++---
>> 3 files changed, 82 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/media/video/v4l2-subdev.c
>> b/drivers/media/video/v4l2-subdev.c index 6fe88e9..30c7fd9 100644
>> --- a/drivers/media/video/v4l2-subdev.c
>> +++ b/drivers/media/video/v4l2-subdev.c
>> @@ -35,14 +35,9 @@
>> static int subdev_fh_init(struct v4l2_subdev_fh *fh, struct v4l2_subdev
>> *sd) {
>> #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
>> - /* Allocate try format and crop in the same memory block */
>> - fh->try_fmt = kzalloc((sizeof(*fh->try_fmt) + sizeof(*fh->try_crop))
>> - * sd->entity.num_pads, GFP_KERNEL);
>> - if (fh->try_fmt == NULL)
>> + fh->pad = kzalloc(sizeof(*fh->pad) * sd->entity.num_pads, GFP_KERNEL);
>> + if (fh->pad == NULL)
>> return -ENOMEM;
>> -
>> - fh->try_crop = (struct v4l2_rect *)
>> - (fh->try_fmt + sd->entity.num_pads);
>> #endif
>> return 0;
>> }
>> @@ -50,9 +45,8 @@ static int subdev_fh_init(struct v4l2_subdev_fh *fh,
>> struct v4l2_subdev *sd) static void subdev_fh_free(struct v4l2_subdev_fh
>> *fh)
>> {
>> #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
>> - kfree(fh->try_fmt);
>> - fh->try_fmt = NULL;
>> - fh->try_crop = NULL;
>> + kfree(fh->pad);
>> + fh->pad = NULL;
>> #endif
>> }
>>
>> @@ -293,6 +287,26 @@ static long subdev_do_ioctl(struct file *file, unsigned
>> int cmd, void *arg) return v4l2_subdev_call(sd, pad, enum_frame_interval,
>> subdev_fh, fie);
>> }
>> +
>> + case VIDIOC_SUBDEV_G_SELECTION: {
>> + struct v4l2_subdev_selection *sel = arg;
>> +
>> + if (sel->pad >= sd->entity.num_pads)
>> + return -EINVAL;
>
> Shouldn't you verify the which field as well, as done for the crop ioctls ?
I thought I got a comment on that and fixed it, but apparently not. :-P
Thanks for pointing this out.
>> +
>> + return v4l2_subdev_call(
>> + sd, pad, get_selection, subdev_fh, sel);
>> + }
>> +
>> + case VIDIOC_SUBDEV_S_SELECTION: {
>> + struct v4l2_subdev_selection *sel = arg;
>> +
Fixed it here as well...
>> + if (sel->pad >= sd->entity.num_pads)
>> + return -EINVAL;
>> +
>> + return v4l2_subdev_call(
>> + sd, pad, set_selection, subdev_fh, sel);
>> + }
>> #endif
>> default:
>> return v4l2_subdev_call(sd, core, ioctl, cmd, arg);
>> diff --git a/include/linux/v4l2-subdev.h b/include/linux/v4l2-subdev.h
>> index ed29cbb..6c84390 100644
>> --- a/include/linux/v4l2-subdev.h
>> +++ b/include/linux/v4l2-subdev.h
>> @@ -123,6 +123,43 @@ struct v4l2_subdev_frame_interval_enum {
>> __u32 reserved[9];
>> };
>>
>> +#define V4L2_SUBDEV_SEL_FLAG_SIZE_GE (1 << 0)
>> +#define V4L2_SUBDEV_SEL_FLAG_SIZE_LE (1 << 1)
>> +#define V4L2_SUBDEV_SEL_FLAG_KEEP_CONFIG (1 << 2)
>> +
>> +/* active cropping area */
>> +#define V4L2_SUBDEV_SEL_TGT_CROP_ACTIVE 0x0000
>> +/* cropping bounds */
>> +#define V4L2_SUBDEV_SEL_TGT_CROP_BOUNDS 0x0002
>> +/* current composing area */
>> +#define V4L2_SUBDEV_SEL_TGT_COMPOSE_ACTIVE 0x0100
>> +/* composing bounds */
>> +#define V4L2_SUBDEV_SEL_TGT_COMPOSE_BOUNDS 0x0102
>> +
>> +
>> +/**
>> + * struct v4l2_subdev_selection - selection info
>> + *
>> + * @which: either V4L2_SUBDEV_FORMAT_ACTIVE or V4L2_SUBDEV_FORMAT_TRY
>> + * @pad: pad number, as reported by the media API
>> + * @target: selection target, used to choose one of possible rectangles
>> + * @flags: constraint flags
>> + * @r: coordinates of the selection window
>> + * @reserved: for future use, rounds structure size to 64 bytes, set to
>> zero
>
> I'm not sure we need to mention that the reserved fields round the structure
> size to 64 bytes.
Fixed.
>> + *
>> + * Hardware may use multiple helper windows to process a video stream.
>> + * The structure is used to exchange this selection areas between
>> + * an application and a driver.
>> + */
>> +struct v4l2_subdev_selection {
>> + __u32 which;
>> + __u32 pad;
>> + __u32 target;
>> + __u32 flags;
>> + struct v4l2_rect r;
>> + __u32 reserved[8];
>> +};
>> +
>> #define VIDIOC_SUBDEV_G_FMT _IOWR('V', 4, struct v4l2_subdev_format)
>> #define VIDIOC_SUBDEV_S_FMT _IOWR('V', 5, struct v4l2_subdev_format)
>> #define VIDIOC_SUBDEV_G_FRAME_INTERVAL \
>> @@ -137,5 +174,9 @@ struct v4l2_subdev_frame_interval_enum {
>> _IOWR('V', 75, struct v4l2_subdev_frame_interval_enum)
>> #define VIDIOC_SUBDEV_G_CROP _IOWR('V', 59, struct v4l2_subdev_crop)
>> #define VIDIOC_SUBDEV_S_CROP _IOWR('V', 60, struct v4l2_subdev_crop)
>> +#define VIDIOC_SUBDEV_G_SELECTION \
>> + _IOWR('V', 61, struct v4l2_subdev_selection)
>> +#define VIDIOC_SUBDEV_S_SELECTION \
>> + _IOWR('V', 62, struct v4l2_subdev_selection)
>>
>> #endif
>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>> index f0f3358..feab950 100644
>> --- a/include/media/v4l2-subdev.h
>> +++ b/include/media/v4l2-subdev.h
>> @@ -466,6 +466,10 @@ struct v4l2_subdev_pad_ops {
>> struct v4l2_subdev_crop *crop);
>> int (*get_crop)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
>> struct v4l2_subdev_crop *crop);
>> + int (*get_selection)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
>> + struct v4l2_subdev_selection *sel);
>> + int (*set_selection)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
>> + struct v4l2_subdev_selection *sel);
>> };
>>
>> struct v4l2_subdev_ops {
>> @@ -549,8 +553,11 @@ struct v4l2_subdev {
>> struct v4l2_subdev_fh {
>> struct v4l2_fh vfh;
>> #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
>> - struct v4l2_mbus_framefmt *try_fmt;
>> - struct v4l2_rect *try_crop;
>> + struct {
>> + struct v4l2_mbus_framefmt try_fmt;
>> + struct v4l2_rect try_crop;
>> + struct v4l2_rect try_compose;
>> + } *pad;
>> #endif
>> };
>>
>> @@ -561,13 +568,19 @@ struct v4l2_subdev_fh {
>> static inline struct v4l2_mbus_framefmt *
>> v4l2_subdev_get_try_format(struct v4l2_subdev_fh *fh, unsigned int pad)
>> {
>> - return &fh->try_fmt[pad];
>> + return &fh->pad[pad].try_fmt;
>> }
>>
>> static inline struct v4l2_rect *
>> v4l2_subdev_get_try_crop(struct v4l2_subdev_fh *fh, unsigned int pad)
>> {
>> - return &fh->try_crop[pad];
>> + return &fh->pad[pad].try_crop;
>> +}
>> +
>> +static inline struct v4l2_rect *
>> +v4l2_subdev_get_try_compose(struct v4l2_subdev_fh *fh, unsigned int pad)
>> +{
>> + return &fh->pad[pad].try_compose;
>> }
>> #endif
--
Sakari Ailus
sakari.ailus@xxxxxx
--
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
