Hi Hans,
Thanks for pointing it out.
On Mon, Sep 30, 2013 at 1:38 PM, Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
> Hi Shaik,
>
> I have a few questions regarding the selection part...
>
> On 09/12/2013 03:09 PM, Shaik Ameer Basha wrote:
>> This patch adds the Makefile and memory to memory (m2m) interface
>> functionality for the SCALER driver.
>>
>> Signed-off-by: Shaik Ameer Basha <shaik.ameer@xxxxxxxxxxx>
>> ---
>> drivers/media/platform/Kconfig | 8 +
>> drivers/media/platform/Makefile | 1 +
>> drivers/media/platform/exynos-scaler/Makefile | 3 +
>> drivers/media/platform/exynos-scaler/scaler-m2m.c | 781 +++++++++++++++++++++
>> 4 files changed, 793 insertions(+)
>> create mode 100644 drivers/media/platform/exynos-scaler/Makefile
>> create mode 100644 drivers/media/platform/exynos-scaler/scaler-m2m.c
>>
>
[...]
>> +
>> +static int scaler_m2m_s_selection(struct file *file, void *fh,
>> + struct v4l2_selection *s)
>> +{
>> + struct scaler_frame *frame;
>> + struct scaler_ctx *ctx = fh_to_ctx(fh);
>> + struct v4l2_crop cr;
>> + struct scaler_variant *variant = ctx->scaler_dev->variant;
>> + int ret;
>> +
>> + cr.type = s->type;
>> + cr.c = s->r;
>> +
>> + if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) &&
>> + (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE))
>> + return -EINVAL;
>> +
>> + ret = scaler_try_crop(ctx, &cr);
>> + if (ret < 0)
>> + return ret;
>> +
>> + if (s->flags & V4L2_SEL_FLAG_LE &&
>> + !is_rectangle_enclosed(&cr.c, &s->r))
>> + return -ERANGE;
>> +
>> + if (s->flags & V4L2_SEL_FLAG_GE &&
>> + !is_rectangle_enclosed(&s->r, &cr.c))
>> + return -ERANGE;
>> +
>> + s->r = cr.c;
>> +
>> + switch (s->target) {
>> + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
>> + case V4L2_SEL_TGT_COMPOSE_DEFAULT:
>> + case V4L2_SEL_TGT_COMPOSE:
>> + frame = &ctx->s_frame;
>> + break;
>> +
>> + case V4L2_SEL_TGT_CROP_BOUNDS:
>> + case V4L2_SEL_TGT_CROP:
>> + case V4L2_SEL_TGT_CROP_DEFAULT:
>> + frame = &ctx->d_frame;
>> + break;
>
> Similar problems as with g_selection above. Tomasz mentioned to me that the selection
> API is not implemented correctly in m2m Samsung drivers. It looks like this code is
> copied-and-pasted from other drivers, so it seems he was right.
Sorry, after going through the documentation, I have to agree with you...
As you mentioned, this part of the code was copied while implementing
the G-Scaler driver :)
I will change the above implementation for M2M devices (GScaler and
SCALER) as below,
I will only allow all V4L2_SEL_TGT_COMPOSE_* target requests if
's->type' is equal to "V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE".
and all V4L2_SEL_TGT_CROP_* target requests if 's->type' is equal to
"V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE".
I hope with the above two checkings taken in to care, there should not
be any issues with using selection APIs here.
Thanks,
Shaik Ameer Basha
>
> The selection API for m2m devices will be discussed during the upcoming V4L2 mini-summit
> since the API may actually need some adjustments to have it work the way it should.
>
> As requested above, if you can explain the exact functionality you are trying to
> implement here, then I can look over this code carefully and see how it should be done.
>
> Thanks!
>
> Hans
>
[...]
--
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
