[meta-rockchip PATCH 0/9] enable v4l2codecs gstreamer plugin for VPU decoding

[meta-rockchip PATCH 0/9] enable v4l2codecs gstreamer plugin for VPU decoding

[Quentin Schulz]

(Most) Rockchip SoCs have a VPU that can be used with v4l2codecs plugin
from gstreamer. In the kernel this VPU is supported by the Hantro driver
which does both stateless HW video encoding and decoding, however
gstreamer only supports decoding for now, see [1] for encoding support.

All Rockchip SoCs supported in this layer to the exception of the RK3308
have a Hantro VPU according to the compatible list in the Linux kernel,
so all SoC includes (except the one for the RK3308) now set HAS_HANTRO
value to enable VPU decoding via gstreamer.

This was only tested on RK3399 Puma and PX30 Ringneck with Haikou (+
Haikou Video Demo adapter for Ringneck) on Scarthgap 5.0.2.

Note that RK3588 upstream Linux kernel only supports AV1 decoding right
now. Also, there's currently no video output support, so this was tested
on a private downstream branch supporting DSI on RK3588 Tiger with
Haikou + Haikou Video Demo adapter).

Note that Gstreamer v4l2codecs only added support for av1 decoding in
1.23.1, so RK3588 commit doesn't necessarily make sense to be backported
right now to other branches (e.g. scarthgap or kirkstone), at the very
least until more than AV1 decoding is supported (which will probably
land this year or the next).

[1] https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5676

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
Quentin Schulz (9):
      gstreamer-plugins-bad: build v4l2codecs if SoC has a VPU
      px30: enable gstreamer v4l2codecs support
      rk3399: enable gstreamer v4l2codecs support
      rk3066: enable gstreamer v4l2codecs support
      rk3188: enable gstreamer v4l2codecs support
      rk3288: enable gstreamer v4l2codecs support
      rk3328: enable gstreamer v4l2codecs support
      rk356x: enable gstreamer v4l2codecs support
      rk3588(s): enable gstreamer v4l2codecs support

 conf/machine/include/px30.inc                                    | 2 ++
 conf/machine/include/rk3066.inc                                  | 2 ++
 conf/machine/include/rk3188.inc                                  | 2 ++
 conf/machine/include/rk3288.inc                                  | 2 ++
 conf/machine/include/rk3328.inc                                  | 2 ++
 conf/machine/include/rk3399.inc                                  | 2 ++
 conf/machine/include/rk3566.inc                                  | 2 ++
 conf/machine/include/rk3568.inc                                  | 2 ++
 conf/machine/include/rk3588s.inc                                 | 2 ++
 recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_%.bbappend | 1 +
 10 files changed, 19 insertions(+)
---
base-commit: d9566c54b72fd83f1a953fb691ad61f1144544a6
change-id: 20240813-gst-hantro-be7a2abd839f

Best regards,
-- 
Quentin Schulz <quentin.schulz@cherry.de>

[meta-rockchip PATCH 1/9] gstreamer-plugins-bad: build v4l2codecs if SoC has a VPU

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

v4l2codecs is the gstreamer plugin for V4L2 stateless video hardware
decoding. The Rockchip SoCs that have a VPU all seems to be based on
Hantro, a stateless encoding/decoding VPU.

Therefore, let's enable VPU decoding in Gstreamer whenever possible,
when the SoC supports it.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_%.bbappend | 1 +
 1 file changed, 1 insertion(+)

diff --git a/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_%.bbappend b/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_%.bbappend
new file mode 100644
index 0000000..a9f40eb
--- /dev/null
+++ b/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_%.bbappend
@@ -0,0 +1 @@
+PACKAGECONFIG:append:rockchip = "${@' v4l2codecs' if bb.utils.to_boolean(d.getVar('HAS_HANTRO'), False) else ''}"

-- 
2.46.0

[meta-rockchip PATCH 2/9] px30: enable gstreamer v4l2codecs support

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

PX30 has a Hantro VPU, so let's set HAS_HANTRO so that gstreamer
v4l2codecs plugin can be built.

This was tested with weston on a PX30 Ringneck with Haikou+Haikou Video
Demo adapter:

$ gst-launch-1.0 filesrc location=$FILE ! parsebin ! v4l2slh264dec ! waylandsink

with FILE storing the path to any h264 file, e.g.
https://download.blender.org/peach/bigbuckbunny_movies/big_buck_bunny_720p_h264.mov
https://download.blender.org/peach/bigbuckbunny_movies/big_buck_bunny_1080p_h264.mov

Needed packages are:
- weston
- gstreamer1.0-plugins-bad (for waylandsink and v4l2slh264dec)
- gstreamer1.0-plugins-base (for parsebin)

CPU load was ~3% according to top for both 720p and 1080p file (see
above), a few frames are dropped every other second for 1080p but
otherwise smooth.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 conf/machine/include/px30.inc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/conf/machine/include/px30.inc b/conf/machine/include/px30.inc
index d78409e..ef325c3 100644
--- a/conf/machine/include/px30.inc
+++ b/conf/machine/include/px30.inc
@@ -19,3 +19,5 @@ TFA_BUILD_TARGET = "bl31"
 
 UBOOT_SUFFIX ?= "itb"
 UBOOT_ENTRYPOINT ?= "0x06000000"
+
+HAS_HANTRO = "1"

-- 
2.46.0

[meta-rockchip PATCH 3/9] rk3399: enable gstreamer v4l2codecs support

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

RK3399 has a Hantro VPU, so let's set HAS_HANTRO so that gstreamer
v4l2codecs plugin can be built.

This was tested with weston on an RK3399 Puma with Haikou:

$ gst-launch-1.0 filesrc location=$FILE ! parsebin ! v4l2slh264dec ! waylandsink

with FILE storing the path to any h264 file, e.g.
https://download.blender.org/peach/bigbuckbunny_movies/big_buck_bunny_1080p_h264.mov
https://download.blender.org/demo/movies/BBB/bbb_sunflower_2160p_30fps_normal.mp4.zip

Needed packages are:
- weston
- gstreamer1.0-plugins-bad (for waylandsink and v4l2codecs)
- gstreamer1.0-plugins-base (for parsebin)

