mbox series

[v6,00/11] media: rkvdec: Add H.264 High 10 and 4:2:2 profile support

Message ID 20240909192522.1076704-1-jonas@kwiboo.se
Headers show
Series media: rkvdec: Add H.264 High 10 and 4:2:2 profile support | expand

Message

Jonas Karlman Sept. 9, 2024, 7:24 p.m. UTC
This series add H.264 High 10 and 4:2:2 profile support to the Rockchip
Video Decoder driver.

Patch 1 add helpers for calculating plane bytesperline and sizeimage.
Patch 2 add two new pixelformats for semi-planer 10-bit 4:2:0/4:2:2 YUV.

Patch 3 change to use bytesperline and buffer height to configure strides.
Patch 4 change to use values from SPS/PPS control to configure the HW.

Patch 5-9 refactor code to support filtering of CAPUTRE formats based
on the image format returned from a get_image_fmt ops.

Patch 10 add final bits to support H.264 High 10 and 4:2:2 profiles.

Patch 11 add a fix for enumerated frame sizes returned to userspace.

Tested on a ROCK Pi 4 (RK3399) and Rock64 (RK3328):

  v4l2-compliance 1.28.1, 64 bits, 64-bit time_t
  ...
  Total for rkvdec device /dev/video1: 48, Succeeded: 48, Failed: 0, Warnings: 0

  Running test suite JVT-FR-EXT with decoder FFmpeg-H.264-v4l2request
  ...
  Ran 65/69 tests successfully

  Running test suite JVT-AVC_V1 with decoder FFmpeg-H.264-v4l2request
  ...
  Ran 129/135 tests successfully

Before this series:

  Running test suite JVT-FR-EXT with decoder FFmpeg-H.264-v4l2request
  ...
  Ran 44/69 tests successfully

Changes in v6:
- Change to use fmt_idx instead of j++ tucked inside a condition (Dan)
- Add patch to fix enumerated frame sizes returned to userspace (Alex)
- Fluster test score is same as v4 and v5, see [4] and [5]
Link to v5: https://lore.kernel.org/linux-media/20240618194647.742037-1-jonas@kwiboo.se/

Changes in v5:
- Drop Remove SPS validation at streaming start patch
- Move buffer align from rkvdec_fill_decoded_pixfmt to min/step_width
- Use correct profiles for V4L2_CID_MPEG_VIDEO_H264_PROFILE
- Collect r-b and t-b tags
- Fluster test score is same as v4, see [4] and [5]
Link to v4: https://lore.kernel.org/linux-media/20231105165521.3592037-1-jonas@kwiboo.se/

Changes in v4:
- Fix failed v4l2-compliance tests related to CAPTURE queue
- Rework CAPTURE format filter anv validate to use an image format
- Run fluster test suite JVT-FR-EXT [4] and JVT-AVC_V1 [5]
Link to v3: https://lore.kernel.org/linux-media/20231029183427.1781554-1-jonas@kwiboo.se/

Changes in v3:
- Drop merged patches
- Use bpp and bpp_div instead of prior misuse of block_w/block_h
- New patch to use values from SPS/PPS control to configure the HW
- New patch to remove an unnecessary call to validate sps at streaming start
- Reworked pixel format validation
Link to v2: https://lore.kernel.org/linux-media/20200706215430.22859-1-jonas@kwiboo.se/

Changes in v2:
- Collect r-b tags
- SPS pic width and height in mbs validation moved to rkvdec_try_ctrl
- New patch to not override output buffer sizeimage
- Reworked pixel format validation
- Only align decoded buffer instead of changing frmsize step_width
Link to v1: https://lore.kernel.org/linux-media/20200701215616.30874-1-jonas@kwiboo.se/

To fully runtime test this series you may need FFmpeg patches from [1]
and fluster patches from [2], this series is also available at [3].

[1] https://github.com/Kwiboo/FFmpeg/commits/v4l2request-2024-v2-rkvdec/
[2] https://github.com/Kwiboo/fluster/commits/ffmpeg-v4l2request-rkvdec/
[3] https://github.com/Kwiboo/linux-rockchip/commits/linuxtv-rkvdec-high-10-v6/
[4] https://gist.github.com/Kwiboo/f4ac15576b2c72887ae2bc5d58b5c865
[5] https://gist.github.com/Kwiboo/459a1c8f1dcb56e45dc7a7a29cc28adf

Regards,
Jonas

Alex Bee (1):
  media: rkvdec: h264: Don't hardcode SPS/PPS parameters

Jonas Karlman (10):
  media: v4l2-common: Add helpers to calculate bytesperline and
    sizeimage
  media: v4l2: Add NV15 and NV20 pixel formats
  media: rkvdec: h264: Use bytesperline and buffer height as virstride
  media: rkvdec: Extract rkvdec_fill_decoded_pixfmt into helper
  media: rkvdec: Move rkvdec_reset_decoded_fmt helper
  media: rkvdec: Extract decoded format enumeration into helper
  media: rkvdec: Add image format concept
  media: rkvdec: Add get_image_fmt ops
  media: rkvdec: h264: Support High 10 and 4:2:2 profiles
  media: rkvdec: Fix enumerate frame sizes

 .../media/v4l/pixfmt-yuv-planar.rst           | 128 ++++++++++
 drivers/media/v4l2-core/v4l2-common.c         |  80 +++---
 drivers/media/v4l2-core/v4l2-ioctl.c          |   2 +
 drivers/staging/media/rkvdec/rkvdec-h264.c    |  64 +++--
 drivers/staging/media/rkvdec/rkvdec.c         | 239 +++++++++++++-----
 drivers/staging/media/rkvdec/rkvdec.h         |  18 +-
 include/uapi/linux/videodev2.h                |   2 +
 7 files changed, 410 insertions(+), 123 deletions(-)

Comments

Diederik de Haas Oct. 3, 2024, 12:23 p.m. UTC | #1
On Mon Sep 9, 2024 at 9:24 PM CEST, Jonas Karlman wrote:
> This series add H.264 High 10 and 4:2:2 profile support to the Rockchip
> Video Decoder driver.
> ...
> Tested on a ROCK Pi 4 (RK3399) and Rock64 (RK3328):
> ...
>
> Link to v1: https://lore.kernel.org/linux-media/20200701215616.30874-1-jonas@kwiboo.se/
>
> To fully runtime test this series you may need FFmpeg patches from [1]
> and fluster patches from [2], this series is also available at [3].

I have been using this patch set and earlier version stemming all the
way back to 2023-10-29 and with a patched ffmpeg and mpv ([1] but
earlier versions before that) I have been enjoying HW accelerated
playback on my Rock64 (rk3328) :-)

So for the series, feel free to add my

Tested-by: Diederik de Haas <didi.debian@cknow.org>

[1] https://github.com/mpv-player/mpv/pull/14690

Cheers,
  Diederik

