Hi Niklas On 05/07/2019 05:55, Niklas Söderlund wrote: > The crop and compose rectangles where reset when s_fmt was called s/where reset/are reset/ s/was called/is called/ > resulting in potentially valid rectangles where lost when updating the s/where lost/being lost/ > format. Fix this by instead trying to map the rectangles inside the new I don't think there's any 'trying' here - just doing. "Fix this by mapping the rectangles inside ..." > format. > It could be worth expanding on the consequences of this patch here: "This may result in the crop and compose windows being modified from the original setting to ensure that they are valid within the dimensions of the new format." But perhaps that's just too much repetition. It's just the point about making it clear that the existing cropping and compose rectangles may be different after this call that might be worth expressing somehow. > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx> Does this deserve a fixes tag? Or is this more of a solitary change. With the wordings fixed, and expanded if you desire: 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 2d94e700a473572c..d5e860ba6d9a2409 100644 > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c > @@ -271,8 +271,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; >
