Re: [PATCH v3 2/4] media: v4l2-common: Add v4l2_fill_pixfmt_aligned() helper

[Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>]

Hi Jacopo, Sven,
Thanks for your comments:

On Fri, Jul 10, 2026 at 02:26:23PM +0200, Sven Püschel wrote:
> Hi Jacopo,
> 
> On 7/10/26 2:15 PM, Jacopo Mondi wrote:
> > Hi Sven
> > 
> > On Fri, Jul 10, 2026 at 01:54:06PM +0200, Sven Püschel wrote:
> > > Hi Jacopo,
> > > 
> > > On 7/10/26 11:38 AM, Jacopo Mondi wrote:
> > > > > This is due to the fact, that while we have a hdiv of 2 we also interleave
> > > > > the cb and cr parts in a single plane, which results in the stride being the
> > > > > same number of bytes as for the y plane (and vdiv isn't relevant for the
> > > > > stride).
> > > > > 
> > > > > Therefore the stride scaling also respects the bits per plane (bpp) value to
> > > > > determine the scaling.
> > > > > 
> > > > > @Tommaso : While the sentence looks ok, the NV12 example is misguided. The
> > > > I guess the usage of NV12 was as example of a "formats that store
> > > > multiple component planes in a single memory"
> > > > 
> > > > NV24/42 works the same, but being 444 it needs the chroma plane stride to
> > > > be a multiple of the fist plane stride and might prove as a better
> > > > example ?
> > > > 
> > > My potential concern is that NV as an example misguides the reader into one
> > > of the following:
> > > 
> > > - It's only for formats which interleave cb/cr into one plane (whereas
> > > YUV420 also gets scaled)
> > > - NV24 in the example being though of including the NV24M variant (whereas
> > > latter won't be affected)
> > M variants are not supported by the single-planar APIs
> > https://docs.kernel.org/userspace-api/media/v4l/pixfmt-yuv-planar.html
> > 
> > Some planar formats allow planes to be placed in independent memory
> > locations. They are identified by an ‘M’ suffix in their name (such as
> > in V4L2_PIX_FMT_NV12M). Those formats are intended to be used only in
> > drivers and applications that support the multi-planar API,
> > 
> > And here we're dealing with single-planar API only if I'm not mistaken
> 
> Oh, sorry. Assumed that the added description of both functions would be
> similar/identical, which isn't the case.
> 
> Given this, I'm fine with the wording and agree to just change the example
> to smth. else than NV12.

So if I'm not wrong we can then use YUV420 instead of NV12.
NV12 is an unlucky example where the alignment scaling factor is 1,
whereas YUV420 has a scaling factor of 2:

# NV12
(Y - luma)      bpp[0] = 1
(CbCr - chroma) bpp[1] = 2
                hdiv = 2

# YUV420
(Y - luma)    bpp[0] = 1
(Cb - chroma) bpp[1] = 1
              hdiv = 2

For plane = 0 (single memory-plane formats):

factor = DIV_ROUND_UP(hdiv * bpp[0], bpp[1])

	NV12:    factor = DIV_ROUND_UP(2 * 1, 2) = 1
	YUV420:  factor = DIV_ROUND_UP(2 * 1, 1) = 2

Then I will leave the wording as is and changing only parenthesis part
like Sven suggested:

	(e.g NV12) --> (e.g. YUV420)

Please correct me if I'm wrong.
If for you is ok I will fix this in v4.

Kind Regards,
Tommaso


> 
> Sincerely
>     Sven
> 
>