> [1] https://github.com/Kwiboo/FFmpeg/commits/v4l2request-2024-v2-rkvdec/
> [2] https://github.com/Kwiboo/fluster/commits/ffmpeg-v4l2request-rkvdec/
> [3] https://github.com/Kwiboo/linux-rockchip/commits/linuxtv-rkvdec-high-10-v6/
> [4] https://gist.github.com/Kwiboo/f4ac15576b2c72887ae2bc5d58b5c865
> [5] https://gist.github.com/Kwiboo/459a1c8f1dcb56e45dc7a7a29cc28adf
>
> Regards,
> Jonas
>
> Alex Bee (1):
>   media: rkvdec: h264: Don't hardcode SPS/PPS parameters
>
> Jonas Karlman (10):
>   media: v4l2-common: Add helpers to calculate bytesperline and
>     sizeimage
>   media: v4l2: Add NV15 and NV20 pixel formats
>   media: rkvdec: h264: Use bytesperline and buffer height as virstride
>   media: rkvdec: Extract rkvdec_fill_decoded_pixfmt into helper
>   media: rkvdec: Move rkvdec_reset_decoded_fmt helper
>   media: rkvdec: Extract decoded format enumeration into helper
>   media: rkvdec: Add image format concept
>   media: rkvdec: Add get_image_fmt ops
>   media: rkvdec: h264: Support High 10 and 4:2:2 profiles
>   media: rkvdec: Fix enumerate frame sizes
>
>  .../media/v4l/pixfmt-yuv-planar.rst           | 128 ++++++++++
>  drivers/media/v4l2-core/v4l2-common.c         |  80 +++---
>  drivers/media/v4l2-core/v4l2-ioctl.c          |   2 +
>  drivers/staging/media/rkvdec/rkvdec-h264.c    |  64 +++--
>  drivers/staging/media/rkvdec/rkvdec.c         | 239 +++++++++++++-----
>  drivers/staging/media/rkvdec/rkvdec.h         |  18 +-
>  include/uapi/linux/videodev2.h                |   2 +
>  7 files changed, 410 insertions(+), 123 deletions(-)
Jonas Karlman Oct. 25, 2024, 8:20 a.m. UTC | #2
Hi Sebastian,

Will you have time to look at this series any time soon?

Would like to send a v2 of the now one year old rkvdec hevc series but
this series is sort of holding that back ;-)

Regards,
Jonas

On 2024-09-09 21:24, Jonas Karlman wrote:
> This series add H.264 High 10 and 4:2:2 profile support to the Rockchip
> Video Decoder driver.
> 
> Patch 1 add helpers for calculating plane bytesperline and sizeimage.
> Patch 2 add two new pixelformats for semi-planer 10-bit 4:2:0/4:2:2 YUV.
> 
> Patch 3 change to use bytesperline and buffer height to configure strides.
> Patch 4 change to use values from SPS/PPS control to configure the HW.
> 
> Patch 5-9 refactor code to support filtering of CAPUTRE formats based
> on the image format returned from a get_image_fmt ops.
> 
> Patch 10 add final bits to support H.264 High 10 and 4:2:2 profiles.
> 
> Patch 11 add a fix for enumerated frame sizes returned to userspace.
> 
> Tested on a ROCK Pi 4 (RK3399) and Rock64 (RK3328):
> 
>   v4l2-compliance 1.28.1, 64 bits, 64-bit time_t
>   ...
>   Total for rkvdec device /dev/video1: 48, Succeeded: 48, Failed: 0, Warnings: 0
> 
>   Running test suite JVT-FR-EXT with decoder FFmpeg-H.264-v4l2request
>   ...
>   Ran 65/69 tests successfully
> 
>   Running test suite JVT-AVC_V1 with decoder FFmpeg-H.264-v4l2request
>   ...
>   Ran 129/135 tests successfully
> 
> Before this series:
> 
>   Running test suite JVT-FR-EXT with decoder FFmpeg-H.264-v4l2request
>   ...
>   Ran 44/69 tests successfully
> 
> Changes in v6:
> - Change to use fmt_idx instead of j++ tucked inside a condition (Dan)
> - Add patch to fix enumerated frame sizes returned to userspace (Alex)
> - Fluster test score is same as v4 and v5, see [4] and [5]
> Link to v5: https://lore.kernel.org/linux-media/20240618194647.742037-1-jonas@kwiboo.se/
> 
> Changes in v5:
> - Drop Remove SPS validation at streaming start patch
> - Move buffer align from rkvdec_fill_decoded_pixfmt to min/step_width
> - Use correct profiles for V4L2_CID_MPEG_VIDEO_H264_PROFILE
> - Collect r-b and t-b tags
> - Fluster test score is same as v4, see [4] and [5]
> Link to v4: https://lore.kernel.org/linux-media/20231105165521.3592037-1-jonas@kwiboo.se/
> 
> Changes in v4:
> - Fix failed v4l2-compliance tests related to CAPTURE queue
> - Rework CAPTURE format filter anv validate to use an image format
> - Run fluster test suite JVT-FR-EXT [4] and JVT-AVC_V1 [5]
> Link to v3: https://lore.kernel.org/linux-media/20231029183427.1781554-1-jonas@kwiboo.se/
> 
> Changes in v3:
> - Drop merged patches
> - Use bpp and bpp_div instead of prior misuse of block_w/block_h
> - New patch to use values from SPS/PPS control to configure the HW
> - New patch to remove an unnecessary call to validate sps at streaming start
> - Reworked pixel format validation
> Link to v2: https://lore.kernel.org/linux-media/20200706215430.22859-1-jonas@kwiboo.se/
> 
> Changes in v2:
> - Collect r-b tags
> - SPS pic width and height in mbs validation moved to rkvdec_try_ctrl
> - New patch to not override output buffer sizeimage
> - Reworked pixel format validation
> - Only align decoded buffer instead of changing frmsize step_width
> Link to v1: https://lore.kernel.org/linux-media/20200701215616.30874-1-jonas@kwiboo.se/
> 
> To fully runtime test this series you may need FFmpeg patches from [1]
> and fluster patches from [2], this series is also available at [3].
> 
> [1] https://github.com/Kwiboo/FFmpeg/commits/v4l2request-2024-v2-rkvdec/
> [2] https://github.com/Kwiboo/fluster/commits/ffmpeg-v4l2request-rkvdec/
> [3] https://github.com/Kwiboo/linux-rockchip/commits/linuxtv-rkvdec-high-10-v6/
> [4] https://gist.github.com/Kwiboo/f4ac15576b2c72887ae2bc5d58b5c865
> [5] https://gist.github.com/Kwiboo/459a1c8f1dcb56e45dc7a7a29cc28adf
> 
> Regards,
> Jonas
> 
> Alex Bee (1):
>   media: rkvdec: h264: Don't hardcode SPS/PPS parameters
> 
> Jonas Karlman (10):
>   media: v4l2-common: Add helpers to calculate bytesperline and
>     sizeimage
>   media: v4l2: Add NV15 and NV20 pixel formats
>   media: rkvdec: h264: Use bytesperline and buffer height as virstride
>   media: rkvdec: Extract rkvdec_fill_decoded_pixfmt into helper
>   media: rkvdec: Move rkvdec_reset_decoded_fmt helper
>   media: rkvdec: Extract decoded format enumeration into helper
>   media: rkvdec: Add image format concept
>   media: rkvdec: Add get_image_fmt ops
>   media: rkvdec: h264: Support High 10 and 4:2:2 profiles
>   media: rkvdec: Fix enumerate frame sizes
> 
>  .../media/v4l/pixfmt-yuv-planar.rst           | 128 ++++++++++
>  drivers/media/v4l2-core/v4l2-common.c         |  80 +++---
>  drivers/media/v4l2-core/v4l2-ioctl.c          |   2 +
>  drivers/staging/media/rkvdec/rkvdec-h264.c    |  64 +++--
>  drivers/staging/media/rkvdec/rkvdec.c         | 239 +++++++++++++-----
>  drivers/staging/media/rkvdec/rkvdec.h         |  18 +-
>  include/uapi/linux/videodev2.h                |   2 +
>  7 files changed, 410 insertions(+), 123 deletions(-)
>
Sebastian Fricke Oct. 25, 2024, 10:30 a.m. UTC | #3
Hey Jonas,

