diff mbox series

[2/4] rcar-vin: Do not reset the crop and compose rectangles in s_fmt

Message ID 20190705045557.25463-3-niklas.soderlund+renesas@ragnatech.se (mailing list archive)
State Superseded
Delegated to: Kieran Bingham
Headers show
Series rcar-vin: Add support for V4L2_FIELD_ALTERNATE | expand

Commit Message

Niklas Söderlund July 5, 2019, 4:55 a.m. UTC
The crop and compose rectangles where reset when s_fmt was called
resulting in potentially valid rectangles where lost when updating the
format. Fix this by instead trying to map the rectangles inside the new
format.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-v4l2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Sergei Shtylyov July 5, 2019, 8:36 a.m. UTC | #1
Hello!

On 05.07.2019 7:55, Niklas Söderlund wrote:

> The crop and compose rectangles where reset when s_fmt was called

    s/where/were/?

> resulting in potentially valid rectangles where lost when updating the

    And here too...

> format. Fix this by instead trying to map the rectangles inside the new
> format.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
[...]

MBR, Sergei
Kieran Bingham July 16, 2019, 2:51 p.m. UTC | #2
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@ragnatech.se>

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@ideasonboard.com>

> ---
>  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;
>
diff mbox series

Patch

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;