From: Rahul T R <r-ravikumar@ti.com>
To: Devarsh Thakkar <devarsht@ti.com>, <meta-arago@lists.yoctoproject.org>
Cc: <reatmon@ti.com>, <j-luthra@ti.com>, <denys@ti.com>,
<b-brnich@ti.com>, <p-mantena@ti.com>
Subject: Re: [meta-arago][master/kirkstone][PATCH 2/2] gstreamer1.0-plugins-good: Add patch to increase min buffers for CSI capture
Date: Tue, 23 Jan 2024 21:04:41 +0530 [thread overview]
Message-ID: <fc9cb1fd-f196-493d-81db-a2a0482c7d23@ti.com> (raw)
In-Reply-To: <1986d806-eb3f-1336-9aa6-008473f48cf7@ti.com>
Hi Devarsh,
On 23/01/24 20:38, Devarsh Thakkar wrote:
> Hi Rahul,
>
> Thanks for the quick response.
>
> On 23/01/24 20:33, Rahul T R wrote:
>> Hi Devarsh,
>>
>> Thanks for the review !
>>
>>
>> On 23/01/24 20:05, Devarsh Thakkar wrote:
>>> Hi Rahul,
>>>
>>> Thanks for the patch.
>>>
>>> On 22/01/24 21:59, Rahul T R wrote:
>>>> This is required to avoid the frame drops and increase the tolrence
>>>> for CSI capture using gsteamer
>>>>
>>>> Signed-off-by: Rahul T R <r-ravikumar@ti.com>
>>>> ---
>>>> ...1-gstv4l2object-Increase-min-buffers.patch | 29 +++++++++++++++++++
>>>> .../gstreamer1.0-plugins-good_1.20.%.bbappend | 3 +-
>>>> 2 files changed, 31 insertions(+), 1 deletion(-)
>>>> create mode 100644
>>>> meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-gstv4l2object-Increase-min-buffers.patch
>>>>
>>>> diff --git
>>>> a/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-gstv4l2object-Increase-min-buffers.patch b/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-gstv4l2object-Increase-min-buffers.patch
>>>> new file mode 100644
>>>> index 00000000..f9ff4faa
>>>> --- /dev/null
>>>> +++
>>>> b/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-gstv4l2object-Increase-min-buffers.patch
>>>> @@ -0,0 +1,29 @@
>>>> +From 9c5e0ee19be372c0477a509889d76ceaf97bdc87 Mon Sep 17 00:00:00 2001
>>>> +From: Rahul T R <r-ravikumar@ti.com>
>>>> +Date: Tue, 10 Oct 2023 14:42:13 +0530
>>>> +Subject: [PATCH] gstv4l2object: Increase min buffers for CSI capture
>>>> +
>>>> +This will improve the latency and tolerance
>>>> +of CSI pipeline
>>>> +
>>> I can understand the tolerance but how does it increase latency.
>> Starvation can lead to DMA error and driver will have to trigger drain DMA,
>> which might lead to some latency
>>
> Ok.
>
>>>> +Signed-off-by: Rahul T R <r-ravikumar@ti.com>
>>>> +---
>>>> + sys/v4l2/gstv4l2object.c | 2 ++
>>>> + 1 file changed, 2 insertions(+)
>>>> +
>>>> +diff --git a/sys/v4l2/gstv4l2object.c b/sys/v4l2/gstv4l2object.c
>>>> +index 4ce9fac..9f5a85b 100644
>>>> +--- a/sys/v4l2/gstv4l2object.c
>>>> ++++ b/sys/v4l2/gstv4l2object.c
>>>> +@@ -5172,6 +5172,8 @@ gst_v4l2_object_decide_allocation (GstV4l2Object *
>>>> obj, GstQuery * query)
>>>> + * pool, we'll need what the driver one, and one more, so we can
>>>> dequeu */
>>>> + if (0 == strcmp(obj->vcap.driver, "wave5-dec")) {
>>>> + own_min = obj->min_buffers + 3;
>>>> ++ } else if (0 == strcmp(obj->vcap.driver, "j721e-csi2rx")) {
>>>> ++ own_min = obj->min_buffers + 4;
>>> Could you please mention in commit message why our csi driver requires extra
>>> buffers ? Is it due to a hardware limitation ? Also mention the use-case and
>>> pipeline for which this is helping.
>> Sure, will add more details
>>
>>> I believe gstreamer has a fail safe flow where it allocates buffer on the fly
>>> in case there is a starvation, ain't that helping ?
>> Can you point me to this code, I am not sure why this is not helping
>> need to analyze the code
>>
> Will share it offline once I get chance, but it was mainly for export mode,
> for import mode it just waits for the downstream element to give back some
> buffers.
Sure
>>> I think it is okay to have this here as short term as we have other patches,
>>> but from upstream point of view it looks like a hack so let's document the
>>> reason why we did it here so that it can act as reference.
>> Agreed, will add more details in the commit
>>
>>> Also I believe this increases the memory footprint in v4l2 export mode ?
>>> For already memory constrained platforms such as am62xSIP this may have a
>>> negative impact. I see some vendors putting in extra env variables to let user
>>> modify the buffering part.
>> This does not affect the export mode, min buffers are increased only
>> for import mode
> Does it help if we increase the allocated buffers in downstream element ?
> If that helps, that would be a better solution as it would avoid impacting
> memory constrained platforms using the csi driver. Else we may have to mark
> this patch to specific platforms only for now where you are running this use-case.
No, increasing number of buffers in downstream pool is not helping
I have observed this on multiple platforms, AM62A, AM68A, TDA4VM
Regards
Rahul T R
> Regards
> Devarsh
>>
>> Regards
>> Rahul T R
>>
>>> Regards
>>> Devarsh
>>>
>>>> + } else {
>>>> + own_min = obj->min_buffers + 1;
>>>> + }
>>>> +--
>>>> +2.34.1
>>>> +
>>>> 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 7ad943f6..bb4fdc09 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
>>>> @@ -5,6 +5,7 @@ SRC_URI:append = " \
>>>> file://0002-Adding-support-for-bayer-formats-with-IR-component.patch \
>>>> file://0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch \
>>>> file://0002-v4l2-Give-preference-to-contiguous-format-if-support.patch \
>>>> + file://0001-gstv4l2object-Increase-min-buffers.patch \
>>>> "
>>>> -PR:append = ".arago1"
>>>> +PR:append = ".arago2"
prev parent reply other threads:[~2024-01-23 15:34 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-22 16:29 [meta-arago][master/kirkstone][PATCH 0/2] Increase min buffers or CSI capture Rahul T R
2024-01-22 16:29 ` [meta-arago][master/kirkstone][PATCH 1/2] gstreamer1.0-plugins-good: Maintain same set of patches for all machines Rahul T R
2024-01-23 14:26 ` Devarsh Thakkar
2024-01-23 14:57 ` Rahul T R
2024-01-22 16:29 ` [meta-arago][master/kirkstone][PATCH 2/2] gstreamer1.0-plugins-good: Add patch to increase min buffers for CSI capture Rahul T R
2024-01-23 14:35 ` Devarsh Thakkar
2024-01-23 15:03 ` Rahul T R
2024-01-23 15:08 ` Devarsh Thakkar
2024-01-23 15:34 ` Rahul T R [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=fc9cb1fd-f196-493d-81db-a2a0482c7d23@ti.com \
--to=r-ravikumar@ti.com \
--cc=b-brnich@ti.com \
--cc=denys@ti.com \
--cc=devarsht@ti.com \
--cc=j-luthra@ti.com \
--cc=meta-arago@lists.yoctoproject.org \
--cc=p-mantena@ti.com \
--cc=reatmon@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).