On 25.10.2024 10:20, Jonas Karlman wrote:
>Hi Sebastian,
>
>Will you have time to look at this series any time soon?
>
>Would like to send a v2 of the now one year old rkvdec hevc series but
>this series is sort of holding that back ;-)

Sorry for the delay we (maintainer & reviewer from Collabora) are
currently a bit busy, this is on top of our review list however, so
please another 1 or 2 weeks of patience :).

>
>Regards,
>Jonas

Thanks for your work and regards!
Sebastian

>
>On 2024-09-09 21:24, Jonas Karlman wrote:
>> This series add H.264 High 10 and 4:2:2 profile support to the Rockchip
>> Video Decoder driver.
>>
>> Patch 1 add helpers for calculating plane bytesperline and sizeimage.
>> Patch 2 add two new pixelformats for semi-planer 10-bit 4:2:0/4:2:2 YUV.
>>
>> Patch 3 change to use bytesperline and buffer height to configure strides.
>> Patch 4 change to use values from SPS/PPS control to configure the HW.
>>
>> Patch 5-9 refactor code to support filtering of CAPUTRE formats based
>> on the image format returned from a get_image_fmt ops.
>>
>> Patch 10 add final bits to support H.264 High 10 and 4:2:2 profiles.
>>
>> Patch 11 add a fix for enumerated frame sizes returned to userspace.
>>
>> Tested on a ROCK Pi 4 (RK3399) and Rock64 (RK3328):
>>
>>   v4l2-compliance 1.28.1, 64 bits, 64-bit time_t
>>   ...
>>   Total for rkvdec device /dev/video1: 48, Succeeded: 48, Failed: 0, Warnings: 0
>>
>>   Running test suite JVT-FR-EXT with decoder FFmpeg-H.264-v4l2request
>>   ...
>>   Ran 65/69 tests successfully
>>
>>   Running test suite JVT-AVC_V1 with decoder FFmpeg-H.264-v4l2request
>>   ...
>>   Ran 129/135 tests successfully
>>
>> Before this series:
>>
>>   Running test suite JVT-FR-EXT with decoder FFmpeg-H.264-v4l2request
>>   ...
>>   Ran 44/69 tests successfully
>>
>> Changes in v6:
>> - Change to use fmt_idx instead of j++ tucked inside a condition (Dan)
>> - Add patch to fix enumerated frame sizes returned to userspace (Alex)
>> - Fluster test score is same as v4 and v5, see [4] and [5]
>> Link to v5: https://lore.kernel.org/linux-media/20240618194647.742037-1-jonas@kwiboo.se/
>>
>> Changes in v5:
>> - Drop Remove SPS validation at streaming start patch
>> - Move buffer align from rkvdec_fill_decoded_pixfmt to min/step_width
>> - Use correct profiles for V4L2_CID_MPEG_VIDEO_H264_PROFILE
>> - Collect r-b and t-b tags
>> - Fluster test score is same as v4, see [4] and [5]
>> Link to v4: https://lore.kernel.org/linux-media/20231105165521.3592037-1-jonas@kwiboo.se/
>>
>> Changes in v4:
>> - Fix failed v4l2-compliance tests related to CAPTURE queue
>> - Rework CAPTURE format filter anv validate to use an image format
>> - Run fluster test suite JVT-FR-EXT [4] and JVT-AVC_V1 [5]
>> Link to v3: https://lore.kernel.org/linux-media/20231029183427.1781554-1-jonas@kwiboo.se/
>>
>> Changes in v3:
>> - Drop merged patches
>> - Use bpp and bpp_div instead of prior misuse of block_w/block_h
>> - New patch to use values from SPS/PPS control to configure the HW
>> - New patch to remove an unnecessary call to validate sps at streaming start
>> - Reworked pixel format validation
>> Link to v2: https://lore.kernel.org/linux-media/20200706215430.22859-1-jonas@kwiboo.se/
>>
>> Changes in v2:
>> - Collect r-b tags
>> - SPS pic width and height in mbs validation moved to rkvdec_try_ctrl
>> - New patch to not override output buffer sizeimage
>> - Reworked pixel format validation
>> - Only align decoded buffer instead of changing frmsize step_width
>> Link to v1: https://lore.kernel.org/linux-media/20200701215616.30874-1-jonas@kwiboo.se/
>>
>> To fully runtime test this series you may need FFmpeg patches from [1]
>> and fluster patches from [2], this series is also available at [3].
>>
>> [1] https://github.com/Kwiboo/FFmpeg/commits/v4l2request-2024-v2-rkvdec/
>> [2] https://github.com/Kwiboo/fluster/commits/ffmpeg-v4l2request-rkvdec/
>> [3] https://github.com/Kwiboo/linux-rockchip/commits/linuxtv-rkvdec-high-10-v6/
>> [4] https://gist.github.com/Kwiboo/f4ac15576b2c72887ae2bc5d58b5c865
>> [5] https://gist.github.com/Kwiboo/459a1c8f1dcb56e45dc7a7a29cc28adf
>>
>> Regards,
>> Jonas
>>
>> Alex Bee (1):
>>   media: rkvdec: h264: Don't hardcode SPS/PPS parameters
>>
>> Jonas Karlman (10):
>>   media: v4l2-common: Add helpers to calculate bytesperline and
>>     sizeimage
>>   media: v4l2: Add NV15 and NV20 pixel formats
>>   media: rkvdec: h264: Use bytesperline and buffer height as virstride
>>   media: rkvdec: Extract rkvdec_fill_decoded_pixfmt into helper
>>   media: rkvdec: Move rkvdec_reset_decoded_fmt helper
>>   media: rkvdec: Extract decoded format enumeration into helper
>>   media: rkvdec: Add image format concept
>>   media: rkvdec: Add get_image_fmt ops
>>   media: rkvdec: h264: Support High 10 and 4:2:2 profiles
>>   media: rkvdec: Fix enumerate frame sizes
>>
>>  .../media/v4l/pixfmt-yuv-planar.rst           | 128 ++++++++++
>>  drivers/media/v4l2-core/v4l2-common.c         |  80 +++---
>>  drivers/media/v4l2-core/v4l2-ioctl.c          |   2 +
>>  drivers/staging/media/rkvdec/rkvdec-h264.c    |  64 +++--
>>  drivers/staging/media/rkvdec/rkvdec.c         | 239 +++++++++++++-----
>>  drivers/staging/media/rkvdec/rkvdec.h         |  18 +-
>>  include/uapi/linux/videodev2.h                |   2 +
>>  7 files changed, 410 insertions(+), 123 deletions(-)
>>
>
>
Sebastian Fricke
Consultant Software Engineer

Collabora Ltd
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales no 5513718.
Nicolas Dufresne Oct. 25, 2024, 12:54 p.m. UTC | #4
Le vendredi 25 octobre 2024 à 12:30 +0200, Sebastian Fricke a écrit :
> Hey Jonas,
> 
> On 25.10.2024 10:20, Jonas Karlman wrote:
> > Hi Sebastian,
> > 
> > Will you have time to look at this series any time soon?
> > 
> > Would like to send a v2 of the now one year old rkvdec hevc series but
> > this series is sort of holding that back ;-)
> 
> Sorry for the delay we (maintainer & reviewer from Collabora) are
> currently a bit busy, this is on top of our review list however, so
> please another 1 or 2 weeks of patience :).

