[libcamera-devel,v2,0/3] raspberrypi: Rework the internal buffer allocation scheme
mbox series

Message ID 20211112100305.2217099-1-naush@raspberrypi.com
Headers show
Series
  • raspberrypi: Rework the internal buffer allocation scheme
Related show

Message

Naushir Patuck Nov. 12, 2021, 10:03 a.m. UTC 
Hi,

The second version of this series changes the following over v1:

- Apply Laurent's suggestions in patch 2/3.
- Allocate 2 internal buffers for Unicam instead of one in patch 2/3.

Thanks,
Naush

Naushir Patuck (3):
  pipeline: raspberrypi: Add const qualifer in isRaw()
  pipeline: raspberrypi: Rework the internal buffer allocation scheme
  pipeline: raspberrypi: Increase the V4L2BufferCache slot allocations

 .../pipeline/raspberrypi/raspberrypi.cpp      | 45 ++++++++++++++-----
 .../pipeline/raspberrypi/rpi_stream.cpp       |  8 ++++
 2 files changed, 41 insertions(+), 12 deletions(-)

Comments

Kieran Bingham Nov. 17, 2021, 4:46 p.m. UTC | #1 
Quoting Naushir Patuck (2021-11-12 10:03:02)
> Hi,
> 
> The second version of this series changes the following over v1:
> 
> - Apply Laurent's suggestions in patch 2/3.
> - Allocate 2 internal buffers for Unicam instead of one in patch 2/3.

I'm getting curious about that VIDEO_MAX_FRAME and whether we should
just always allocate that in the V4L2VideoDevice class, and then do away
with the importBuffers(count); function altogether, as it would be
redundant ... But that's not necessarily a requriement for this series -
just thinking out loud.

Would you like this series merged? Or are we waiting on confirmation of
the symptoms/issues from Roman before we commit to these patches?

--
Kieran


> 
> Thanks,
> Naush
> 
> Naushir Patuck (3):
>   pipeline: raspberrypi: Add const qualifer in isRaw()
>   pipeline: raspberrypi: Rework the internal buffer allocation scheme
>   pipeline: raspberrypi: Increase the V4L2BufferCache slot allocations
> 
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 45 ++++++++++++++-----
>  .../pipeline/raspberrypi/rpi_stream.cpp       |  8 ++++
>  2 files changed, 41 insertions(+), 12 deletions(-)
> 
> -- 
> 2.25.1
>
Naushir Patuck Nov. 17, 2021, 9:50 p.m. UTC | #2 
Hi Kieran,

On Wed, 17 Nov 2021 at 16:47, Kieran Bingham <
kieran.bingham@ideasonboard.com> wrote:

> Quoting Naushir Patuck (2021-11-12 10:03:02)
> > Hi,
> >
> > The second version of this series changes the following over v1:
> >
> > - Apply Laurent's suggestions in patch 2/3.
> > - Allocate 2 internal buffers for Unicam instead of one in patch 2/3.
>
> I'm getting curious about that VIDEO_MAX_FRAME and whether we should
> just always allocate that in the V4L2VideoDevice class, and then do away
> with the importBuffers(count); function altogether, as it would be
> redundant ... But that's not necessarily a requriement for this series -
> just thinking out loud.
>

Yes, that could be an option, buffer slots are fairly small storage
elements,
so it should not increase the memory footprint too much.


>
> Would you like this series merged? Or are we waiting on confirmation of
> the symptoms/issues from Roman before we commit to these patches?
>

I'm inclined to say we merge this series now, as it does fix
overallocations.
Given we don't know exactly what the cause of Roman's issues are, I would
prefer to fix time on top of these patches.

Regards,
Naush


>
> --
> Kieran
>
>
> >
> > Thanks,
> > Naush
> >
> > Naushir Patuck (3):
> >   pipeline: raspberrypi: Add const qualifer in isRaw()
> >   pipeline: raspberrypi: Rework the internal buffer allocation scheme
> >   pipeline: raspberrypi: Increase the V4L2BufferCache slot allocations
> >
> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 45 ++++++++++++++-----
> >  .../pipeline/raspberrypi/rpi_stream.cpp       |  8 ++++
> >  2 files changed, 41 insertions(+), 12 deletions(-)
> >
> > --
> > 2.25.1
> >
>