CPU load was ~1-3% according to top for the files above.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 conf/machine/include/rk3399.inc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/conf/machine/include/rk3399.inc b/conf/machine/include/rk3399.inc
index 47f0560..8650d0a 100644
--- a/conf/machine/include/rk3399.inc
+++ b/conf/machine/include/rk3399.inc
@@ -20,3 +20,5 @@ TFA_BUILD_TARGET = "bl31"
 
 UBOOT_SUFFIX ?= "itb"
 UBOOT_ENTRYPOINT ?= "0x06000000"
+
+HAS_HANTRO = "1"

-- 
2.46.0

[meta-rockchip PATCH 4/9] rk3066: enable gstreamer v4l2codecs support

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

RK3066 has a Hantro VPU, so let's set HAS_HANTRO so that gstreamer
v4l2codecs plugin can be built.

This was not tested.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 conf/machine/include/rk3066.inc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/conf/machine/include/rk3066.inc b/conf/machine/include/rk3066.inc
index 026d5b5..d865fec 100644
--- a/conf/machine/include/rk3066.inc
+++ b/conf/machine/include/rk3066.inc
@@ -12,3 +12,5 @@ SERIAL_CONSOLES = "115200;ttyS2"
 KBUILD_DEFCONFIG = "multi_v7_defconfig"
 KERNEL_FEATURES:append:rk3066 = " bsp/rockchip/remove-non-rockchip-arch-arm.scc"
 KERNEL_IMAGETYPE ?= "zImage"
+
+HAS_HANTRO = "1"

-- 
2.46.0

[meta-rockchip PATCH 5/9] rk3188: enable gstreamer v4l2codecs support

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

RK3188 has a Hantro VPU, so let's set HAS_HANTRO so that gstreamer
v4l2codecs plugin can be built.

This was not tested.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 conf/machine/include/rk3188.inc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/conf/machine/include/rk3188.inc b/conf/machine/include/rk3188.inc
index 0120e2f..9593c6d 100644
--- a/conf/machine/include/rk3188.inc
+++ b/conf/machine/include/rk3188.inc
@@ -12,3 +12,5 @@ SERIAL_CONSOLES = "115200;ttyFIQ0"
 KBUILD_DEFCONFIG = "multi_v7_defconfig"
 KERNEL_FEATURES:append:rk3188 = " bsp/rockchip/remove-non-rockchip-arch-arm.scc"
 KERNEL_IMAGETYPE ?= "zImage"
+
+HAS_HANTRO = "1"

-- 
2.46.0

[meta-rockchip PATCH 6/9] rk3288: enable gstreamer v4l2codecs support

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

RK3288 has a Hantro VPU, so let's set HAS_HANTRO so that gstreamer
v4l2codecs plugin can be built.

This was not tested.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 conf/machine/include/rk3288.inc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/conf/machine/include/rk3288.inc b/conf/machine/include/rk3288.inc
index 684e864..8fc3a06 100644
--- a/conf/machine/include/rk3288.inc
+++ b/conf/machine/include/rk3288.inc
@@ -14,3 +14,5 @@ KERNEL_FEATURES:append:rk3288 = " bsp/rockchip/remove-non-rockchip-arch-arm.scc"
 KERNEL_IMAGETYPE ?= "zImage"
 
 UBOOT_SUFFIX ?= "bin"
+
+HAS_HANTRO = "1"

-- 
2.46.0

[meta-rockchip PATCH 7/9] rk3328: enable gstreamer v4l2codecs support

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

RK3328 has a Hantro VPU, so let's set HAS_HANTRO so that gstreamer
v4l2codecs plugin can be built.

This was not tested.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 conf/machine/include/rk3328.inc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/conf/machine/include/rk3328.inc b/conf/machine/include/rk3328.inc
index 4ecd681..24d9ff2 100644
--- a/conf/machine/include/rk3328.inc
+++ b/conf/machine/include/rk3328.inc
@@ -20,3 +20,5 @@ TFA_BUILD_TARGET = "bl31"
 
 UBOOT_SUFFIX ?= "itb"
 UBOOT_ENTRYPOINT ?= "0x06000000"
+
+HAS_HANTRO = "1"

-- 
2.46.0

[meta-rockchip PATCH 8/9] rk356x: enable gstreamer v4l2codecs support

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

RK3566/RK3568 have a Hantro VPU, so let's set HAS_HANTRO so that
gstreamer v4l2codecs plugin can be built.

This was not tested.

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 conf/machine/include/rk3566.inc | 2 ++
 conf/machine/include/rk3568.inc | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/conf/machine/include/rk3566.inc b/conf/machine/include/rk3566.inc
index 586d688..c09c7e0 100644
--- a/conf/machine/include/rk3566.inc
+++ b/conf/machine/include/rk3566.inc
@@ -19,3 +19,5 @@ PREFERRED_PROVIDER_optee-os = "rockchip-rkbin"
 
 UBOOT_SUFFIX ?= "itb"
 UBOOT_ENTRYPOINT ?= "0x06000000"
+
+HAS_HANTRO = "1"
diff --git a/conf/machine/include/rk3568.inc b/conf/machine/include/rk3568.inc
index 1da212a..2a66432 100644
--- a/conf/machine/include/rk3568.inc
+++ b/conf/machine/include/rk3568.inc
@@ -19,3 +19,5 @@ PREFERRED_PROVIDER_optee-os = "rockchip-rkbin"
 
 UBOOT_SUFFIX ?= "itb"
 UBOOT_ENTRYPOINT ?= "0x06000000"
+
+HAS_HANTRO = "1"

-- 
2.46.0

[meta-rockchip PATCH 9/9] rk3588(s): enable gstreamer v4l2codecs support

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@cherry.de>

RK3588/RK3588s have a Hantro VPU, so let's set HAS_HANTRO so that
gstreamer v4l2codecs plugin can be built.

This was tested on a RK3588 Tiger with Haikou+Haikou Video Demo adapter
- on a downstream v6.6 (upstream-based, not
Rockchip BSP-based) with DSI patches - :

$ gst-launch-1.0 filesrc location=$FILE ! parsebin ! v4l2slav1dec ! fakesink

with FILE storing the path to any AV1 file, e.g.
http://download.opencontent.netflix.com.s3.amazonaws.com/AV1/cmaf/spark-8b-59.94fps/spark_606kbps_432p.mp4
https://woolyss.com/f/av1-nosound-chimera.mp4
https://woolyss.com/f/av1-opus-sita.webm

Needed packages are:
- gstreamer1.0-plugins-bad (for fakesink and v4l2slav1dec)
- gstreamer1.0-plugins-base (for parsebin)

