| Message ID | 20220623232210.18742-1-laurent.pinchart@ideasonboard.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Quoting Laurent Pinchart via libcamera-devel (2022-06-24 00:21:57) > Hello, > > This patch series fixes a long-standing issue in the libcamerasrc > element, namely the fact that it never queues more than one request at a > time. Excellent, I've heard reports of performance on Raspberry Pi with libcamerasrc being half the expected rate as well, so I suspect this was also a cause - and will fix things there. \o/ > It took me quite a while to grasp the implementation of libcamerasrc, > and this probably shows through the patch series. I don't claim any of > this is particularly good, only that it enables libcamerasrc usage with > rkisp1 and vimc, which isn't possible today, and that it didn't > introduce any regression I could notice. > > Reviews from developers with more experience in GStreamer than me would > be appreciated :-) > > Laurent Pinchart (13): > gstreamer: Use gst_task_resume() when available > gstreamer: Inline gst_libcamera_buffer_get_frame_buffer() > gstreamer: Move variable to loop scope > gstreamer: Pass Stream to RequestWrap::addBuffer() > gstreamer: Move timestamp calculation out of pad loop > gstreamer: Rename queued requests queue to queuedRequests_ > gstreamer: Handle completed requests in the libcamerasrc task > gstreamer: Combine the two pad loops in the task run handler > gstreamer: Use dedicated lock for request queues > gstreamer: Fix pads locking > gstreamer: Split request creation to a separate function > gstreamer: Split completed request processing to a separate function > gstreamer: Fix race conditions in task pause/resume > > src/gstreamer/gstlibcamera-utils.cpp | 16 +- > src/gstreamer/gstlibcamera-utils.h | 4 +- > src/gstreamer/gstlibcameraallocator.cpp | 3 +- > src/gstreamer/gstlibcameraallocator.h | 2 +- > src/gstreamer/gstlibcamerapad.cpp | 35 --- > src/gstreamer/gstlibcamerapad.h | 6 - > src/gstreamer/gstlibcamerapool.cpp | 14 -- > src/gstreamer/gstlibcamerapool.h | 4 - > src/gstreamer/gstlibcamerasrc.cpp | 308 ++++++++++++++++-------- > 9 files changed, 227 insertions(+), 165 deletions(-) > > > base-commit: 7ec3bfedbe22962600b438a13d9e13d37d55ce25 > -- > Regards, > > Laurent Pinchart >
On Fri, Jun 24, 2022 at 01:42:46PM +0100, Kieran Bingham wrote: > Quoting Laurent Pinchart via libcamera-devel (2022-06-24 00:21:57) > > Hello, > > > > This patch series fixes a long-standing issue in the libcamerasrc > > element, namely the fact that it never queues more than one request at a > > time. > > Excellent, I've heard reports of performance on Raspberry Pi with > libcamerasrc being half the expected rate as well, so I suspect this was > also a cause - and will fix things there. Any volunteer to test this on Raspberry Pi ? :-) > \o/ > > > It took me quite a while to grasp the implementation of libcamerasrc, > > and this probably shows through the patch series. I don't claim any of > > this is particularly good, only that it enables libcamerasrc usage with > > rkisp1 and vimc, which isn't possible today, and that it didn't > > introduce any regression I could notice. > > > > Reviews from developers with more experience in GStreamer than me would > > be appreciated :-) > > > > Laurent Pinchart (13): > > gstreamer: Use gst_task_resume() when available > > gstreamer: Inline gst_libcamera_buffer_get_frame_buffer() > > gstreamer: Move variable to loop scope > > gstreamer: Pass Stream to RequestWrap::addBuffer() > > gstreamer: Move timestamp calculation out of pad loop > > gstreamer: Rename queued requests queue to queuedRequests_ > > gstreamer: Handle completed requests in the libcamerasrc task > > gstreamer: Combine the two pad loops in the task run handler > > gstreamer: Use dedicated lock for request queues > > gstreamer: Fix pads locking > > gstreamer: Split request creation to a separate function > > gstreamer: Split completed request processing to a separate function > > gstreamer: Fix race conditions in task pause/resume > > > > src/gstreamer/gstlibcamera-utils.cpp | 16 +- > > src/gstreamer/gstlibcamera-utils.h | 4 +- > > src/gstreamer/gstlibcameraallocator.cpp | 3 +- > > src/gstreamer/gstlibcameraallocator.h | 2 +- > > src/gstreamer/gstlibcamerapad.cpp | 35 --- > > src/gstreamer/gstlibcamerapad.h | 6 - > > src/gstreamer/gstlibcamerapool.cpp | 14 -- > > src/gstreamer/gstlibcamerapool.h | 4 - > > src/gstreamer/gstlibcamerasrc.cpp | 308 ++++++++++++++++-------- > > 9 files changed, 227 insertions(+), 165 deletions(-) > > > > > > base-commit: 7ec3bfedbe22962600b438a13d9e13d37d55ce25
Hello Laurent, I think Rishikesh can test this on his Raspberry Pi setup. He's already working on the gstreamer element. cc'ing him here. Regards, Vedant Paranjape On Mon, Jun 27, 2022 at 1:30 AM Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > On Fri, Jun 24, 2022 at 01:42:46PM +0100, Kieran Bingham wrote: > > Quoting Laurent Pinchart via libcamera-devel (2022-06-24 00:21:57) > > > Hello, > > > > > > This patch series fixes a long-standing issue in the libcamerasrc > > > element, namely the fact that it never queues more than one request at > a > > > time. > > > > Excellent, I've heard reports of performance on Raspberry Pi with > > libcamerasrc being half the expected rate as well, so I suspect this was > > also a cause - and will fix things there. > > Any volunteer to test this on Raspberry Pi ? :-) > > > \o/ > > > > > It took me quite a while to grasp the implementation of libcamerasrc, > > > and this probably shows through the patch series. I don't claim any of > > > this is particularly good, only that it enables libcamerasrc usage with > > > rkisp1 and vimc, which isn't possible today, and that it didn't > > > introduce any regression I could notice. > > > > > > Reviews from developers with more experience in GStreamer than me would > > > be appreciated :-) > > > > > > Laurent Pinchart (13): > > > gstreamer: Use gst_task_resume() when available > > > gstreamer: Inline gst_libcamera_buffer_get_frame_buffer() > > > gstreamer: Move variable to loop scope > > > gstreamer: Pass Stream to RequestWrap::addBuffer() > > > gstreamer: Move timestamp calculation out of pad loop > > > gstreamer: Rename queued requests queue to queuedRequests_ > > > gstreamer: Handle completed requests in the libcamerasrc task > > > gstreamer: Combine the two pad loops in the task run handler > > > gstreamer: Use dedicated lock for request queues > > > gstreamer: Fix pads locking > > > gstreamer: Split request creation to a separate function > > > gstreamer: Split completed request processing to a separate function > > > gstreamer: Fix race conditions in task pause/resume > > > > > > src/gstreamer/gstlibcamera-utils.cpp | 16 +- > > > src/gstreamer/gstlibcamera-utils.h | 4 +- > > > src/gstreamer/gstlibcameraallocator.cpp | 3 +- > > > src/gstreamer/gstlibcameraallocator.h | 2 +- > > > src/gstreamer/gstlibcamerapad.cpp | 35 --- > > > src/gstreamer/gstlibcamerapad.h | 6 - > > > src/gstreamer/gstlibcamerapool.cpp | 14 -- > > > src/gstreamer/gstlibcamerapool.h | 4 - > > > src/gstreamer/gstlibcamerasrc.cpp | 308 ++++++++++++++++-------- > > > 9 files changed, 227 insertions(+), 165 deletions(-) > > > > > > > > > base-commit: 7ec3bfedbe22962600b438a13d9e13d37d55ce25 > > -- > Regards, > > Laurent Pinchart >