Em Thu, 13 Jun 2024 16:26:43 +0100
Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx> escreveu:
> Quoting Hans Verkuil (2024-06-13 16:19:08)
> > The CENTERED_RECTANGLE define fails to compile on clang and old gcc
> > versions. Just drop it and fill in the crop rectangles explicitly.
>
> So back when I was playing around with this I thought it would have got
> dropped during review / upstreaming before now - because I couldn't find
> a way to make sure the macro args were guaranteed to be used only once,
> by putting some locals in the macro (because of the initialisation).
>
> So I'm not surprised that it needs to be removed, but I am surprised
> that it wasn't for the reason I expected ;-)
>
> Anyway - maybe later I'll experiement with more common helpers perhaps -
> but not if it hits compile errors..
IMO, a helper just makes it worse for humans. I mean, just looking at:
.crop = CENTERED_RECTANGLE(imx283_active_area, 5472, 3648),
I can't tell what values for top/left would be used.
> I do recall using v4l2_rects to define the active area so they could be
> used collectively rather than initialising things as
> .width = WIDTH,
> .height = HEIGHT,
Using defines for the minimum/maximum visible area makes sense,
e. g. something similar to this:
.crop = {
.top = MIN_VISIBLE_TOP,
.left = MIN_VISIBLE_LEFT,
.width = MAX_WIDTH,
.height = MAX_HEIGHT,
},
would also be fine, as it would be clear that the crop region is
the one containing the hardware limits.
> So - perhaps this could be (if it compiles):
> .crop = imx283_active_area,
This should equally work, but maybe you could do, instead:
.crop = &imx283_active_area, // e.g. using a pointer
to avoid duplicating for every supported mode.
Thanks,
Mauro