CPU load was 0% according to top for all of the aforementioned files.
For some reason though, waylandsink is very choppy. Combining
fpsdisplaysink with fakesink shows a ~60fps when decoding the 432p file,
~24fps for the two others.
Note that 10b-depth isn't supported (at least in my setup).

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 conf/machine/include/rk3588s.inc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/conf/machine/include/rk3588s.inc b/conf/machine/include/rk3588s.inc
index cd84461..c96f8f5 100644
--- a/conf/machine/include/rk3588s.inc
+++ b/conf/machine/include/rk3588s.inc
@@ -18,3 +18,5 @@ PREFERRED_PROVIDER_optee-os = "rockchip-rkbin"
 
 UBOOT_SUFFIX ?= "itb"
 UBOOT_ENTRYPOINT ?= "0x06000000"
+
+HAS_HANTRO = "1"

-- 
2.46.0

Re: [meta-rockchip PATCH 0/9] enable v4l2codecs gstreamer plugin for VPU decoding

[Quentin Schulz]

Hi all,

On 8/20/24 2:56 PM, Quentin Schulz wrote:
> (Most) Rockchip SoCs have a VPU that can be used with v4l2codecs plugin
> from gstreamer. In the kernel this VPU is supported by the Hantro driver
> which does both stateless HW video encoding and decoding, however
> gstreamer only supports decoding for now, see [1] for encoding support.
> 
> All Rockchip SoCs supported in this layer to the exception of the RK3308
> have a Hantro VPU according to the compatible list in the Linux kernel,
> so all SoC includes (except the one for the RK3308) now set HAS_HANTRO
> value to enable VPU decoding via gstreamer.
> 
> This was only tested on RK3399 Puma and PX30 Ringneck with Haikou (+
> Haikou Video Demo adapter for Ringneck) on Scarthgap 5.0.2.
> 
> Note that RK3588 upstream Linux kernel only supports AV1 decoding right
> now. Also, there's currently no video output support, so this was tested
> on a private downstream branch supporting DSI on RK3588 Tiger with
> Haikou + Haikou Video Demo adapter).
> 
> Note that Gstreamer v4l2codecs only added support for av1 decoding in
> 1.23.1, so RK3588 commit doesn't necessarily make sense to be backported
> right now to other branches (e.g. scarthgap or kirkstone), at the very
> least until more than AV1 decoding is supported (which will probably
> land this year or the next).
> 

Note that this cannot be backported to scarthgap as is as the "rockchip" 
OVERRIDES doesn't exist in that branch (yet?). I'll try to not forget 
sending a series for scarthgap if this gets merged (I am only planning 
to do so for RK3399 and PX30 as those are the only SoCs I can test and 
that can work with Gstreamer from scarthgap).

Cheers,
Quentin

Re: [meta-rockchip PATCH 3/9] rk3399: enable gstreamer v4l2codecs support

[Quentin Schulz]

On 8/20/24 2:56 PM, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> RK3399 has a Hantro VPU, so let's set HAS_HANTRO so that gstreamer
> v4l2codecs plugin can be built.
> 

RK3399 actually also has an RKVDEC VPU which is used for decoding H.264, 
VP9 and H.265. Currently, rkvdec supports H.264 and VP9 decoding, while 
Hantro supports VP8 and MPEG2 decoding as well as JPEG encoding. 
Therefore, I'm not sure the naming of the variable is proper? Should we 
go for HAS_STATELESS_VPU instead?

Cheers,
Quentin

Re: [yocto-patches] [meta-rockchip PATCH 3/9] rk3399: enable gstreamer v4l2codecs support

[Trevor Woerner]

On Tue 2024-08-20 @ 03:48:16 PM, Quentin Schulz via lists.yoctoproject.org wrote:
> 
> 
> On 8/20/24 2:56 PM, Quentin Schulz wrote:
> > From: Quentin Schulz <quentin.schulz@cherry.de>
> > 
> > RK3399 has a Hantro VPU, so let's set HAS_HANTRO so that gstreamer
> > v4l2codecs plugin can be built.
> > 
> 
> RK3399 actually also has an RKVDEC VPU which is used for decoding H.264, VP9
> and H.265. Currently, rkvdec supports H.264 and VP9 decoding, while Hantro
> supports VP8 and MPEG2 decoding as well as JPEG encoding. Therefore, I'm not
> sure the naming of the variable is proper? Should we go for
> HAS_STATELESS_VPU instead?

What about 2 knobs:
	1. HAS_HANTRO
	2. HAS_RKVDEC
?

Does gstreamer have knobs for both sets?