Actually, I'll go over it today.

regards,
Nicolas

> 
> > 
> > Regards,
> > Jonas
> 
> Thanks for your work and regards!
> Sebastian
> 
> > 
> > On 2024-09-09 21:24, Jonas Karlman wrote:
> > > This series add H.264 High 10 and 4:2:2 profile support to the Rockchip
> > > Video Decoder driver.
> > > 
> > > Patch 1 add helpers for calculating plane bytesperline and sizeimage.
> > > Patch 2 add two new pixelformats for semi-planer 10-bit 4:2:0/4:2:2 YUV.
> > > 
> > > Patch 3 change to use bytesperline and buffer height to configure strides.
> > > Patch 4 change to use values from SPS/PPS control to configure the HW.
> > > 
> > > Patch 5-9 refactor code to support filtering of CAPUTRE formats based
> > > on the image format returned from a get_image_fmt ops.
> > > 
> > > Patch 10 add final bits to support H.264 High 10 and 4:2:2 profiles.
> > > 
> > > Patch 11 add a fix for enumerated frame sizes returned to userspace.
> > > 
> > > Tested on a ROCK Pi 4 (RK3399) and Rock64 (RK3328):
> > > 
> > >   v4l2-compliance 1.28.1, 64 bits, 64-bit time_t
> > >   ...
> > >   Total for rkvdec device /dev/video1: 48, Succeeded: 48, Failed: 0, Warnings: 0
> > > 
> > >   Running test suite JVT-FR-EXT with decoder FFmpeg-H.264-v4l2request
> > >   ...
> > >   Ran 65/69 tests successfully
> > > 
> > >   Running test suite JVT-AVC_V1 with decoder FFmpeg-H.264-v4l2request
> > >   ...
> > >   Ran 129/135 tests successfully
> > > 
> > > Before this series:
> > > 
> > >   Running test suite JVT-FR-EXT with decoder FFmpeg-H.264-v4l2request
> > >   ...
> > >   Ran 44/69 tests successfully
> > > 
> > > Changes in v6:
> > > - Change to use fmt_idx instead of j++ tucked inside a condition (Dan)
> > > - Add patch to fix enumerated frame sizes returned to userspace (Alex)
> > > - Fluster test score is same as v4 and v5, see [4] and [5]
> > > Link to v5: https://lore.kernel.org/linux-media/20240618194647.742037-1-jonas@kwiboo.se/
> > > 
> > > Changes in v5:
> > > - Drop Remove SPS validation at streaming start patch
> > > - Move buffer align from rkvdec_fill_decoded_pixfmt to min/step_width
> > > - Use correct profiles for V4L2_CID_MPEG_VIDEO_H264_PROFILE
> > > - Collect r-b and t-b tags
> > > - Fluster test score is same as v4, see [4] and [5]
> > > Link to v4: https://lore.kernel.org/linux-media/20231105165521.3592037-1-jonas@kwiboo.se/
> > > 
> > > Changes in v4:
> > > - Fix failed v4l2-compliance tests related to CAPTURE queue
> > > - Rework CAPTURE format filter anv validate to use an image format
> > > - Run fluster test suite JVT-FR-EXT [4] and JVT-AVC_V1 [5]
> > > Link to v3: https://lore.kernel.org/linux-media/20231029183427.1781554-1-jonas@kwiboo.se/
> > > 
> > > Changes in v3:
> > > - Drop merged patches
> > > - Use bpp and bpp_div instead of prior misuse of block_w/block_h
> > > - New patch to use values from SPS/PPS control to configure the HW
> > > - New patch to remove an unnecessary call to validate sps at streaming start
> > > - Reworked pixel format validation
> > > Link to v2: https://lore.kernel.org/linux-media/20200706215430.22859-1-jonas@kwiboo.se/
> > > 
> > > Changes in v2:
> > > - Collect r-b tags
> > > - SPS pic width and height in mbs validation moved to rkvdec_try_ctrl
> > > - New patch to not override output buffer sizeimage
> > > - Reworked pixel format validation
> > > - Only align decoded buffer instead of changing frmsize step_width
> > > Link to v1: https://lore.kernel.org/linux-media/20200701215616.30874-1-jonas@kwiboo.se/
> > > 
> > > To fully runtime test this series you may need FFmpeg patches from [1]
> > > and fluster patches from [2], this series is also available at [3].
> > > 
> > > [1] https://github.com/Kwiboo/FFmpeg/commits/v4l2request-2024-v2-rkvdec/
> > > [2] https://github.com/Kwiboo/fluster/commits/ffmpeg-v4l2request-rkvdec/
> > > [3] https://github.com/Kwiboo/linux-rockchip/commits/linuxtv-rkvdec-high-10-v6/
> > > [4] https://gist.github.com/Kwiboo/f4ac15576b2c72887ae2bc5d58b5c865
> > > [5] https://gist.github.com/Kwiboo/459a1c8f1dcb56e45dc7a7a29cc28adf
> > > 
> > > Regards,
> > > Jonas
> > > 
> > > Alex Bee (1):
> > >   media: rkvdec: h264: Don't hardcode SPS/PPS parameters
> > > 
> > > Jonas Karlman (10):
> > >   media: v4l2-common: Add helpers to calculate bytesperline and
> > >     sizeimage
> > >   media: v4l2: Add NV15 and NV20 pixel formats
> > >   media: rkvdec: h264: Use bytesperline and buffer height as virstride
> > >   media: rkvdec: Extract rkvdec_fill_decoded_pixfmt into helper
> > >   media: rkvdec: Move rkvdec_reset_decoded_fmt helper
> > >   media: rkvdec: Extract decoded format enumeration into helper
> > >   media: rkvdec: Add image format concept
> > >   media: rkvdec: Add get_image_fmt ops
> > >   media: rkvdec: h264: Support High 10 and 4:2:2 profiles
> > >   media: rkvdec: Fix enumerate frame sizes
> > > 
> > >  .../media/v4l/pixfmt-yuv-planar.rst           | 128 ++++++++++
> > >  drivers/media/v4l2-core/v4l2-common.c         |  80 +++---
> > >  drivers/media/v4l2-core/v4l2-ioctl.c          |   2 +
> > >  drivers/staging/media/rkvdec/rkvdec-h264.c    |  64 +++--
> > >  drivers/staging/media/rkvdec/rkvdec.c         | 239 +++++++++++++-----
> > >  drivers/staging/media/rkvdec/rkvdec.h         |  18 +-
> > >  include/uapi/linux/videodev2.h                |   2 +
> > >  7 files changed, 410 insertions(+), 123 deletions(-)
> > > 
> > 
> > 
> Sebastian Fricke
> Consultant Software Engineer
> 
> Collabora Ltd
> Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
> Registered in England & Wales no 5513718.
Nicolas Dufresne Oct. 25, 2024, 5:20 p.m. UTC | #5
Le lundi 09 septembre 2024 à 19:24 +0000, Jonas Karlman a écrit :
> Add helper functions to calculate plane bytesperline and sizeimage,
> these new helpers consider bpp div, block width and height when
> calculating plane bytesperline and sizeimage.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Tested-by: Christopher Obbard <chris.obbard@collabora.com>
> ---
> v6:
> - No change
> 
> v5:
> - Collect t-b tags
> 
> v4:
> - No change
> 
> v3:
> - Consider bpp_div in calculation
> ---
>  drivers/media/v4l2-core/v4l2-common.c | 78 +++++++++++++--------------
>  1 file changed, 39 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 0a2f4f0d0a07..8ad04d48a023 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -357,6 +357,34 @@ static inline unsigned int v4l2_format_block_height(const struct v4l2_format_inf
>  	return info->block_h[plane];
>  }
>  
> +static inline unsigned int v4l2_format_plane_width(const struct v4l2_format_info *info, int plane,

nit: What would you think of naming this one v4l2_format_plane_stride() ? As the
returned value is in bytes, not in pixels. (I'm not a huge fan of
_bytesperline(), but would seem valid and consistent too).

With or without this, I'm happy to see this land:

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> +						   unsigned int width)
> +{
> +	unsigned int hdiv = plane ? info->hdiv : 1;
> +	unsigned int aligned_width =
> +		ALIGN(width, v4l2_format_block_width(info, plane));
> +
> +	return DIV_ROUND_UP(aligned_width, hdiv) *
> +	       info->bpp[plane] / info->bpp_div[plane];
> +}
> +
> +static inline unsigned int v4l2_format_plane_height(const struct v4l2_format_info *info, int plane,
> +						    unsigned int height)
> +{
> +	unsigned int vdiv = plane ? info->vdiv : 1;
> +	unsigned int aligned_height =
> +		ALIGN(height, v4l2_format_block_height(info, plane));
> +
> +	return DIV_ROUND_UP(aligned_height, vdiv);
> +}
> +
> +static inline unsigned int v4l2_format_plane_size(const struct v4l2_format_info *info, int plane,
> +						  unsigned int width, unsigned int height)
> +{
> +	return v4l2_format_plane_width(info, plane, width) *
> +	       v4l2_format_plane_height(info, plane, height);
> +}
> +
>  void v4l2_apply_frmsize_constraints(u32 *width, u32 *height,
>  				    const struct v4l2_frmsize_stepwise *frmsize)
>  {
> @@ -392,37 +420,19 @@ int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt,
>  
>  	if (info->mem_planes == 1) {
>  		plane = &pixfmt->plane_fmt[0];
> -		plane->bytesperline = ALIGN(width, v4l2_format_block_width(info, 0)) * info->bpp[0] / info->bpp_div[0];
> +		plane->bytesperline = v4l2_format_plane_width(info, 0, width);
>  		plane->sizeimage = 0;
>  
> -		for (i = 0; i < info->comp_planes; i++) {
> -			unsigned int hdiv = (i == 0) ? 1 : info->hdiv;
> -			unsigned int vdiv = (i == 0) ? 1 : info->vdiv;
> -			unsigned int aligned_width;
> -			unsigned int aligned_height;
> -
> -			aligned_width = ALIGN(width, v4l2_format_block_width(info, i));
> -			aligned_height = ALIGN(height, v4l2_format_block_height(info, i));
> -
> -			plane->sizeimage += info->bpp[i] *
> -				DIV_ROUND_UP(aligned_width, hdiv) *
> -				DIV_ROUND_UP(aligned_height, vdiv) / info->bpp_div[i];
> -		}
> +		for (i = 0; i < info->comp_planes; i++)
> +			plane->sizeimage +=
> +				v4l2_format_plane_size(info, i, width, height);
>  	} else {
>  		for (i = 0; i < info->comp_planes; i++) {
> -			unsigned int hdiv = (i == 0) ? 1 : info->hdiv;
> -			unsigned int vdiv = (i == 0) ? 1 : info->vdiv;
> -			unsigned int aligned_width;
> -			unsigned int aligned_height;
> -
> -			aligned_width = ALIGN(width, v4l2_format_block_width(info, i));
> -			aligned_height = ALIGN(height, v4l2_format_block_height(info, i));
> -
>  			plane = &pixfmt->plane_fmt[i];
>  			plane->bytesperline =
> -				info->bpp[i] * DIV_ROUND_UP(aligned_width, hdiv) / info->bpp_div[i];
> -			plane->sizeimage =
> -				plane->bytesperline * DIV_ROUND_UP(aligned_height, vdiv);
> +				v4l2_format_plane_width(info, i, width);
> +			plane->sizeimage = plane->bytesperline *
> +				v4l2_format_plane_height(info, i, height);
>  		}
>  	}
>  	return 0;
> @@ -446,22 +456,12 @@ int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
>  	pixfmt->width = width;
>  	pixfmt->height = height;
>  	pixfmt->pixelformat = pixelformat;
> -	pixfmt->bytesperline = ALIGN(width, v4l2_format_block_width(info, 0)) * info->bpp[0] / info->bpp_div[0];
> +	pixfmt->bytesperline = v4l2_format_plane_width(info, 0, width);
>  	pixfmt->sizeimage = 0;
>  
> -	for (i = 0; i < info->comp_planes; i++) {
> -		unsigned int hdiv = (i == 0) ? 1 : info->hdiv;
> -		unsigned int vdiv = (i == 0) ? 1 : info->vdiv;
> -		unsigned int aligned_width;
> -		unsigned int aligned_height;
> -
> -		aligned_width = ALIGN(width, v4l2_format_block_width(info, i));
> -		aligned_height = ALIGN(height, v4l2_format_block_height(info, i));
> -
> -		pixfmt->sizeimage += info->bpp[i] *
> -			DIV_ROUND_UP(aligned_width, hdiv) *
> -			DIV_ROUND_UP(aligned_height, vdiv) / info->bpp_div[i];
> -	}
> +	for (i = 0; i < info->comp_planes; i++)
> +		pixfmt->sizeimage +=
> +			v4l2_format_plane_size(info, i, width, height);
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt);
Nicolas Dufresne Oct. 25, 2024, 5:38 p.m. UTC | #6
Le lundi 09 septembre 2024 à 19:25 +0000, Jonas Karlman a écrit :
> Add support for a get_image_fmt() ops that return the required image
> format.
> 
> The CAPTURE format is reset when required image format changes and the
> buffer queue is not busy.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Tested-by: Christopher Obbard <chris.obbard@collabora.com>

