| Message ID | 20231017115119.306574-1-r-ravikumar@ti.com |
|---|---|
| State | Accepted |
| Delegated to: | Ryan Eatmon |
| Headers | show |
| Series | [meta-arago,master/kirkstone] gstreamer1.0-plugins-good: wave5-codec: Set max buffers same as min buffers | expand |
Hi Rahul, On 17/10/23 17:21, Rahul T R wrote: > wave5 codec driver can not work with new buffers allocated post stream I am not aware of such bug in wave5 driver also iiuc it's actually not a wave5 driver problem but probably related to v4l2 plugin not able to provide the buffer with same index always to the driver for which the workaround patch 0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch was made to maintain 1-to-1 between v4l2 buffer indices and buffer. I assume the problem related to the workaround not able to handle 1-to-1 for dynamically allocated buffers post streaming right(if required you can probably check with Prasanth if it would be possible to support this), if that's the case then I would suggest to update commit message likewise. > start. To avoid issues because of this set max_buffers same as min > buffers, so that new buffers are not allocated post stream start > > Signed-off-by: Rahul T R <r-ravikumar@ti.com> With the suggested changes in commit message, Reviewed-by: Devarsh Thakkar <devarsht@ti.com> Regards Devarsh > --- > .../0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch b/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch > index a1471661..bf73e257 100644 > --- a/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch > +++ b/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch > @@ -14,9 +14,9 @@ Signed-off-by: Prasanth Babu Mantena <p-mantena@ti.com> > --- > sys/v4l2/gstv4l2bufferpool.c | 17 ++++++++++++++--- > sys/v4l2/gstv4l2bufferpool.h | 2 ++ > - sys/v4l2/gstv4l2object.c | 15 ++++++++++++--- > + sys/v4l2/gstv4l2object.c | 16 +++++++++++++--- > sys/v4l2/gstv4l2videodec.c | 20 ++++++++++++++------ > - 4 files changed, 42 insertions(+), 12 deletions(-) > + 4 files changed, 43 insertions(+), 12 deletions(-) > > diff --git a/sys/v4l2/gstv4l2bufferpool.c b/sys/v4l2/gstv4l2bufferpool.c > index d85f036..e6a60dc 100644 > @@ -86,7 +86,7 @@ index ee60540..eff1cf2 100644 > own_min = MAX (own_min, GST_V4L2_MIN_BUFFERS (obj)); > > /* for the downstream pool, we keep what downstream wants, though ensure > -@@ -5049,8 +5053,13 @@ gst_v4l2_object_decide_allocation (GstV4l2Object * obj, GstQuery * query) > +@@ -5049,8 +5053,14 @@ gst_v4l2_object_decide_allocation (GstV4l2Object * obj, GstQuery * query) > min = MAX (min, GST_V4L2_MIN_BUFFERS (obj)); > > /* To import we need the other pool to hold at least own_min */ > @@ -95,6 +95,7 @@ index ee60540..eff1cf2 100644 > + if (obj_pool == pool) { > + if (0 == strcmp(obj->vcap.driver, "wave5-dec")) { > + min = own_min; > ++ max = min; > + } else { > + min += own_min; > + }
On 13:08-20231018, Devarsh Thakkar wrote: > Hi Rahul, > > On 17/10/23 17:21, Rahul T R wrote: > > wave5 codec driver can not work with new buffers allocated post stream > > I am not aware of such bug in wave5 driver also iiuc it's actually not a wave5 > driver problem but probably related to v4l2 plugin not able to provide the > buffer with same index always to the driver for which the workaround patch > 0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch was made to maintain 1-to-1 > between v4l2 buffer indices and buffer. > > I assume the problem related to the workaround not able to handle 1-to-1 for > dynamically allocated buffers post streaming right(if required you can > probably check with Prasanth if it would be possible to support this), if > that's the case then I would suggest to update commit message likewise. > > > start. To avoid issues because of this set max_buffers same as min > > buffers, so that new buffers are not allocated post stream start > > > > Signed-off-by: Rahul T R <r-ravikumar@ti.com> > > With the suggested changes in commit message, > > Reviewed-by: Devarsh Thakkar <devarsht@ti.com> Agreed. This is not really a bug in driver. It is on the plugin side, where the mapping of buffers with v4l2 indices is not being maintained. Please modify the commit message. With this suggestion, Reviewed-by: Prasanth Babu Mantena <p-mantena@ti.com> > > Regards > Devarsh > > --- > > .../0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch b/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch > > index a1471661..bf73e257 100644 > > --- a/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch > > +++ b/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch > > @@ -14,9 +14,9 @@ Signed-off-by: Prasanth Babu Mantena <p-mantena@ti.com> > > --- > > sys/v4l2/gstv4l2bufferpool.c | 17 ++++++++++++++--- > > sys/v4l2/gstv4l2bufferpool.h | 2 ++ > > - sys/v4l2/gstv4l2object.c | 15 ++++++++++++--- > > + sys/v4l2/gstv4l2object.c | 16 +++++++++++++--- > > sys/v4l2/gstv4l2videodec.c | 20 ++++++++++++++------ > > - 4 files changed, 42 insertions(+), 12 deletions(-) > > + 4 files changed, 43 insertions(+), 12 deletions(-) > > > > diff --git a/sys/v4l2/gstv4l2bufferpool.c b/sys/v4l2/gstv4l2bufferpool.c > > index d85f036..e6a60dc 100644 > > @@ -86,7 +86,7 @@ index ee60540..eff1cf2 100644 > > own_min = MAX (own_min, GST_V4L2_MIN_BUFFERS (obj)); > > > > /* for the downstream pool, we keep what downstream wants, though ensure > > -@@ -5049,8 +5053,13 @@ gst_v4l2_object_decide_allocation (GstV4l2Object * obj, GstQuery * query) > > +@@ -5049,8 +5053,14 @@ gst_v4l2_object_decide_allocation (GstV4l2Object * obj, GstQuery * query) > > min = MAX (min, GST_V4L2_MIN_BUFFERS (obj)); > > > > /* To import we need the other pool to hold at least own_min */ > > @@ -95,6 +95,7 @@ index ee60540..eff1cf2 100644 > > + if (obj_pool == pool) { > > + if (0 == strcmp(obj->vcap.driver, "wave5-dec")) { > > + min = own_min; > > ++ max = min; > > + } else { > > + min += own_min; > > + }
On 18/10/23 13:19, Prasanth Mantena wrote: > On 13:08-20231018, Devarsh Thakkar wrote: >> Hi Rahul, >> >> On 17/10/23 17:21, Rahul T R wrote: >>> wave5 codec driver can not work with new buffers allocated post stream >> I am not aware of such bug in wave5 driver also iiuc it's actually not a wave5 >> driver problem but probably related to v4l2 plugin not able to provide the >> buffer with same index always to the driver for which the workaround patch >> 0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch was made to maintain 1-to-1 >> between v4l2 buffer indices and buffer. >> >> I assume the problem related to the workaround not able to handle 1-to-1 for >> dynamically allocated buffers post streaming right(if required you can >> probably check with Prasanth if it would be possible to support this), if >> that's the case then I would suggest to update commit message likewise. >> >>> start. To avoid issues because of this set max_buffers same as min >>> buffers, so that new buffers are not allocated post stream start >>> >>> Signed-off-by: Rahul T R <r-ravikumar@ti.com> >> With the suggested changes in commit message, >> >> Reviewed-by: Devarsh Thakkar <devarsht@ti.com> > Agreed. This is not really a bug in driver. It is on the plugin side, > where the mapping of buffers with v4l2 indices is not being maintained. > Please modify the commit message. > > With this suggestion, > > Reviewed-by: Prasanth Babu Mantena <p-mantena@ti.com> Hi Devarsh, Prasanth, Thanks for the review I have send a v2 addressing the comments https://lists.yoctoproject.org/g/meta-arago/message/14948 Regards Rahul T R >> Regards >> Devarsh >>> --- >>> .../0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch | 7 ++++--- >>> 1 file changed, 4 insertions(+), 3 deletions(-) >>> >>> diff --git a/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch b/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch >>> index a1471661..bf73e257 100644 >>> --- a/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch >>> +++ b/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch >>> @@ -14,9 +14,9 @@ Signed-off-by: Prasanth Babu Mantena <p-mantena@ti.com> >>> --- >>> sys/v4l2/gstv4l2bufferpool.c | 17 ++++++++++++++--- >>> sys/v4l2/gstv4l2bufferpool.h | 2 ++ >>> - sys/v4l2/gstv4l2object.c | 15 ++++++++++++--- >>> + sys/v4l2/gstv4l2object.c | 16 +++++++++++++--- >>> sys/v4l2/gstv4l2videodec.c | 20 ++++++++++++++------ >>> - 4 files changed, 42 insertions(+), 12 deletions(-) >>> + 4 files changed, 43 insertions(+), 12 deletions(-) >>> >>> diff --git a/sys/v4l2/gstv4l2bufferpool.c b/sys/v4l2/gstv4l2bufferpool.c >>> index d85f036..e6a60dc 100644 >>> @@ -86,7 +86,7 @@ index ee60540..eff1cf2 100644 >>> own_min = MAX (own_min, GST_V4L2_MIN_BUFFERS (obj)); >>> >>> /* for the downstream pool, we keep what downstream wants, though ensure >>> -@@ -5049,8 +5053,13 @@ gst_v4l2_object_decide_allocation (GstV4l2Object * obj, GstQuery * query) >>> +@@ -5049,8 +5053,14 @@ gst_v4l2_object_decide_allocation (GstV4l2Object * obj, GstQuery * query) >>> min = MAX (min, GST_V4L2_MIN_BUFFERS (obj)); >>> >>> /* To import we need the other pool to hold at least own_min */ >>> @@ -95,6 +95,7 @@ index ee60540..eff1cf2 100644 >>> + if (obj_pool == pool) { >>> + if (0 == strcmp(obj->vcap.driver, "wave5-dec")) { >>> + min = own_min; >>> ++ max = min; >>> + } else { >>> + min += own_min; >>> + }
diff --git a/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch b/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch index a1471661..bf73e257 100644 --- a/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch +++ b/meta-arago-extras/recipes-multimedia/gstreamer/gstreamer1.0-plugins-good/0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch @@ -14,9 +14,9 @@ Signed-off-by: Prasanth Babu Mantena <p-mantena@ti.com> --- sys/v4l2/gstv4l2bufferpool.c | 17 ++++++++++++++--- sys/v4l2/gstv4l2bufferpool.h | 2 ++ - sys/v4l2/gstv4l2object.c | 15 ++++++++++++--- + sys/v4l2/gstv4l2object.c | 16 +++++++++++++--- sys/v4l2/gstv4l2videodec.c | 20 ++++++++++++++------ - 4 files changed, 42 insertions(+), 12 deletions(-) + 4 files changed, 43 insertions(+), 12 deletions(-) diff --git a/sys/v4l2/gstv4l2bufferpool.c b/sys/v4l2/gstv4l2bufferpool.c index d85f036..e6a60dc 100644 @@ -86,7 +86,7 @@ index ee60540..eff1cf2 100644 own_min = MAX (own_min, GST_V4L2_MIN_BUFFERS (obj)); /* for the downstream pool, we keep what downstream wants, though ensure -@@ -5049,8 +5053,13 @@ gst_v4l2_object_decide_allocation (GstV4l2Object * obj, GstQuery * query) +@@ -5049,8 +5053,14 @@ gst_v4l2_object_decide_allocation (GstV4l2Object * obj, GstQuery * query) min = MAX (min, GST_V4L2_MIN_BUFFERS (obj)); /* To import we need the other pool to hold at least own_min */ @@ -95,6 +95,7 @@ index ee60540..eff1cf2 100644 + if (obj_pool == pool) { + if (0 == strcmp(obj->vcap.driver, "wave5-dec")) { + min = own_min; ++ max = min; + } else { + min += own_min; + }
wave5 codec driver can not work with new buffers allocated post stream start. To avoid issues because of this set max_buffers same as min buffers, so that new buffers are not allocated post stream start Signed-off-by: Rahul T R <r-ravikumar@ti.com> --- .../0001-v4l2-Changes-for-DMA-Buf-import-j721s2.patch | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)