> Cheers,
> Quentin
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#535): https://lists.yoctoproject.org/g/yocto-patches/message/535
> Mute This Topic: https://lists.yoctoproject.org/mt/107999800/900817
> Group Owner: yocto-patches+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/yocto-patches/leave/13168745/900817/63955952/xyzzy [twoerner@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 
> 

Re: [yocto-patches] [meta-rockchip PATCH 3/9] rk3399: enable gstreamer v4l2codecs support

[Quentin Schulz]

Hi Trevor,

On 8/21/24 6:42 AM, Trevor Woerner via lists.yoctoproject.org wrote:
> On Tue 2024-08-20 @ 03:48:16 PM, Quentin Schulz via lists.yoctoproject.org wrote:
>>
>>
>> On 8/20/24 2:56 PM, Quentin Schulz wrote:
>>> From: Quentin Schulz <quentin.schulz@cherry.de>
>>>
>>> RK3399 has a Hantro VPU, so let's set HAS_HANTRO so that gstreamer
>>> v4l2codecs plugin can be built.
>>>
>>
>> RK3399 actually also has an RKVDEC VPU which is used for decoding H.264, VP9
>> and H.265. Currently, rkvdec supports H.264 and VP9 decoding, while Hantro
>> supports VP8 and MPEG2 decoding as well as JPEG encoding. Therefore, I'm not
>> sure the naming of the variable is proper? Should we go for
>> HAS_STATELESS_VPU instead?
> 
> What about 2 knobs:
> 	1. HAS_HANTRO
> 	2. HAS_RKVDEC
> ?
> 
> Does gstreamer have knobs for both sets?
> 

They are both stateless VPUs, supported by the same kernel API I 
believe. So the same meson option is used, v4l2codecs, so I don't think 
we need to have two separate knobs for those? At least, I used the same 
gstreamer plugin for decoding h264 on PX30 and RK3399 and PX30 uses 
Hantro while RK3399 uses RKVDEC for that. Similarly, RK3562, RK356x and 
RK3588(s) all seems to have an RKVDECv2, which likely is also stateless?

Cheers,
Quentin

Re: [meta-rockchip PATCH 0/9] enable v4l2codecs gstreamer plugin for VPU decoding

[Quentin Schulz]

Hi all,

On 8/20/24 2:56 PM, Quentin Schulz wrote:
> (Most) Rockchip SoCs have a VPU that can be used with v4l2codecs plugin
> from gstreamer. In the kernel this VPU is supported by the Hantro driver
> which does both stateless HW video encoding and decoding, however
> gstreamer only supports decoding for now, see [1] for encoding support.
> 
> All Rockchip SoCs supported in this layer to the exception of the RK3308
> have a Hantro VPU according to the compatible list in the Linux kernel,
> so all SoC includes (except the one for the RK3308) now set HAS_HANTRO
> value to enable VPU decoding via gstreamer.
> 
> This was only tested on RK3399 Puma and PX30 Ringneck with Haikou (+
> Haikou Video Demo adapter for Ringneck) on Scarthgap 5.0.2.
> 

Ok, so I had leftovers from previous debugging sessions where I had an 
old version of weston (10.0.2) instead of the one from Scarthgap 
(13.0.x). Note that the color is off (red on RK3399, "negative"-like 
filter on PX30) when using weston from Scarthgap. This is apparently a 
known issue, c.f. 
https://gitlab.freedesktop.org/wayland/weston/-/issues/839. Note that 
Robert said it would be fixed in gstreamer 1.22 but the linked patch 
only made it to 1.23 (Scarthgap has 1.22, Styhead 1.24).

With Styhead, RK3399 and PX30 are working fine with weston. Considering 
what Robert said in the link above, I assume people using another 
compositor than weston may be fine on Scarthgap, so I would suggest to 
backport the patches I submitted in this series to scarthgap anyway? One 
is not forced to use weston to do VPU decoding.
FYI, I gave ~30min at backporting the gstreamer patches to 1.22.11 but 
this is above my skill set today.

Cheers,
Quentin

> Note that RK3588 upstream Linux kernel only supports AV1 decoding right
> now. Also, there's currently no video output support, so this was tested
> on a private downstream branch supporting DSI on RK3588 Tiger with
> Haikou + Haikou Video Demo adapter).
> 
> Note that Gstreamer v4l2codecs only added support for av1 decoding in
> 1.23.1, so RK3588 commit doesn't necessarily make sense to be backported
> right now to other branches (e.g. scarthgap or kirkstone), at the very
> least until more than AV1 decoding is supported (which will probably
> land this year or the next).
> 
> [1] https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5676
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
> Quentin Schulz (9):
>        gstreamer-plugins-bad: build v4l2codecs if SoC has a VPU
>        px30: enable gstreamer v4l2codecs support
>        rk3399: enable gstreamer v4l2codecs support
>        rk3066: enable gstreamer v4l2codecs support
>        rk3188: enable gstreamer v4l2codecs support
>        rk3288: enable gstreamer v4l2codecs support
>        rk3328: enable gstreamer v4l2codecs support
>        rk356x: enable gstreamer v4l2codecs support
>        rk3588(s): enable gstreamer v4l2codecs support
> 
>   conf/machine/include/px30.inc                                    | 2 ++
>   conf/machine/include/rk3066.inc                                  | 2 ++
>   conf/machine/include/rk3188.inc                                  | 2 ++
>   conf/machine/include/rk3288.inc                                  | 2 ++
>   conf/machine/include/rk3328.inc                                  | 2 ++
>   conf/machine/include/rk3399.inc                                  | 2 ++
>   conf/machine/include/rk3566.inc                                  | 2 ++
>   conf/machine/include/rk3568.inc                                  | 2 ++
>   conf/machine/include/rk3588s.inc                                 | 2 ++
>   recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_%.bbappend | 1 +
>   10 files changed, 19 insertions(+)
> ---
> base-commit: d9566c54b72fd83f1a953fb691ad61f1144544a6
> change-id: 20240813-gst-hantro-be7a2abd839f
> 
> Best regards,

Re: [yocto-patches] [meta-rockchip PATCH 3/9] rk3399: enable gstreamer v4l2codecs support

[Trevor Woerner]

On Wed 2024-08-21 @ 10:17:52 AM, Quentin Schulz via lists.yoctoproject.org wrote:
> Hi Trevor,
> 
> On 8/21/24 6:42 AM, Trevor Woerner via lists.yoctoproject.org wrote:
> > On Tue 2024-08-20 @ 03:48:16 PM, Quentin Schulz via lists.yoctoproject.org wrote:
> > > 
> > > 
> > > On 8/20/24 2:56 PM, Quentin Schulz wrote:
> > > > From: Quentin Schulz <quentin.schulz@cherry.de>
> > > > 
> > > > RK3399 has a Hantro VPU, so let's set HAS_HANTRO so that gstreamer
> > > > v4l2codecs plugin can be built.
> > > > 
> > > 
> > > RK3399 actually also has an RKVDEC VPU which is used for decoding H.264, VP9
> > > and H.265. Currently, rkvdec supports H.264 and VP9 decoding, while Hantro
> > > supports VP8 and MPEG2 decoding as well as JPEG encoding. Therefore, I'm not
> > > sure the naming of the variable is proper? Should we go for
> > > HAS_STATELESS_VPU instead?
> > 
> > What about 2 knobs:
> > 	1. HAS_HANTRO
> > 	2. HAS_RKVDEC
> > ?
> > 
> > Does gstreamer have knobs for both sets?
> > 
> 
> They are both stateless VPUs, supported by the same kernel API I believe. So
> the same meson option is used, v4l2codecs, so I don't think we need to have
> two separate knobs for those? At least, I used the same gstreamer plugin for
> decoding h264 on PX30 and RK3399 and PX30 uses Hantro while RK3399 uses
> RKVDEC for that. Similarly, RK3562, RK356x and RK3588(s) all seems to have
> an RKVDECv2, which likely is also stateless?

Is this something the user will definitely always want on (i.e. it won't
work without it) or should we allow the user the choose whether they want it
enabled or not?

Personally I would rather see one patch to add this one feature, rather than
8.

Should we add a note in the README about this, or perhaps the gstreamer
bbappend?

