Message ID | 20200913182140.32466-1-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
---|---|
State | Changes Requested, archived |
Headers |
Received: from vger.kernel.org ([23.128.96.18]) by www.linuxtv.org with esmtp (Exim 4.92) (envelope-from <linux-media-owner@vger.kernel.org>) id 1kHWXS-008O8N-Ah; Sun, 13 Sep 2020 18:15:47 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725943AbgIMSVu (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Sun, 13 Sep 2020 14:21:50 -0400 Received: from relmlor2.renesas.com ([210.160.252.172]:11279 "EHLO relmlie6.idc.renesas.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1725936AbgIMSVs (ORCPT <rfc822;linux-media@vger.kernel.org>); Sun, 13 Sep 2020 14:21:48 -0400 X-IronPort-AV: E=Sophos;i="5.76,422,1592838000"; d="scan'208";a="56924822" Received: from unknown (HELO relmlir6.idc.renesas.com) ([10.200.68.152]) by relmlie6.idc.renesas.com with ESMTP; 14 Sep 2020 03:21:46 +0900 Received: from localhost.localdomain (unknown [10.226.36.204]) by relmlir6.idc.renesas.com (Postfix) with ESMTP id 2247940B696E; Mon, 14 Sep 2020 03:21:43 +0900 (JST) From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> To: =?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>, Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Hans Verkuil <hverkuil-cisco@xs4all.nl>, Mauro Carvalho Chehab <mchehab@kernel.org>, Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>, linux-media@vger.kernel.org, linux-renesas-soc@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Biju Das <biju.das.jz@bp.renesas.com>, Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>, Prabhakar <prabhakar.csengg@gmail.com> Subject: [PATCH v2] media: rcar-vin: Update crop and compose settings for every s_fmt call Date: Sun, 13 Sep 2020 19:21:40 +0100 Message-Id: <20200913182140.32466-1-prabhakar.mahadev-lad.rj@bp.renesas.com> X-Mailer: git-send-email 2.17.1 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-LSpam-Score: -2.4 (--) X-LSpam-Report: No, score=-2.4 required=5.0 tests=BAYES_00=-1.9,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1 autolearn=ham autolearn_force=no |
Series |
[v2] media: rcar-vin: Update crop and compose settings for every s_fmt call
|
|
Commit Message
Prabhakar Mahadev Lad
Sept. 13, 2020, 6:21 p.m. UTC
The crop and compose settings for VIN in non mc mode werent updated in s_fmt call this resulted in captured images being clipped. With the below sequence on the third capture where size is set to 640x480 resulted in clipped image of size 320x240. high(640x480) -> low (320x240) -> high (640x480) This patch makes sure the VIN crop and compose settings are updated. Fixes: 104464f573d ("media: rcar-vin: Do not reset the crop and compose rectangles in s_fmt") Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> --- Changes for v2: * Dropped redundant code mapping crop and compose rects v1 - https://lkml.org/lkml/2020/7/31/364 --- drivers/media/platform/rcar-vin/rcar-v4l2.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
Comments
Hi Prabhakar, Thank you for the patch. On Sun, Sep 13, 2020 at 07:21:40PM +0100, Lad Prabhakar wrote: > The crop and compose settings for VIN in non mc mode werent updated s/mc/MC/ s/werent/weren't/ > in s_fmt call this resulted in captured images being clipped. > > With the below sequence on the third capture where size is set to > 640x480 resulted in clipped image of size 320x240. > > high(640x480) -> low (320x240) -> high (640x480) > > This patch makes sure the VIN crop and compose settings are updated. > > Fixes: 104464f573d ("media: rcar-vin: Do not reset the crop and compose rectangles in s_fmt") > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > Changes for v2: > * Dropped redundant code mapping crop and compose rects > > v1 - https://lkml.org/lkml/2020/7/31/364 > --- > drivers/media/platform/rcar-vin/rcar-v4l2.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c > index 0e066bba747e..1bd59a8453b4 100644 > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c > @@ -305,7 +305,7 @@ static int rvin_s_fmt_vid_cap(struct file *file, void *priv, > struct v4l2_format *f) > { > struct rvin_dev *vin = video_drvdata(file); > - struct v4l2_rect fmt_rect, src_rect; > + struct v4l2_rect src_rect; > int ret; > > if (vb2_is_busy(&vin->queue)) > @@ -317,14 +317,11 @@ static int rvin_s_fmt_vid_cap(struct file *file, void *priv, > return ret; > > vin->format = f->fmt.pix; > - > - fmt_rect.top = 0; > - fmt_rect.left = 0; > - fmt_rect.width = vin->format.width; > - fmt_rect.height = vin->format.height; > - > - v4l2_rect_map_inside(&vin->crop, &src_rect); > - v4l2_rect_map_inside(&vin->compose, &fmt_rect); I would add a comment here: /* Reset the crop and compose rectangles to default values. */ Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + vin->crop.top = 0; > + vin->crop.left = 0; > + vin->crop.width = vin->format.width; > + vin->crop.height = vin->format.height; > + vin->compose = vin->crop; > vin->src_rect = src_rect; > > return 0;
Hi Lad, Thanks for your work. On 2020-09-13 19:21:40 +0100, Lad Prabhakar wrote: > The crop and compose settings for VIN in non mc mode werent updated > in s_fmt call this resulted in captured images being clipped. > > With the below sequence on the third capture where size is set to > 640x480 resulted in clipped image of size 320x240. > > high(640x480) -> low (320x240) -> high (640x480) > > This patch makes sure the VIN crop and compose settings are updated. With this change I get failures with v4l2-compliance I don't have before. I think we need to align this the other way around, copy the non-MC behavior to the MC S_FTM implementation. # v4l2-compliance -d /dev/video28 v4l2-compliance SHA: c7f03287bbd64c168975e7ff3192e6fd3b507686, 32 bits, 32-bit time_t Compliance test for rcar_vin device /dev/video28: Driver Info: Driver name : rcar_vin Card type : R_Car_VIN Bus info : platform:e6ef1000.video Driver version : 5.9.0 Capabilities : 0x85200001 Video Capture Read/Write Streaming Extended Pix Format Device Capabilities Device Caps : 0x05200001 Video Capture Read/Write Streaming Extended Pix Format Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second /dev/video28 open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK test for unlimited opens: OK test invalid ioctls: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 1 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls (Input 0): test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 6 Private Controls: 1 Format ioctls (Input 0): test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK test VIDIOC_TRY_FMT: OK test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) fail: v4l2-test-formats.cpp(1544): !rect_is_inside(&sel_crop.r, &sel_bounds.r) fail: v4l2-test-formats.cpp(1649): testBasicCrop(node, V4L2_BUF_TYPE_VIDEO_CAPTURE) test Cropping: FAIL test Composing: OK test Scaling: OK Codec ioctls (Input 0): test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls (Input 0): test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK test Requests: OK (Not Supported) Total for rcar_vin device /dev/video28: 45, Succeeded: 44, Failed: 1, Warnings: 0 Without this patch all test pass. > > Fixes: 104464f573d ("media: rcar-vin: Do not reset the crop and compose rectangles in s_fmt") > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > Changes for v2: > * Dropped redundant code mapping crop and compose rects > > v1 - https://lkml.org/lkml/2020/7/31/364 > --- > drivers/media/platform/rcar-vin/rcar-v4l2.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c > index 0e066bba747e..1bd59a8453b4 100644 > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c > @@ -305,7 +305,7 @@ static int rvin_s_fmt_vid_cap(struct file *file, void *priv, > struct v4l2_format *f) > { > struct rvin_dev *vin = video_drvdata(file); > - struct v4l2_rect fmt_rect, src_rect; > + struct v4l2_rect src_rect; > int ret; > > if (vb2_is_busy(&vin->queue)) > @@ -317,14 +317,11 @@ static int rvin_s_fmt_vid_cap(struct file *file, void *priv, > return ret; > > vin->format = f->fmt.pix; > - > - fmt_rect.top = 0; > - fmt_rect.left = 0; > - fmt_rect.width = vin->format.width; > - fmt_rect.height = vin->format.height; > - > - v4l2_rect_map_inside(&vin->crop, &src_rect); > - v4l2_rect_map_inside(&vin->compose, &fmt_rect); > + vin->crop.top = 0; > + vin->crop.left = 0; > + vin->crop.width = vin->format.width; > + vin->crop.height = vin->format.height; > + vin->compose = vin->crop; > vin->src_rect = src_rect; > > return 0; > -- > 2.17.1 >
On 13/09/2020 20:21, Lad Prabhakar wrote: > The crop and compose settings for VIN in non mc mode werent updated > in s_fmt call this resulted in captured images being clipped. > > With the below sequence on the third capture where size is set to > 640x480 resulted in clipped image of size 320x240. > > high(640x480) -> low (320x240) -> high (640x480) > > This patch makes sure the VIN crop and compose settings are updated. I'm not sure the original behavior was wrong at all. When calling S_FMT(320x240) it should force the crop and compose rectangles into 320x240, but when calling S_FMT(640x480) the crop and compose rectangles do not need to be modified and are kept. It is up to userspace to update those crop/compose rectangles. Calling S_FMT must, however, update the crop/compose bounds/default rectangles where applicable. Note that the crop coordinates are against the video source resolution, *not* the format width/height. So this patch is definitely wrong in that respect. Regards, Hans > > Fixes: 104464f573d ("media: rcar-vin: Do not reset the crop and compose rectangles in s_fmt") > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > Changes for v2: > * Dropped redundant code mapping crop and compose rects > > v1 - https://lkml.org/lkml/2020/7/31/364 > --- > drivers/media/platform/rcar-vin/rcar-v4l2.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c > index 0e066bba747e..1bd59a8453b4 100644 > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c > @@ -305,7 +305,7 @@ static int rvin_s_fmt_vid_cap(struct file *file, void *priv, > struct v4l2_format *f) > { > struct rvin_dev *vin = video_drvdata(file); > - struct v4l2_rect fmt_rect, src_rect; > + struct v4l2_rect src_rect; > int ret; > > if (vb2_is_busy(&vin->queue)) > @@ -317,14 +317,11 @@ static int rvin_s_fmt_vid_cap(struct file *file, void *priv, > return ret; > > vin->format = f->fmt.pix; > - > - fmt_rect.top = 0; > - fmt_rect.left = 0; > - fmt_rect.width = vin->format.width; > - fmt_rect.height = vin->format.height; > - > - v4l2_rect_map_inside(&vin->crop, &src_rect); > - v4l2_rect_map_inside(&vin->compose, &fmt_rect); > + vin->crop.top = 0; > + vin->crop.left = 0; > + vin->crop.width = vin->format.width; > + vin->crop.height = vin->format.height; > + vin->compose = vin->crop; > vin->src_rect = src_rect; > > return 0; >
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c index 0e066bba747e..1bd59a8453b4 100644 --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c @@ -305,7 +305,7 @@ static int rvin_s_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f) { struct rvin_dev *vin = video_drvdata(file); - struct v4l2_rect fmt_rect, src_rect; + struct v4l2_rect src_rect; int ret; if (vb2_is_busy(&vin->queue)) @@ -317,14 +317,11 @@ static int rvin_s_fmt_vid_cap(struct file *file, void *priv, return ret; vin->format = f->fmt.pix; - - fmt_rect.top = 0; - fmt_rect.left = 0; - fmt_rect.width = vin->format.width; - fmt_rect.height = vin->format.height; - - v4l2_rect_map_inside(&vin->crop, &src_rect); - v4l2_rect_map_inside(&vin->compose, &fmt_rect); + vin->crop.top = 0; + vin->crop.left = 0; + vin->crop.width = vin->format.width; + vin->crop.height = vin->format.height; + vin->compose = vin->crop; vin->src_rect = src_rect; return 0;