From: Ryan Eatmon <reatmon@ti.com>
To: Andrew Davis <afd@ti.com>, Devarsh Thakkar <devarsht@ti.com>,
<meta-arago@lists.yoctoproject.org>, <praneeth@ti.com>,
<denys@konsulko.com>
Cc: <nm@ti.com>, <vigneshr@ti.com>, <a-bhatia1@ti.com>,
<j-luthra@ti.com>, <khasim@ti.com>, <nsekhar@ti.com>, <rs@ti.com>,
<k-bhargav@ti.com>, <a-limaye@ti.com>, <shyam.jagannathan@ti.com>,
<r-ravikumar@ti.com>, <vijayp@ti.com>, <detheridge@ti.com>
Subject: Re: [meta-arago] [kirkstone] [PATCH] receipes-multimedia: gstreamer: Add patches for AM62P
Date: Fri, 8 Dec 2023 08:55:11 -0600 [thread overview]
Message-ID: <00f90f45-2f2c-4540-b973-e57099bce742@ti.com> (raw)
In-Reply-To: <17a8d9a4-499c-4010-a5c9-e77969a377e8@ti.com>
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 <devarsht@ti.com>
>>>> ---
>>>> .../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 reatmon@ti.com
-----------------------------------------
Texas Instruments, Inc. - LCPD - MGTS
prev parent reply other threads:[~2023-12-08 14:55 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-06 14:48 [meta-arago] [kirkstone] [PATCH] receipes-multimedia: gstreamer: Add patches for AM62P Devarsh Thakkar
2023-12-06 15:11 ` Andrew Davis
2023-12-06 15:56 ` Devarsh Thakkar
2023-12-06 16:02 ` Rahul T R
2023-12-06 17:03 ` Andrew Davis
2023-12-08 14:55 ` Ryan Eatmon [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=00f90f45-2f2c-4540-b973-e57099bce742@ti.com \
--to=reatmon@ti.com \
--cc=a-bhatia1@ti.com \
--cc=a-limaye@ti.com \
--cc=afd@ti.com \
--cc=denys@konsulko.com \
--cc=detheridge@ti.com \
--cc=devarsht@ti.com \
--cc=j-luthra@ti.com \
--cc=k-bhargav@ti.com \
--cc=khasim@ti.com \
--cc=meta-arago@lists.yoctoproject.org \
--cc=nm@ti.com \
--cc=nsekhar@ti.com \
--cc=praneeth@ti.com \
--cc=r-ravikumar@ti.com \
--cc=rs@ti.com \
--cc=shyam.jagannathan@ti.com \
--cc=vigneshr@ti.com \
--cc=vijayp@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).