> 
> Cheers,
> Quentin
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#538): https://lists.yoctoproject.org/g/yocto-patches/message/538
> Mute This Topic: https://lists.yoctoproject.org/mt/107999800/900817
> Group Owner: yocto-patches+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/yocto-patches/leave/13168745/900817/63955952/xyzzy [twoerner@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 
> 

Re: [yocto-patches] [meta-rockchip PATCH 3/9] rk3399: enable gstreamer v4l2codecs support

[Quentin Schulz]

Hi Trevor,

On 8/22/24 2:23 PM, Trevor Woerner via lists.yoctoproject.org wrote:
> On Wed 2024-08-21 @ 10:17:52 AM, Quentin Schulz via lists.yoctoproject.org wrote:
>> Hi Trevor,
>>
>> On 8/21/24 6:42 AM, Trevor Woerner via lists.yoctoproject.org wrote:
>>> On Tue 2024-08-20 @ 03:48:16 PM, Quentin Schulz via lists.yoctoproject.org wrote:
>>>>
>>>>
>>>> On 8/20/24 2:56 PM, Quentin Schulz wrote:
>>>>> From: Quentin Schulz <quentin.schulz@cherry.de>
>>>>>
>>>>> RK3399 has a Hantro VPU, so let's set HAS_HANTRO so that gstreamer
>>>>> v4l2codecs plugin can be built.
>>>>>
>>>>
>>>> RK3399 actually also has an RKVDEC VPU which is used for decoding H.264, VP9
>>>> and H.265. Currently, rkvdec supports H.264 and VP9 decoding, while Hantro
>>>> supports VP8 and MPEG2 decoding as well as JPEG encoding. Therefore, I'm not
>>>> sure the naming of the variable is proper? Should we go for
>>>> HAS_STATELESS_VPU instead?
>>>
>>> What about 2 knobs:
>>> 	1. HAS_HANTRO
>>> 	2. HAS_RKVDEC
>>> ?
>>>
>>> Does gstreamer have knobs for both sets?
>>>
>>
>> They are both stateless VPUs, supported by the same kernel API I believe. So
>> the same meson option is used, v4l2codecs, so I don't think we need to have
>> two separate knobs for those? At least, I used the same gstreamer plugin for
>> decoding h264 on PX30 and RK3399 and PX30 uses Hantro while RK3399 uses
>> RKVDEC for that. Similarly, RK3562, RK356x and RK3588(s) all seems to have
>> an RKVDECv2, which likely is also stateless?
> 
> Is this something the user will definitely always want on (i.e. it won't
> work without it) or should we allow the user the choose whether they want it
> enabled or not?
> 

Considering that the only other tool I'm aware of for decoding is ffmpeg 
and it does not currently support stateless VPUs 
(https://ffmpeg.org/pipermail/ffmpeg-devel/2024-August/332034.html), if 
one wants to do decoding on upstream kernel on Rockchip boards, they'll 
need the v4l2codecs PACKAGECONFIG option for gst-plugins-bad recipe 
selected.

They could also disable it for their board if they want to by redefining 
HAS_HANTRO variable in their own config file? Or in a distro, etc.

> Personally I would rather see one patch to add this one feature, rather than
> 8.
> 

Those were separate patches for multiple reasons:
- I only tested on 2 vanilla upstream kernel (RK3399, PX30)
- I only tested on 1 downstream upstream-based kernel (RK3588)
- the rest is untested (not even built)
- All patches except the one for RK3588(s) could be backported to other 
branches (e.g. scarthgap); this turned out to be incorrect because the 
:rockchip OVERRIDES isn't available in that branch (but we can fix this 
:) ).

