All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Stevenson <dave.stevenson@raspberrypi.com>
To: John Cox <jc@kynesim.co.uk>
Cc: Nicolas Dufresne <nicolas@ndufresne.ca>,
	linux-media@vger.kernel.org, hverkuil-cisco@xs4all.nl
Subject: Re: [PATCH 2/2] media: v4l: Add documentation for Broadcom sand formats
Date: Thu, 9 Feb 2023 19:22:42 +0000	[thread overview]
Message-ID: <CAPY8ntCKRWomcWaBLTy7VD7j4k9VBTc6dEeJOWzA2xMyor0=Dw@mail.gmail.com> (raw)
In-Reply-To: <0oeauhhaqbbmn1l9ea2dlk62bvbli5i325@4ax.com>

Hi Nicolas (and John)

On Thu, 9 Feb 2023 at 18:54, John Cox <jc@kynesim.co.uk> wrote:
>
> >Le vendredi 27 janvier 2023 à 15:34 +0000, John Cox a écrit :
> >> Add documentation for for the Broadcom sand formats to pixfmt-yuv-planar.
> >
> >Not a review comment, but does SAND stand for anything you can share ?
>
> Honestly I don't know - I got the term from Broadcom's MMAL definitions.

Broadcom acquired Sand Video back in 2004, and it was the format that
they used in their video codec IP that was then integrated into
VideoCore. Internally it was always referred to as "Sand Video
Format", or just Sand.

  Dave

https://www.eetimes.com/broadcom-pays-77-5-million-for-sand-video/

