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.

[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"


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


Reply via email to