I wanted to provide also the context for tests I've performed on those 
boards in the commit log. It could be quite long if I put all tests for 
all SoC in the same commit log (but that's fine with me).

> Should we add a note in the README about this, or perhaps the gstreamer
> bbappend?
> 

What exactly would you like to see there?

Cheers,
Quentin

Re: [yocto-patches] [meta-rockchip PATCH 3/9] rk3399: enable gstreamer v4l2codecs support

[Trevor Woerner]

On Thu 2024-08-22 @ 02:38:11 PM, Quentin Schulz via lists.yoctoproject.org wrote:
> Hi Trevor,
> 
> On 8/22/24 2:23 PM, Trevor Woerner via lists.yoctoproject.org wrote:
> > On Wed 2024-08-21 @ 10:17:52 AM, Quentin Schulz via lists.yoctoproject.org wrote:
> > > Hi Trevor,
> > > 
> > > On 8/21/24 6:42 AM, Trevor Woerner via lists.yoctoproject.org wrote:
> > > > On Tue 2024-08-20 @ 03:48:16 PM, Quentin Schulz via lists.yoctoproject.org wrote:
> > > > > 
> > > > > 
> > > > > On 8/20/24 2:56 PM, Quentin Schulz wrote:
> > > > > > From: Quentin Schulz <quentin.schulz@cherry.de>
> > > > > > 
> > > > > > RK3399 has a Hantro VPU, so let's set HAS_HANTRO so that gstreamer
> > > > > > v4l2codecs plugin can be built.
> > > > > > 
> > > > > 
> > > > > RK3399 actually also has an RKVDEC VPU which is used for decoding H.264, VP9
> > > > > and H.265. Currently, rkvdec supports H.264 and VP9 decoding, while Hantro
> > > > > supports VP8 and MPEG2 decoding as well as JPEG encoding. Therefore, I'm not
> > > > > sure the naming of the variable is proper? Should we go for
> > > > > HAS_STATELESS_VPU instead?
> > > > 
> > > > What about 2 knobs:
> > > > 	1. HAS_HANTRO
> > > > 	2. HAS_RKVDEC
> > > > ?
> > > > 
> > > > Does gstreamer have knobs for both sets?
> > > > 
> > > 
> > > They are both stateless VPUs, supported by the same kernel API I believe. So
> > > the same meson option is used, v4l2codecs, so I don't think we need to have
> > > two separate knobs for those? At least, I used the same gstreamer plugin for
> > > decoding h264 on PX30 and RK3399 and PX30 uses Hantro while RK3399 uses
> > > RKVDEC for that. Similarly, RK3562, RK356x and RK3588(s) all seems to have
> > > an RKVDECv2, which likely is also stateless?
> > 
> > Is this something the user will definitely always want on (i.e. it won't
> > work without it) or should we allow the user the choose whether they want it
> > enabled or not?
> > 
> 
> Considering that the only other tool I'm aware of for decoding is ffmpeg and
> it does not currently support stateless VPUs
> (https://ffmpeg.org/pipermail/ffmpeg-devel/2024-August/332034.html), if one
> wants to do decoding on upstream kernel on Rockchip boards, they'll need the
> v4l2codecs PACKAGECONFIG option for gst-plugins-bad recipe selected.
> 
> They could also disable it for their board if they want to by redefining
> HAS_HANTRO variable in their own config file? Or in a distro, etc.

I just wanted to make sure the = in the patches wasn't going to prevent them
from doing that.

> 
> > Personally I would rather see one patch to add this one feature, rather than
> > 8.
> > 
> 
> Those were separate patches for multiple reasons:
> - I only tested on 2 vanilla upstream kernel (RK3399, PX30)
> - I only tested on 1 downstream upstream-based kernel (RK3588)
> - the rest is untested (not even built)
> - All patches except the one for RK3588(s) could be backported to other
> branches (e.g. scarthgap); this turned out to be incorrect because the
> :rockchip OVERRIDES isn't available in that branch (but we can fix this :)
> ).
> 
> I wanted to provide also the context for tests I've performed on those
> boards in the commit log. It could be quite long if I put all tests for all
> SoC in the same commit log (but that's fine with me).

Have you seen the size of some of my commit messages? ;-) I don't mind a large
commit message. I've build-tested your patchset against all rockchip MACHINEs
and it builds fine. When it comes time to backport, the patch can be adjusted
as necessary.

> > Should we add a note in the README about this, or perhaps the gstreamer
> > bbappend?
> > 
> 
> What exactly would you like to see there?

Nobody starts by reading the README, but if a user is browsing the MACHINE
files, notices the HANTRO line and is looking for a quick understanding, they
might reach for the README file then? You know that HANTRO refers to something
to do with video decoding, does everybody? Even if all you do is duplicate the
commit message from 1/9 that would be great.


> 
> Cheers,
> Quentin
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#550): https://lists.yoctoproject.org/g/yocto-patches/message/550
> Mute This Topic: https://lists.yoctoproject.org/mt/107999800/900817
> Group Owner: yocto-patches+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/yocto-patches/leave/13168745/900817/63955952/xyzzy [twoerner@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 
> 

Re: [yocto-patches] [meta-rockchip PATCH 3/9] rk3399: enable gstreamer v4l2codecs support

[Quentin Schulz]

Hi Trevor,

On 8/23/24 3:55 PM, Trevor Woerner via lists.yoctoproject.org wrote:
> On Thu 2024-08-22 @ 02:38:11 PM, Quentin Schulz via lists.yoctoproject.org wrote:
>> Hi Trevor,
>>
>> On 8/22/24 2:23 PM, Trevor Woerner via lists.yoctoproject.org wrote:
>>> On Wed 2024-08-21 @ 10:17:52 AM, Quentin Schulz via lists.yoctoproject.org wrote:
>>>> Hi Trevor,
>>>>
>>>> On 8/21/24 6:42 AM, Trevor Woerner via lists.yoctoproject.org wrote:
>>>>> On Tue 2024-08-20 @ 03:48:16 PM, Quentin Schulz via lists.yoctoproject.org wrote:
>>>>>>
>>>>>>
>>>>>> On 8/20/24 2:56 PM, Quentin Schulz wrote:
>>>>>>> From: Quentin Schulz <quentin.schulz@cherry.de>
>>>>>>>
>>>>>>> RK3399 has a Hantro VPU, so let's set HAS_HANTRO so that gstreamer
>>>>>>> v4l2codecs plugin can be built.
>>>>>>>
>>>>>>
>>>>>> RK3399 actually also has an RKVDEC VPU which is used for decoding H.264, VP9
>>>>>> and H.265. Currently, rkvdec supports H.264 and VP9 decoding, while Hantro
>>>>>> supports VP8 and MPEG2 decoding as well as JPEG encoding. Therefore, I'm not
>>>>>> sure the naming of the variable is proper? Should we go for
>>>>>> HAS_STATELESS_VPU instead?
>>>>>
>>>>> What about 2 knobs:
>>>>> 	1. HAS_HANTRO
>>>>> 	2. HAS_RKVDEC
>>>>> ?
>>>>>
>>>>> Does gstreamer have knobs for both sets?
>>>>>
>>>>
>>>> They are both stateless VPUs, supported by the same kernel API I believe. So
>>>> the same meson option is used, v4l2codecs, so I don't think we need to have
>>>> two separate knobs for those? At least, I used the same gstreamer plugin for
>>>> decoding h264 on PX30 and RK3399 and PX30 uses Hantro while RK3399 uses
>>>> RKVDEC for that. Similarly, RK3562, RK356x and RK3588(s) all seems to have
>>>> an RKVDECv2, which likely is also stateless?
>>>
>>> Is this something the user will definitely always want on (i.e. it won't
>>> work without it) or should we allow the user the choose whether they want it
>>> enabled or not?
>>>
>>
>> Considering that the only other tool I'm aware of for decoding is ffmpeg and
>> it does not currently support stateless VPUs
>> (https://ffmpeg.org/pipermail/ffmpeg-devel/2024-August/332034.html), if one
>> wants to do decoding on upstream kernel on Rockchip boards, they'll need the
>> v4l2codecs PACKAGECONFIG option for gst-plugins-bad recipe selected.
>>
>> They could also disable it for their board if they want to by redefining
>> HAS_HANTRO variable in their own config file? Or in a distro, etc.
> 
> I just wanted to make sure the = in the patches wasn't going to prevent them
> from doing that.
> 

Well, it does. So thanks for bringing this up :)

>>
>>> Personally I would rather see one patch to add this one feature, rather than
>>> 8.
>>>
>>
>> Those were separate patches for multiple reasons:
>> - I only tested on 2 vanilla upstream kernel (RK3399, PX30)
>> - I only tested on 1 downstream upstream-based kernel (RK3588)
>> - the rest is untested (not even built)
>> - All patches except the one for RK3588(s) could be backported to other
>> branches (e.g. scarthgap); this turned out to be incorrect because the
>> :rockchip OVERRIDES isn't available in that branch (but we can fix this :)
>> ).
>>
>> I wanted to provide also the context for tests I've performed on those
>> boards in the commit log. It could be quite long if I put all tests for all
>> SoC in the same commit log (but that's fine with me).
> 
> Have you seen the size of some of my commit messages? ;-) I don't mind a large
> commit message. I've build-tested your patchset against all rockchip MACHINEs
> and it builds fine. When it comes time to backport, the patch can be adjusted
> as necessary.
> 

ACK, will do.

>>> Should we add a note in the README about this, or perhaps the gstreamer
>>> bbappend?
>>>
>>
>> What exactly would you like to see there?
> 
> Nobody starts by reading the README, but if a user is browsing the MACHINE
> files, notices the HANTRO line and is looking for a quick understanding, they
> might reach for the README file then? You know that HANTRO refers to something
> to do with video decoding, does everybody? Even if all you do is duplicate the
> commit message from 1/9 that would be great.
> 

Fair point, the hardest part is now to come up with a proper name for 
the variable that is not misleading.

Something like:

UPSTREAM_STATELESS_VPU ?= "1"

maybe?

I would still add a comment in the README as requested.

Cheers,
Quentin

Re: [yocto-patches] [meta-rockchip PATCH 3/9] rk3399: enable gstreamer v4l2codecs support

[Trevor Woerner]

On Mon 2024-08-26 @ 05:35:14 PM, Quentin Schulz via lists.yoctoproject.org wrote:
> Hi Trevor,
> 
> On 8/23/24 3:55 PM, Trevor Woerner via lists.yoctoproject.org wrote:
> > On Thu 2024-08-22 @ 02:38:11 PM, Quentin Schulz via lists.yoctoproject.org wrote:
> > > Hi Trevor,
> > > 
> > > On 8/22/24 2:23 PM, Trevor Woerner via lists.yoctoproject.org wrote:
> > > > On Wed 2024-08-21 @ 10:17:52 AM, Quentin Schulz via lists.yoctoproject.org wrote:
> > > > > Hi Trevor,
> > > > > 
> > > > > On 8/21/24 6:42 AM, Trevor Woerner via lists.yoctoproject.org wrote:
> > > > > > On Tue 2024-08-20 @ 03:48:16 PM, Quentin Schulz via lists.yoctoproject.org wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > On 8/20/24 2:56 PM, Quentin Schulz wrote:
> > > > > > > > From: Quentin Schulz <quentin.schulz@cherry.de>
> > > > > > > > 
> > > > > > > > RK3399 has a Hantro VPU, so let's set HAS_HANTRO so that gstreamer
> > > > > > > > v4l2codecs plugin can be built.
> > > > > > > > 
> > > > > > > 
> > > > > > > RK3399 actually also has an RKVDEC VPU which is used for decoding H.264, VP9
> > > > > > > and H.265. Currently, rkvdec supports H.264 and VP9 decoding, while Hantro
> > > > > > > supports VP8 and MPEG2 decoding as well as JPEG encoding. Therefore, I'm not
> > > > > > > sure the naming of the variable is proper? Should we go for
> > > > > > > HAS_STATELESS_VPU instead?
> > > > > > 
> > > > > > What about 2 knobs:
> > > > > > 	1. HAS_HANTRO
> > > > > > 	2. HAS_RKVDEC
> > > > > > ?
> > > > > > 
> > > > > > Does gstreamer have knobs for both sets?
> > > > > > 
> > > > > 
> > > > > They are both stateless VPUs, supported by the same kernel API I believe. So
> > > > > the same meson option is used, v4l2codecs, so I don't think we need to have
> > > > > two separate knobs for those? At least, I used the same gstreamer plugin for
> > > > > decoding h264 on PX30 and RK3399 and PX30 uses Hantro while RK3399 uses
> > > > > RKVDEC for that. Similarly, RK3562, RK356x and RK3588(s) all seems to have
> > > > > an RKVDECv2, which likely is also stateless?
> > > > 
> > > > Is this something the user will definitely always want on (i.e. it won't
> > > > work without it) or should we allow the user the choose whether they want it
> > > > enabled or not?
> > > > 
> > > 
> > > Considering that the only other tool I'm aware of for decoding is ffmpeg and
> > > it does not currently support stateless VPUs
> > > (https://ffmpeg.org/pipermail/ffmpeg-devel/2024-August/332034.html), if one
> > > wants to do decoding on upstream kernel on Rockchip boards, they'll need the
> > > v4l2codecs PACKAGECONFIG option for gst-plugins-bad recipe selected.
> > > 
> > > They could also disable it for their board if they want to by redefining
> > > HAS_HANTRO variable in their own config file? Or in a distro, etc.
> > 
> > I just wanted to make sure the = in the patches wasn't going to prevent them
> > from doing that.
> > 
> 
> Well, it does. So thanks for bringing this up :)
> 
> > > 
> > > > Personally I would rather see one patch to add this one feature, rather than
> > > > 8.
> > > > 
> > > 
> > > Those were separate patches for multiple reasons:
> > > - I only tested on 2 vanilla upstream kernel (RK3399, PX30)
> > > - I only tested on 1 downstream upstream-based kernel (RK3588)
> > > - the rest is untested (not even built)
> > > - All patches except the one for RK3588(s) could be backported to other
> > > branches (e.g. scarthgap); this turned out to be incorrect because the
> > > :rockchip OVERRIDES isn't available in that branch (but we can fix this :)
> > > ).
> > > 
> > > I wanted to provide also the context for tests I've performed on those
> > > boards in the commit log. It could be quite long if I put all tests for all
> > > SoC in the same commit log (but that's fine with me).
> > 
> > Have you seen the size of some of my commit messages? ;-) I don't mind a large
> > commit message. I've build-tested your patchset against all rockchip MACHINEs
> > and it builds fine. When it comes time to backport, the patch can be adjusted
> > as necessary.
> > 
> 
> ACK, will do.
> 
> > > > Should we add a note in the README about this, or perhaps the gstreamer
> > > > bbappend?
> > > > 
> > > 
> > > What exactly would you like to see there?
> > 
> > Nobody starts by reading the README, but if a user is browsing the MACHINE
> > files, notices the HANTRO line and is looking for a quick understanding, they
> > might reach for the README file then? You know that HANTRO refers to something
> > to do with video decoding, does everybody? Even if all you do is duplicate the
> > commit message from 1/9 that would be great.
> > 
> 
> Fair point, the hardest part is now to come up with a proper name for the
> variable that is not misleading.
> 
> Something like:
> 
> UPSTREAM_STATELESS_VPU ?= "1"
> 
> maybe?