Looks good to me.

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
> v6:
> - No change
> 
> v5:
> - Collect t-b tags
> 
> v4:
> - Change fmt_opaque into an image format
> - Split patch into two
> 
> v3:
> - New patch
> ---
>  drivers/staging/media/rkvdec/rkvdec.c | 49 +++++++++++++++++++++++++--
>  drivers/staging/media/rkvdec/rkvdec.h |  2 ++
>  2 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
> index 467fc05b347a..8df49ee12820 100644
> --- a/drivers/staging/media/rkvdec/rkvdec.c
> +++ b/drivers/staging/media/rkvdec/rkvdec.c
> @@ -111,15 +111,60 @@ static int rkvdec_try_ctrl(struct v4l2_ctrl *ctrl)
>  {
>  	struct rkvdec_ctx *ctx = container_of(ctrl->handler, struct rkvdec_ctx, ctrl_hdl);
>  	const struct rkvdec_coded_fmt_desc *desc = ctx->coded_fmt_desc;
> +	struct v4l2_pix_format_mplane *pix_mp = &ctx->decoded_fmt.fmt.pix_mp;
> +	enum rkvdec_image_fmt image_fmt;
> +	struct vb2_queue *vq;
> +	int ret;
> +
> +	if (desc->ops->try_ctrl) {
> +		ret = desc->ops->try_ctrl(ctx, ctrl);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (!desc->ops->get_image_fmt)
> +		return 0;
>  
> -	if (desc->ops->try_ctrl)
> -		return desc->ops->try_ctrl(ctx, ctrl);
> +	image_fmt = desc->ops->get_image_fmt(ctx, ctrl);
> +	if (ctx->image_fmt == image_fmt)
> +		return 0;
> +
> +	if (rkvdec_is_valid_fmt(ctx, pix_mp->pixelformat, image_fmt))
> +		return 0;
> +
> +	/* format change not allowed when queue is busy */
> +	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
> +			     V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> +	if (vb2_is_busy(vq))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int rkvdec_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +	struct rkvdec_ctx *ctx = container_of(ctrl->handler, struct rkvdec_ctx, ctrl_hdl);
> +	const struct rkvdec_coded_fmt_desc *desc = ctx->coded_fmt_desc;
> +	struct v4l2_pix_format_mplane *pix_mp = &ctx->decoded_fmt.fmt.pix_mp;
> +	enum rkvdec_image_fmt image_fmt;
> +
> +	if (!desc->ops->get_image_fmt)
> +		return 0;
> +
> +	image_fmt = desc->ops->get_image_fmt(ctx, ctrl);
> +	if (ctx->image_fmt == image_fmt)
> +		return 0;
> +
> +	ctx->image_fmt = image_fmt;
> +	if (!rkvdec_is_valid_fmt(ctx, pix_mp->pixelformat, ctx->image_fmt))
> +		rkvdec_reset_decoded_fmt(ctx);
>  
>  	return 0;
>  }
>  
>  static const struct v4l2_ctrl_ops rkvdec_ctrl_ops = {
>  	.try_ctrl = rkvdec_try_ctrl,
> +	.s_ctrl = rkvdec_s_ctrl,
>  };
>  
>  static const struct rkvdec_ctrl_desc rkvdec_h264_ctrl_descs[] = {
> diff --git a/drivers/staging/media/rkvdec/rkvdec.h b/drivers/staging/media/rkvdec/rkvdec.h
> index 6f8cf50c5d99..e466a2753ccf 100644
> --- a/drivers/staging/media/rkvdec/rkvdec.h
> +++ b/drivers/staging/media/rkvdec/rkvdec.h
> @@ -73,6 +73,8 @@ struct rkvdec_coded_fmt_ops {
>  		     struct vb2_v4l2_buffer *dst_buf,
>  		     enum vb2_buffer_state result);
>  	int (*try_ctrl)(struct rkvdec_ctx *ctx, struct v4l2_ctrl *ctrl);
> +	enum rkvdec_image_fmt (*get_image_fmt)(struct rkvdec_ctx *ctx,
> +					       struct v4l2_ctrl *ctrl);
>  };
>  
>  enum rkvdec_image_fmt {
Nicolas Dufresne Oct. 25, 2024, 5:46 p.m. UTC | #7
Le lundi 09 septembre 2024 à 19:25 +0000, Jonas Karlman a écrit :
> The VIDIOC_ENUM_FRAMESIZES ioctl should return all frame sizes (i. e.
> width and height in pixels) that the device supports for the given pixel
> format.
> 
> For coded format returning the frame size used to enforce HW alignment
> requirements for CAPTURE buffers does not make fully sense.
> 
> Instead, signal applications what the maximum frame size that is
> supported by the HW decoder using a frame size of continuous type.
> 
> Fixes: cd33c830448b ("media: rkvdec: Add the rkvdec driver")
> Suggested-by: Alex Bee <knaerzche@gmail.com>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
> v6:
> - New patch
> 
> With this change FFmpeg V4L2 Request API hwaccels can implement a strict
> check if frame size is supported by the video device:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2024-August/332037.html
> ---
>  drivers/staging/media/rkvdec/rkvdec.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
> index c8c14f35ac44..9002eb3a59e5 100644
> --- a/drivers/staging/media/rkvdec/rkvdec.c
> +++ b/drivers/staging/media/rkvdec/rkvdec.c
> @@ -334,8 +334,14 @@ static int rkvdec_enum_framesizes(struct file *file, void *priv,
>  	if (!fmt)
>  		return -EINVAL;
>  
> -	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
> -	fsize->stepwise = fmt->frmsize;
> +	fsize->type = V4L2_FRMSIZE_TYPE_CONTINUOUS;
> +	fsize->stepwise.min_width = 1;
> +	fsize->stepwise.max_width = fmt->frmsize.max_width;
> +	fsize->stepwise.step_width = 1;
> +	fsize->stepwise.min_height = 1;
> +	fsize->stepwise.max_height = fmt->frmsize.max_height;
> +	fsize->stepwise.step_height = 1;
> +
>  	return 0;
>  }
>
Nicolas Dufresne Oct. 25, 2024, 5:47 p.m. UTC | #8
Le vendredi 25 octobre 2024 à 08:54 -0400, Nicolas Dufresne a écrit :
> Le vendredi 25 octobre 2024 à 12:30 +0200, Sebastian Fricke a écrit :
> > Hey Jonas,
> > 
> > On 25.10.2024 10:20, Jonas Karlman wrote:
> > > Hi Sebastian,
> > > 
> > > Will you have time to look at this series any time soon?
> > > 
> > > Would like to send a v2 of the now one year old rkvdec hevc series but
> > > this series is sort of holding that back ;-)
> > 
> > Sorry for the delay we (maintainer & reviewer from Collabora) are
> > currently a bit busy, this is on top of our review list however, so
> > please another 1 or 2 weeks of patience :).
> 
> Actually, I'll go over it today.

