On 12/6/2023 11:03 AM, Andrew Davis wrote:
On 12/6/23 9:56 AM, Devarsh Thakkar wrote:
Hi Andrew,

Thanks for the review.

On 06/12/23 20:41, Andrew Davis wrote:
On 12/6/23 8:48 AM, Devarsh Thakkar wrote:
Add patches to prefer contiguous video format and supporting
dmabuf-import mode for video decoder.

This is to support use-cases involving video codec with other components
viz. GPU, Camera e.t.c.


Why does this "support use-cases"? What is wrong with the default formats?

For 0002-v4l2-Give-preference-to-contiguous-format-if-support.patch :
I think we have certain blocks such as ISP, Camera seem to only support
contigous formats which doesn't go well with gstreamer v4l2 which prefers
non-contigous by default and we fail at negotiation.

For 0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch :
I think this is a local patch which complements a patch already made available
in driver, this to support dmabuf import for the decoder.

Are these posted upstream to upstream gstreamer, if not why? Or should we
instead fix the consuming software to handle the default non-contiguous
formats better..

For 0002-v4l2-Give-preference-to-contiguous-format-if-support.patch :

The crux of the issue is that unlike v4l2 gstreamer has only single GST video format for both NV12 and NV12M (non-contigous) and it prefers to advertise latter during caps negotiation. I think there was a proposal long back to have
separate GST video formats for NV12 and NV12M but that didn't converge,
perhaps they wanted to support this with a VideoMeta based scheme as per logs [0].

Also I see other vendors also carrying similar patch like this locally.

For 0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch :
There again is discussion around this to support this in upstream but the
upstream patch is not yet merged [1].

Anyways, I don't see any harm in applying these patches if they are helping achieve use-cases and I see they are already getting applied for other platforms.


This is a lot of good information, it should be in the commit message.

Devarsh, can you put together a v2 with the above information in the commit message? History like this is very helpful to keep track of so that we do not lose it to the passage of time.


I do not see any harm either, this was not a NAK. But we do need to remember to
work on fixing our drivers/applications to better handle NV12M (seems like
it will remain the default in gstreamer). Otherwise you will have to support this hack forever, and for every distro (we have Debian now, I don't see you updating
gstreamer.deb for the same..).

Andrew

[0] https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/issues/414
[1] :
https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4550/diffs?commit_id=b12eaf65f489bab50917e22242758e000d69df58

Regards
Devarsh

Andrew

Signed-off-by: Devarsh Thakkar <[email protected]>
---
   .../gstreamer/gstreamer1.0-plugins-good_1.20.%.bbappend       | 4 ++++
   1 file changed, 4 insertions(+)

diff --git
a/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good_1.20.%.bbappend
 
b/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good_1.20.%.bbappend
index a36c9760..e14a3c93 100644
---
a/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good_1.20.%.bbappend
+++
b/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good_1.20.%.bbappend
@@ -20,4 +20,8 @@ SRC_URI:append:am62axx = " \
file://0002-v4l2-Give-preference-to-contiguous-format-if-support.patch \
   "
   +SRC_URI:append:am62pxx = " \
+    file://0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch \
+ file://0002-v4l2-Give-preference-to-contiguous-format-if-support.patch \
+"
   PR:append = ".arago0"

--
Ryan Eatmon                [email protected]
-----------------------------------------
Texas Instruments, Inc.  -  LCPD  -  MGTS


-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#15051): 
https://lists.yoctoproject.org/g/meta-arago/message/15051
Mute This Topic: https://lists.yoctoproject.org/mt/103013831/21656
Group Owner: [email protected]
Unsubscribe: 
https://lists.yoctoproject.org/g/meta-arago/leave/10763299/21656/89520264/xyzzy 
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to