> >> Signed-off-by: John Cox <jc@kynesim.co.uk>
> >> ---
> >>  .../media/v4l/pixfmt-yuv-planar.rst           | 192 ++++++++++++++++++
> >>  1 file changed, 192 insertions(+)
> >>
> >> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst b/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
> >> index f1d5bb7b806d..c1dd0856f497 100644
> >> --- a/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
> >> +++ b/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
> >> @@ -123,6 +123,20 @@ All components are stored with the same number of bits per component.
> >>        - Cb, Cr
> >>        - Yes
> >>        - 4x4 tiles
> >> +    * - V4L2_PIX_FMT_NV12_C128
> >> +      - 'CN12'
> >> +      - 8
> >> +      - 4:2:0
> >> +      - Cb, Cr
> >> +      - Yes
> >> +      - 128 byte columns
> >> +    * - V4L2_PIX_FMT_P030_C128
> >> +      - 'CN30'
> >> +      - 10
> >> +      - 4:2:0
> >> +      - Cb, Cr
> >> +      - Yes
> >> +      - 128 byte columns
> >>      * - V4L2_PIX_FMT_NV16
> >>        - 'NV16'
> >>        - 8
> >> @@ -277,6 +291,8 @@ of the luma plane.
> >>  .. _V4L2-PIX-FMT-NV12M-10BE-8L128:
> >>  .. _V4L2-PIX-FMT-NV12-10BE-8L128:
> >>  .. _V4L2-PIX-FMT-MM21:
> >> +.. _V4L2-PIX-FMT-NV12-C128:
> >> +.. _V4L2-PIX-FMT-P030-C128:
> >>
> >>  Tiled NV12
> >>  ----------
> >> @@ -364,6 +380,182 @@ two non-contiguous planes.
> >>
> >>      Example V4L2_PIX_FMT_NV12MT memory layout of tiles
> >>
> >> +``V4L2_PIX_FMT_NV21_C128`` store 8 bit luma and chroma data in 128 byte
> >
> >I think you meant V4L2_PIX_FMT_NV12_C128 ?
> Yes - will fix
>
> >> +columns. Chroma data follows luma in each column. Height, which must be
> >> +a multiple of 2 (h in the table below) determines the offset to the start
> >> +of chroma data. Overall (luma + chroma)
> >> +column height (ch in the table below) is also required and this is obtained
> >> +by dividing the sizeimage by bytesperline. bytesperline is width rounded up
> >> +to the next multiple of the column width (128).
> >
> >Assuming I understood the description, unlike any other tiled formats, the Y and
> >UV columns are not being split in two plane, but instead Y and UV columns are
> >being interleaved. Am I understanding correctly ? I think the text could be
> >improved in this regard.
>
> I'll see what I can do
>
> Its a somewhat confusing format. You get a 128-byte wide, full picture Y
> height column of luma, optional padding, followed by a 128-byte wide,
> full chroma height column of interleaved CrCb, more optional padding,
> then the start of the next luma column.
>
> The thing that makes it uniqely annoying as far as representation is
> concerned is that in conventional terms it effectively has a fixed pitch
> or bytesperline of 128 (i.e. you always add 128 to get from x,y to
> x,y+1) and it has a "vertical pitch" to go between columns which doesn't
> fit into any normal picture description.
>
> >Note, I totally agree on using NV12 as reference, I see lower that CbCr is
> >effectively interleaved.
> >
> >> +
> >> +.. flat-table::
> >> +    :header-rows:  0
> >> +    :stub-columns: 0
> >> +    :widths: 15 10 10 10 10 4 10 10 10 10
> >> +
> >> +    * - start + 0:
> >> +      - Y'\ :sub:`0,0`
> >> +      - Y'\ :sub:`0,1`
> >> +      - Y'\ :sub:`0,2`
> >> +      - Y'\ :sub:`0,3`
> >> +      - ...
> >> +      - Y'\ :sub:`0,124`
> >> +      - Y'\ :sub:`0,125`
> >> +      - Y'\ :sub:`0,126`
> >> +      - Y'\ :sub:`0,127`
> >> +    * - start + 128:
> >> +      - Y'\ :sub:`1,0`
> >> +      - Y'\ :sub:`1,1`
> >> +      - Y'\ :sub:`1,2`
> >> +      - Y'\ :sub:`1,3`
> >> +      - ...
> >> +      - Y'\ :sub:`1,124`
> >> +      - Y'\ :sub:`1,125`
> >> +      - Y'\ :sub:`1,126`
> >> +      - Y'\ :sub:`1,127`
> >> +    * - start + 256:
> >> +      - Y'\ :sub:`2,0`
> >> +      - Y'\ :sub:`2,1`
> >> +      - Y'\ :sub:`2,2`
> >> +      - Y'\ :sub:`2,3`
> >> +      - ...
> >> +      - Y'\ :sub:`2,124`
> >> +      - Y'\ :sub:`2,125`
> >> +      - Y'\ :sub:`2,126`
> >> +      - Y'\ :sub:`2,127`
> >> +    * - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +    * - start + ((h-1) * 128):
> >> +      - Y'\ :sub:`h-1,0`
> >> +      - Y'\ :sub:`h-1,1`
> >> +      - Y'\ :sub:`h-1,2`
> >> +      - Y'\ :sub:`h-1,3`
> >> +      - ...
> >> +      - Y'\ :sub:`h-1,124`
> >> +      - Y'\ :sub:`h-1,125`
> >> +      - Y'\ :sub:`h-1,126`
> >> +      - Y'\ :sub:`h-1,127`
> >> +    * - start + ((h) * 128):
> >> +      - Cb\ :sub:`0,0`
> >> +      - Cr\ :sub:`0,0`
> >> +      - Cb\ :sub:`0,1`
> >> +      - Cr\ :sub:`0,1`
> >> +      - ...
> >> +      - Cb\ :sub:`0,62`
> >> +      - Cr\ :sub:`0,62`
> >> +      - Cb\ :sub:`0,63`
> >> +      - Cr\ :sub:`0,63`
> >> +    * - start + ((h+1) * 128):
> >> +      - Cb\ :sub:`1,0`
> >> +      - Cr\ :sub:`1,0`
> >> +      - Cb\ :sub:`1,1`
> >> +      - Cr\ :sub:`1,1`
> >> +      - ...
> >> +      - Cb\ :sub:`1,62`
> >> +      - Cr\ :sub:`1,62`
> >> +      - Cb\ :sub:`1,63`
> >> +      - Cr\ :sub:`1,63`
> >> +    * - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +    * - start + ((h+(h/2)-1) * 128):
> >> +      - Cb\ :sub:`(h/2)-1,0`
> >> +      - Cr\ :sub:`(h/2)-1,0`
> >> +      - Cb\ :sub:`(h/2)-1,1`
> >> +      - Cr\ :sub:`(h/2)-1,1`
> >> +      - ...
> >> +      - Cb\ :sub:`(h/2)-1,62`
> >> +      - Cr\ :sub:`(h/2)-1,62`
> >> +      - Cb\ :sub:`(h/2)-1,63`
> >> +      - Cr\ :sub:`(h/2)-1,63`
> >> +    * - start + (ch * 128):
> >> +      - Y'\ :sub:`0,128`
> >> +      - Y'\ :sub:`0,129`
> >> +      - Y'\ :sub:`0,130`
> >> +      - Y'\ :sub:`0,131`
> >> +      - ...
> >> +      - Y'\ :sub:`0,252`
> >> +      - Y'\ :sub:`0,253`
> >> +      - Y'\ :sub:`0,254`
> >> +      - Y'\ :sub:`0,255`
> >> +    * - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +      - ...
> >> +
> >> +``V4L2_PIX_FMT_P030_C128`` uses the same 128 byte column structure as
> >> +``V4L2_PIX_FMT_NV12_C128``, but encodes 10-bit YUV.
> >> +3 10-bit values are packed into 4 bytes as bits 9:0, 19:10, and 29:20, with
> >> +bits 30 & 31 unused. For the luma plane, bits 9:0 are Y0, 19:10 are Y1, and
> >> +29:20 are Y2. For the chroma plane the samples always come in pairs of Cr
> >> +and Cb, so it needs to be considered 6 values packed in 8 bytes. bytesperline
> >> +is ((width + 95)/96)*128
> >
> >Maybe insert a phrase related to the use of the same columns pattern as
> >NV12_C128 ? Then from there, its easier to understand why this bytesperline
> >formula. I'm guessing that 96 is the number of pixels that fits 1 row of 128
> >bytes (128 / 4 * 3). I could not guess why you need 128 times that size though ?
> >Maybe I'm missing something ?
>
> 96 is indeed the number of pixels that fits in one 128 byte column.
>
> (width+95)/96 is the number of columns required, mutiplying it by 128
> gets you a "pitch" that when mutiplied by height gives you the number of
> bytes occupied by the luma plane. This is mostly to keep software that
> expects width <= bytesperline happy.
>
> >> +
> >> +Bit-packed representation - Luma:
> >> +
> >> +.. flat-table::
> >> +    :header-rows:  1
> >> +    :stub-columns: 0
> >> +
> >> +    * - byte
> >> +      - value(s)
> >> +    * - 0
> >> +      - Y'\ :sub:`00[7:0]`
> >> +    * - 1
> >> +      - Y'\ :sub:`01[5:0]`\  (bits 7--2)
> >> +      - Y'\ :sub:`00[9:8]`\  (bits 1--0)
> >> +    * - 2
> >> +      - Y'\ :sub:`02[3:0]`\  (bits 7--4)
> >> +      - Y'\ :sub:`01[9:6]`\  (bits 3--0)
> >> +    * - 3
> >> +      - unused (bits 7--6)
> >> +      - Y'\ :sub:`02[9:4]`\  (bits 5--0)
> >> +
> >> +Bit-packed representation - Chroma:
> >> +
> >> +.. flat-table::
> >> +    :header-rows:  1
> >> +    :stub-columns: 0
> >> +
> >> +    * - byte
> >> +      - value(s)
> >> +    * - 0
> >> +      - Cb\ :sub:`00[7:0]`
> >> +    * - 1
> >> +      - Cr\ :sub:`00[5:0]`\  (bits 7--2)
> >> +      - Cb\ :sub:`00[9:8]`\  (bits 1--0)
> >> +    * - 2
> >> +      - Cb\ :sub:`01[3:0]`\  (bits 7--4)
> >> +      - Cr\ :sub:`00[9:6]`\  (bits 3--0)
> >> +    * - 3
> >> +      - unused (bits 7--6)
> >> +      - Cb\ :sub:`02[9:4]`\  (bits 5--0)
> >> +    * - 4
> >> +      - Cr\ :sub:`01[7:0]`
> >> +    * - 5
> >> +      - Cb\ :sub:`02[5:0]`\  (bits 7--2)
> >> +      - Cr\ :sub:`01[9:8]`\  (bits 1--0)
> >> +    * - 6
> >> +      - Cr\ :sub:`02[3:0]`\  (bits 7--4)
> >> +      - Cb\ :sub:`02[9:6]`\  (bits 3--0)
> >> +    * - 7
> >> +      - unused (bits 7--6)
> >> +      - Cr\ :sub:`02[9:4]`\  (bits 5--0)
> >>
> >>  .. _V4L2-PIX-FMT-NV16:
> >>  .. _V4L2-PIX-FMT-NV61:

  reply	other threads:[~2023-02-09 19:23 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-27 15:34 [PATCH 0/2] media: v4l: Add Broadcom sand format to the list of V4L formats John Cox
2023-01-27 15:34 ` [PATCH 1/2] media: v4l: Add Broadcom sand formats to videodev2.h John Cox
2023-02-09 18:20   ` Nicolas Dufresne
2023-02-09 19:06     ` John Cox
2023-02-10 16:17       ` Nicolas Dufresne
2023-02-12 18:49         ` John Cox
2023-02-13 16:25           ` Nicolas Dufresne
2023-01-27 15:34 ` [PATCH 2/2] media: v4l: Add documentation for Broadcom sand formats John Cox
2023-02-09 18:21   ` Nicolas Dufresne
2023-02-09 18:54     ` John Cox
2023-02-09 19:22       ` Dave Stevenson [this message]
2023-02-10 14:50       ` Nicolas Dufresne
2023-02-09 17:56 ` [PATCH 0/2] media: v4l: Add Broadcom sand format to the list of V4L formats Nicolas Dufresne
2023-02-09 18:21   ` John Cox
2023-02-10 14:42     ` Nicolas Dufresne
2023-02-10 15:10       ` John Cox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAPY8ntCKRWomcWaBLTy7VD7j4k9VBTc6dEeJOWzA2xMyor0=Dw@mail.gmail.com' \
    --to=dave.stevenson@raspberrypi.com \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=jc@kynesim.co.uk \
    --cc=linux-media@vger.kernel.org \
    --cc=nicolas@ndufresne.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.