I'm done, there is only cosmetic comment, if you respin quickly, I'm sure we can
get this one in soon.

> 
> regards,
> Nicolas
> 
> > 
> > > 
> > > Regards,
> > > Jonas
> > 
> > Thanks for your work and regards!
> > Sebastian
> > 
> > > 
> > > On 2024-09-09 21:24, Jonas Karlman wrote:
> > > > This series add H.264 High 10 and 4:2:2 profile support to the Rockchip
> > > > Video Decoder driver.
> > > > 
> > > > Patch 1 add helpers for calculating plane bytesperline and sizeimage.
> > > > Patch 2 add two new pixelformats for semi-planer 10-bit 4:2:0/4:2:2 YUV.
> > > > 
> > > > Patch 3 change to use bytesperline and buffer height to configure strides.
> > > > Patch 4 change to use values from SPS/PPS control to configure the HW.
> > > > 
> > > > Patch 5-9 refactor code to support filtering of CAPUTRE formats based
> > > > on the image format returned from a get_image_fmt ops.
> > > > 
> > > > Patch 10 add final bits to support H.264 High 10 and 4:2:2 profiles.
> > > > 
> > > > Patch 11 add a fix for enumerated frame sizes returned to userspace.
> > > > 
> > > > Tested on a ROCK Pi 4 (RK3399) and Rock64 (RK3328):
> > > > 
> > > >   v4l2-compliance 1.28.1, 64 bits, 64-bit time_t
> > > >   ...
> > > >   Total for rkvdec device /dev/video1: 48, Succeeded: 48, Failed: 0, Warnings: 0
> > > > 
> > > >   Running test suite JVT-FR-EXT with decoder FFmpeg-H.264-v4l2request
> > > >   ...
> > > >   Ran 65/69 tests successfully
> > > > 
> > > >   Running test suite JVT-AVC_V1 with decoder FFmpeg-H.264-v4l2request
> > > >   ...
> > > >   Ran 129/135 tests successfully
> > > > 
> > > > Before this series:
> > > > 
> > > >   Running test suite JVT-FR-EXT with decoder FFmpeg-H.264-v4l2request
> > > >   ...
> > > >   Ran 44/69 tests successfully
> > > > 
> > > > Changes in v6:
> > > > - Change to use fmt_idx instead of j++ tucked inside a condition (Dan)
> > > > - Add patch to fix enumerated frame sizes returned to userspace (Alex)
> > > > - Fluster test score is same as v4 and v5, see [4] and [5]
> > > > Link to v5: https://lore.kernel.org/linux-media/20240618194647.742037-1-jonas@kwiboo.se/
> > > > 
> > > > Changes in v5:
> > > > - Drop Remove SPS validation at streaming start patch
> > > > - Move buffer align from rkvdec_fill_decoded_pixfmt to min/step_width
> > > > - Use correct profiles for V4L2_CID_MPEG_VIDEO_H264_PROFILE
> > > > - Collect r-b and t-b tags
> > > > - Fluster test score is same as v4, see [4] and [5]
> > > > Link to v4: https://lore.kernel.org/linux-media/20231105165521.3592037-1-jonas@kwiboo.se/
> > > > 
> > > > Changes in v4:
> > > > - Fix failed v4l2-compliance tests related to CAPTURE queue
> > > > - Rework CAPTURE format filter anv validate to use an image format
> > > > - Run fluster test suite JVT-FR-EXT [4] and JVT-AVC_V1 [5]
> > > > Link to v3: https://lore.kernel.org/linux-media/20231029183427.1781554-1-jonas@kwiboo.se/
> > > > 
> > > > Changes in v3:
> > > > - Drop merged patches
> > > > - Use bpp and bpp_div instead of prior misuse of block_w/block_h
> > > > - New patch to use values from SPS/PPS control to configure the HW
> > > > - New patch to remove an unnecessary call to validate sps at streaming start
> > > > - Reworked pixel format validation
> > > > Link to v2: https://lore.kernel.org/linux-media/20200706215430.22859-1-jonas@kwiboo.se/
> > > > 
> > > > Changes in v2:
> > > > - Collect r-b tags
> > > > - SPS pic width and height in mbs validation moved to rkvdec_try_ctrl
> > > > - New patch to not override output buffer sizeimage
> > > > - Reworked pixel format validation
> > > > - Only align decoded buffer instead of changing frmsize step_width
> > > > Link to v1: https://lore.kernel.org/linux-media/20200701215616.30874-1-jonas@kwiboo.se/
> > > > 
> > > > To fully runtime test this series you may need FFmpeg patches from [1]
> > > > and fluster patches from [2], this series is also available at [3].
> > > > 
> > > > [1] https://github.com/Kwiboo/FFmpeg/commits/v4l2request-2024-v2-rkvdec/
> > > > [2] https://github.com/Kwiboo/fluster/commits/ffmpeg-v4l2request-rkvdec/
> > > > [3] https://github.com/Kwiboo/linux-rockchip/commits/linuxtv-rkvdec-high-10-v6/
> > > > [4] https://gist.github.com/Kwiboo/f4ac15576b2c72887ae2bc5d58b5c865
> > > > [5] https://gist.github.com/Kwiboo/459a1c8f1dcb56e45dc7a7a29cc28adf
> > > > 
> > > > Regards,
> > > > Jonas
> > > > 
> > > > Alex Bee (1):
> > > >   media: rkvdec: h264: Don't hardcode SPS/PPS parameters
> > > > 
> > > > Jonas Karlman (10):
> > > >   media: v4l2-common: Add helpers to calculate bytesperline and
> > > >     sizeimage
> > > >   media: v4l2: Add NV15 and NV20 pixel formats
> > > >   media: rkvdec: h264: Use bytesperline and buffer height as virstride
> > > >   media: rkvdec: Extract rkvdec_fill_decoded_pixfmt into helper
> > > >   media: rkvdec: Move rkvdec_reset_decoded_fmt helper
> > > >   media: rkvdec: Extract decoded format enumeration into helper
> > > >   media: rkvdec: Add image format concept
> > > >   media: rkvdec: Add get_image_fmt ops
> > > >   media: rkvdec: h264: Support High 10 and 4:2:2 profiles
> > > >   media: rkvdec: Fix enumerate frame sizes
> > > > 
> > > >  .../media/v4l/pixfmt-yuv-planar.rst           | 128 ++++++++++
> > > >  drivers/media/v4l2-core/v4l2-common.c         |  80 +++---
> > > >  drivers/media/v4l2-core/v4l2-ioctl.c          |   2 +
> > > >  drivers/staging/media/rkvdec/rkvdec-h264.c    |  64 +++--
> > > >  drivers/staging/media/rkvdec/rkvdec.c         | 239 +++++++++++++-----
> > > >  drivers/staging/media/rkvdec/rkvdec.h         |  18 +-
> > > >  include/uapi/linux/videodev2.h                |   2 +
> > > >  7 files changed, 410 insertions(+), 123 deletions(-)
> > > > 
> > > 
> > > 
> > Sebastian Fricke
> > Consultant Software Engineer
> > 
> > Collabora Ltd
> > Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
> > Registered in England & Wales no 5513718.
> 
>
Jonas Karlman Oct. 25, 2024, 8:13 p.m. UTC | #9
On 2024-10-25 19:47, Nicolas Dufresne wrote:
> Le vendredi 25 octobre 2024 à 08:54 -0400, Nicolas Dufresne a écrit :
>> Le vendredi 25 octobre 2024 à 12:30 +0200, Sebastian Fricke a écrit :
>>> Hey Jonas,
>>>
>>> On 25.10.2024 10:20, Jonas Karlman wrote:
>>>> Hi Sebastian,
>>>>
>>>> Will you have time to look at this series any time soon?
>>>>
>>>> Would like to send a v2 of the now one year old rkvdec hevc series but
>>>> this series is sort of holding that back ;-)
>>>
>>> Sorry for the delay we (maintainer & reviewer from Collabora) are
>>> currently a bit busy, this is on top of our review list however, so
>>> please another 1 or 2 weeks of patience :).
>>
>> Actually, I'll go over it today.
> 
> I'm done, there is only cosmetic comment, if you respin quickly, I'm sure we can
> get this one in soon.

