On 8/8/19 3:35 PM, Niklas Söderlund wrote: > Hi Laurent, > > Thanks for your feedback. > > On 2019-08-08 11:37:51 +0300, Laurent Pinchart wrote: >> Hi Niklas, >> >> Thank you for the patch. >> >> On Thu, Aug 08, 2019 at 03:18:48AM +0200, Niklas Söderlund wrote: >>> The crop and compose rectangles are reset when s_fmt is called >>> resulting in potentially valid rectangles being lost when updating the >>> format. >> >> Isn't this the expected behaviour ? > > I though so to at first but I had a nagging feeling as I was not sure. > So I went and looked at vivid and it does do map the old crop/compose > rectangles inside the new limits vivid_s_fmt_vid_cap(). > > Please note that the variable names in this patch are horrible and are > fixed later in this series. In essence the current crop rectangle is > mapped inside the size of the video coming from the sensor and the > compose rectangle is mapped inside the new format set on the VIN video > device. The only ioctls that completely reset everything are S_STD and S_DV_TIMINGS. Ioctls such as S_FMT and S_SELECTION will try to keep as much of the pre-existing configuration as possible. So this patch is correct. Regards, Hans > > I'm open to dropping this patch, I just want this driver to get this > part right so I mimic vivid. > >> >>> Fix this by mapping the rectangles inside the new format. >>> >>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx> >>> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx> >>> --- >>> drivers/media/platform/rcar-vin/rcar-v4l2.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c >>> index 8b30267f1636aaf1..5dcd787a9cf96ac9 100644 >>> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c >>> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c >>> @@ -279,8 +279,8 @@ static int rvin_s_fmt_vid_cap(struct file *file, void *priv, >>> return ret; >>> >>> vin->format = f->fmt.pix; >>> - vin->crop = crop; >>> - vin->compose = compose; >>> + v4l2_rect_map_inside(&vin->crop, &crop); >>> + v4l2_rect_map_inside(&vin->compose, &compose); >>> vin->src_rect = crop; >>> >>> return 0; >> >> -- >> Regards, >> >> Laurent Pinchart >