If one reads the text that scrolls past while booting, the word Hantro does
show up on devices that have it. Could the variable include the string
"hantro" in it somewhere?

> 
> I would still add a comment in the README as requested.
> 
> Cheers,
> Quentin
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#563): https://lists.yoctoproject.org/g/yocto-patches/message/563
> Mute This Topic: https://lists.yoctoproject.org/mt/107999800/900817
> Group Owner: yocto-patches+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/yocto-patches/leave/13168745/900817/63955952/xyzzy [twoerner@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 
> 

Re: [yocto-patches] [meta-rockchip PATCH 3/9] rk3399: enable gstreamer v4l2codecs support

[Quentin Schulz]

Hi Trevor,

On 8/27/24 5:10 PM, Trevor Woerner via lists.yoctoproject.org wrote:
> On Mon 2024-08-26 @ 05:35:14 PM, Quentin Schulz via lists.yoctoproject.org wrote:
>> Hi Trevor,
>>
>> On 8/23/24 3:55 PM, Trevor Woerner via lists.yoctoproject.org wrote:
>>> On Thu 2024-08-22 @ 02:38:11 PM, Quentin Schulz via lists.yoctoproject.org wrote:
>>>> Hi Trevor,
>>>>
>>>> On 8/22/24 2:23 PM, Trevor Woerner via lists.yoctoproject.org wrote:
>>>>> On Wed 2024-08-21 @ 10:17:52 AM, Quentin Schulz via lists.yoctoproject.org wrote:
>>>>>> Hi Trevor,
>>>>>>
>>>>>> On 8/21/24 6:42 AM, Trevor Woerner via lists.yoctoproject.org wrote:
>>>>>>> On Tue 2024-08-20 @ 03:48:16 PM, Quentin Schulz via lists.yoctoproject.org wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 8/20/24 2:56 PM, Quentin Schulz wrote:
>>>>>>>>> From: Quentin Schulz <quentin.schulz@cherry.de>
>>>>>>>>>
>>>>>>>>> RK3399 has a Hantro VPU, so let's set HAS_HANTRO so that gstreamer
>>>>>>>>> v4l2codecs plugin can be built.
>>>>>>>>>
>>>>>>>>
>>>>>>>> RK3399 actually also has an RKVDEC VPU which is used for decoding H.264, VP9
>>>>>>>> and H.265. Currently, rkvdec supports H.264 and VP9 decoding, while Hantro
>>>>>>>> supports VP8 and MPEG2 decoding as well as JPEG encoding. Therefore, I'm not
>>>>>>>> sure the naming of the variable is proper? Should we go for
>>>>>>>> HAS_STATELESS_VPU instead?
>>>>>>>
>>>>>>> What about 2 knobs:
>>>>>>> 	1. HAS_HANTRO
>>>>>>> 	2. HAS_RKVDEC
>>>>>>> ?
>>>>>>>
>>>>>>> Does gstreamer have knobs for both sets?
>>>>>>>
>>>>>>
>>>>>> They are both stateless VPUs, supported by the same kernel API I believe. So
>>>>>> the same meson option is used, v4l2codecs, so I don't think we need to have
>>>>>> two separate knobs for those? At least, I used the same gstreamer plugin for
>>>>>> decoding h264 on PX30 and RK3399 and PX30 uses Hantro while RK3399 uses
>>>>>> RKVDEC for that. Similarly, RK3562, RK356x and RK3588(s) all seems to have
>>>>>> an RKVDECv2, which likely is also stateless?
>>>>>
>>>>> Is this something the user will definitely always want on (i.e. it won't
>>>>> work without it) or should we allow the user the choose whether they want it
>>>>> enabled or not?
>>>>>
>>>>
>>>> Considering that the only other tool I'm aware of for decoding is ffmpeg and
>>>> it does not currently support stateless VPUs
>>>> (https://ffmpeg.org/pipermail/ffmpeg-devel/2024-August/332034.html), if one
>>>> wants to do decoding on upstream kernel on Rockchip boards, they'll need the
>>>> v4l2codecs PACKAGECONFIG option for gst-plugins-bad recipe selected.
>>>>
>>>> They could also disable it for their board if they want to by redefining
>>>> HAS_HANTRO variable in their own config file? Or in a distro, etc.
>>>
>>> I just wanted to make sure the = in the patches wasn't going to prevent them
>>> from doing that.
>>>
>>
>> Well, it does. So thanks for bringing this up :)
>>
>>>>
>>>>> Personally I would rather see one patch to add this one feature, rather than
>>>>> 8.
>>>>>
>>>>
>>>> Those were separate patches for multiple reasons:
>>>> - I only tested on 2 vanilla upstream kernel (RK3399, PX30)
>>>> - I only tested on 1 downstream upstream-based kernel (RK3588)
>>>> - the rest is untested (not even built)
>>>> - All patches except the one for RK3588(s) could be backported to other
>>>> branches (e.g. scarthgap); this turned out to be incorrect because the
>>>> :rockchip OVERRIDES isn't available in that branch (but we can fix this :)
>>>> ).
>>>>
>>>> I wanted to provide also the context for tests I've performed on those
>>>> boards in the commit log. It could be quite long if I put all tests for all
>>>> SoC in the same commit log (but that's fine with me).
>>>
>>> Have you seen the size of some of my commit messages? ;-) I don't mind a large
>>> commit message. I've build-tested your patchset against all rockchip MACHINEs
>>> and it builds fine. When it comes time to backport, the patch can be adjusted
>>> as necessary.
>>>
>>
>> ACK, will do.
>>
>>>>> Should we add a note in the README about this, or perhaps the gstreamer
>>>>> bbappend?
>>>>>
>>>>
>>>> What exactly would you like to see there?
>>>
>>> Nobody starts by reading the README, but if a user is browsing the MACHINE
>>> files, notices the HANTRO line and is looking for a quick understanding, they
>>> might reach for the README file then? You know that HANTRO refers to something
>>> to do with video decoding, does everybody? Even if all you do is duplicate the
>>> commit message from 1/9 that would be great.
>>>
>>
>> Fair point, the hardest part is now to come up with a proper name for the
>> variable that is not misleading.
>>
>> Something like:
>>
>> UPSTREAM_STATELESS_VPU ?= "1"
>>
>> maybe?
> 
> If one reads the text that scrolls past while booting, the word Hantro does
> show up on devices that have it. Could the variable include the string
> "hantro" in it somewhere?
> 

That;s the thing... not all Rockchip stateless VPUs are called Hantro :) 
Some are called rkvdec, some rkvdec2, some hantro. Some SoCs have 
multiple of those :)

It;s a wonderful mess :)

Cheers,
Quentin