Thanks!, I will respin a v7 with the nits fixed later this weekend :-)

Regards,
Jonas

> 
>>
>> regards,
>> Nicolas
>>
>>>
>>>>
>>>> Regards,
>>>> Jonas
>>>
>>> Thanks for your work and regards!
>>> Sebastian
>>>
>>>>
>>>> On 2024-09-09 21:24, Jonas Karlman wrote:
>>>>> This series add H.264 High 10 and 4:2:2 profile support to the Rockchip
>>>>> Video Decoder driver.
>>>>>
>>>>> Patch 1 add helpers for calculating plane bytesperline and sizeimage.
>>>>> Patch 2 add two new pixelformats for semi-planer 10-bit 4:2:0/4:2:2 YUV.
>>>>>
>>>>> Patch 3 change to use bytesperline and buffer height to configure strides.
>>>>> Patch 4 change to use values from SPS/PPS control to configure the HW.
>>>>>
>>>>> Patch 5-9 refactor code to support filtering of CAPUTRE formats based
>>>>> on the image format returned from a get_image_fmt ops.
>>>>>
>>>>> Patch 10 add final bits to support H.264 High 10 and 4:2:2 profiles.
>>>>>
>>>>> Patch 11 add a fix for enumerated frame sizes returned to userspace.
>>>>>
>>>>> Tested on a ROCK Pi 4 (RK3399) and Rock64 (RK3328):
>>>>>
>>>>>   v4l2-compliance 1.28.1, 64 bits, 64-bit time_t
>>>>>   ...
>>>>>   Total for rkvdec device /dev/video1: 48, Succeeded: 48, Failed: 0, Warnings: 0
>>>>>
>>>>>   Running test suite JVT-FR-EXT with decoder FFmpeg-H.264-v4l2request
>>>>>   ...
>>>>>   Ran 65/69 tests successfully
>>>>>
>>>>>   Running test suite JVT-AVC_V1 with decoder FFmpeg-H.264-v4l2request
>>>>>   ...
>>>>>   Ran 129/135 tests successfully
>>>>>
>>>>> Before this series:
>>>>>
>>>>>   Running test suite JVT-FR-EXT with decoder FFmpeg-H.264-v4l2request
>>>>>   ...
>>>>>   Ran 44/69 tests successfully
>>>>>
>>>>> Changes in v6:
>>>>> - Change to use fmt_idx instead of j++ tucked inside a condition (Dan)
>>>>> - Add patch to fix enumerated frame sizes returned to userspace (Alex)
>>>>> - Fluster test score is same as v4 and v5, see [4] and [5]
>>>>> Link to v5: https://lore.kernel.org/linux-media/20240618194647.742037-1-jonas@kwiboo.se/
>>>>>
>>>>> Changes in v5:
>>>>> - Drop Remove SPS validation at streaming start patch
>>>>> - Move buffer align from rkvdec_fill_decoded_pixfmt to min/step_width
>>>>> - Use correct profiles for V4L2_CID_MPEG_VIDEO_H264_PROFILE
>>>>> - Collect r-b and t-b tags
>>>>> - Fluster test score is same as v4, see [4] and [5]
>>>>> Link to v4: https://lore.kernel.org/linux-media/20231105165521.3592037-1-jonas@kwiboo.se/
>>>>>
>>>>> Changes in v4:
>>>>> - Fix failed v4l2-compliance tests related to CAPTURE queue
>>>>> - Rework CAPTURE format filter anv validate to use an image format
>>>>> - Run fluster test suite JVT-FR-EXT [4] and JVT-AVC_V1 [5]
>>>>> Link to v3: https://lore.kernel.org/linux-media/20231029183427.1781554-1-jonas@kwiboo.se/
>>>>>
>>>>> Changes in v3:
>>>>> - Drop merged patches
>>>>> - Use bpp and bpp_div instead of prior misuse of block_w/block_h
>>>>> - New patch to use values from SPS/PPS control to configure the HW
>>>>> - New patch to remove an unnecessary call to validate sps at streaming start
>>>>> - Reworked pixel format validation
>>>>> Link to v2: https://lore.kernel.org/linux-media/20200706215430.22859-1-jonas@kwiboo.se/
>>>>>
>>>>> Changes in v2:
>>>>> - Collect r-b tags
>>>>> - SPS pic width and height in mbs validation moved to rkvdec_try_ctrl
>>>>> - New patch to not override output buffer sizeimage
>>>>> - Reworked pixel format validation
>>>>> - Only align decoded buffer instead of changing frmsize step_width
>>>>> Link to v1: https://lore.kernel.org/linux-media/20200701215616.30874-1-jonas@kwiboo.se/
>>>>>
>>>>> To fully runtime test this series you may need FFmpeg patches from [1]
>>>>> and fluster patches from [2], this series is also available at [3].
>>>>>
>>>>> [1] https://github.com/Kwiboo/FFmpeg/commits/v4l2request-2024-v2-rkvdec/
>>>>> [2] https://github.com/Kwiboo/fluster/commits/ffmpeg-v4l2request-rkvdec/
>>>>> [3] https://github.com/Kwiboo/linux-rockchip/commits/linuxtv-rkvdec-high-10-v6/
>>>>> [4] https://gist.github.com/Kwiboo/f4ac15576b2c72887ae2bc5d58b5c865
>>>>> [5] https://gist.github.com/Kwiboo/459a1c8f1dcb56e45dc7a7a29cc28adf
>>>>>
>>>>> Regards,
>>>>> Jonas
>>>>>
>>>>> Alex Bee (1):
>>>>>   media: rkvdec: h264: Don't hardcode SPS/PPS parameters
>>>>>
>>>>> Jonas Karlman (10):
>>>>>   media: v4l2-common: Add helpers to calculate bytesperline and
>>>>>     sizeimage
>>>>>   media: v4l2: Add NV15 and NV20 pixel formats
>>>>>   media: rkvdec: h264: Use bytesperline and buffer height as virstride
>>>>>   media: rkvdec: Extract rkvdec_fill_decoded_pixfmt into helper
>>>>>   media: rkvdec: Move rkvdec_reset_decoded_fmt helper
>>>>>   media: rkvdec: Extract decoded format enumeration into helper
>>>>>   media: rkvdec: Add image format concept
>>>>>   media: rkvdec: Add get_image_fmt ops
>>>>>   media: rkvdec: h264: Support High 10 and 4:2:2 profiles
>>>>>   media: rkvdec: Fix enumerate frame sizes
>>>>>
>>>>>  .../media/v4l/pixfmt-yuv-planar.rst           | 128 ++++++++++
>>>>>  drivers/media/v4l2-core/v4l2-common.c         |  80 +++---
>>>>>  drivers/media/v4l2-core/v4l2-ioctl.c          |   2 +
>>>>>  drivers/staging/media/rkvdec/rkvdec-h264.c    |  64 +++--
>>>>>  drivers/staging/media/rkvdec/rkvdec.c         | 239 +++++++++++++-----
>>>>>  drivers/staging/media/rkvdec/rkvdec.h         |  18 +-
>>>>>  include/uapi/linux/videodev2.h                |   2 +
>>>>>  7 files changed, 410 insertions(+), 123 deletions(-)
>>>>>
>>>>
>>>>
>>> Sebastian Fricke
>>> Consultant Software Engineer
>>>
>>> Collabora Ltd
>>> Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
>>> Registered in England & Wales no 5513718.
>>
>>
>
Nicolas Dufresne Nov. 7, 2024, 3:11 p.m. UTC | #10
Hi Jonas,

Le vendredi 25 octobre 2024 à 22:13 +0200, Jonas Karlman a écrit :
> On 2024-10-25 19:47, Nicolas Dufresne wrote:
> > Le vendredi 25 octobre 2024 à 08:54 -0400, Nicolas Dufresne a écrit :
> > > Le vendredi 25 octobre 2024 à 12:30 +0200, Sebastian Fricke a écrit :
> > > > Hey Jonas,
> > > > 

[...]

> > 
> > I'm done, there is only cosmetic comment, if you respin quickly, I'm sure we can
> > get this one in soon.
> 
> Thanks!, I will respin a v7 with the nits fixed later this weekend :-)

I simply wanted to check with you if everything was going well with this V7.
Feel free to ping for help.

Nicolas