Message ID | 20221216122939.256534-1-paul.elder@ideasonboard.com |
---|---|
Headers | show |
Series |
|
Related | show |
Hi Paul you have missed porting the newly introduced ISI pipeline handler On Fri, Dec 16, 2022 at 09:29:21PM +0900, Paul Elder via libcamera-devel wrote: > The purpose of this series is to add a new test to lc-compliance that tests > queuing a lot of requests at once in order to ensure that pipeline handlers are > able to handle more requests than they have resources for (like internal buffers > and V4L2 buffer slots) [1]. > > [1] https://bugs.libcamera.org/show_bug.cgi?id=24 > > In order to achieve this, the FrameBufferAllocator had to be adapted to allow an > arbitrary number of buffers to be allocated. But there's also the issue of > reporting the minimum number of requests required by the pipeline handler, which > was solved by creating a new MinimumRequests property. > > Briefly, patches 2 through 10 rework the core and pipeline handlers to use > MinimumRequests and remove bufferCount, while patches 10 through 17 rework > lc-compliance to add the new test and an additional test for MinimumRequests. > > Patch 2 adds the new MinimumRequests property to report the minimum number of > requests needed by the pipeline handler. > > Patch 3 adds a count argument to allocate() so that the number of buffers to > allocate needs to be specified, as it is no longer assumed through bufferCount. > > Patches 4-6 decouple the number of internal buffers and V4L2 buffer slots from > bufferCount in each pipeline handler. > > Patch 9 reworks the V4L2 compatibility layer to not depend on bufferCount. > > Patch 10 removes bufferCount from the StreamConfiguration and everywhere it was > still used, as it is no longer needed. > > Patch 11 fixes a file ordering issue in lc-compliance's meson.build. > > Patches 12-13 does some refactoring in lc-compliance in order to reduce code > duplication. > > Patch 16 adds the test to lc-compliance. > > Patch 17 adds checks in lc-compliance to ensure that requests which failed to be > enqueued are reported as test failure. > > Patch 18 adds another, very short, test to lc-compliance to make sure that the > MinimumRequests property is set in the pipeline handler. > > Nícolas had run v8 of this new lc-compliance test on the raspberrypi, > rkisp1, uvcvideo and vimc pipelines. raspberrypi already handles it > well, while the other three run successfully after applying the series > in [2]. The ipu3 should run fine as well since the series in [2] was > based on the internal queue already present there. A patch for the > simple pipeline is still pending. > > [2] https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022508.html > > The number of buffers to allocate in the lc-compliance test (patch 15) was > hardcoded to 8 since more than that would cause errors when allocating. > > v7: https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022577.html > v6: https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022356.html > v5: https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022098.html > v4: https://lists.libcamera.org/pipermail/libcamera-devel/2021-May/020150.html > v3: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019613.html > v2: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019398.html > v1: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019139.html > > Changes in v9: > - picked up and rebased by Paul > > Changes in v8: > (thanks to Laurent and Kieran) > - Changed internal buffer count constants for pipeline handlers to better values > - Reordered patches to group non-lc-compliance changes together > - Split buffer allocation changes into separate commits for each pipeline > handler (patches 3-7) > - Changed the MinimumRequests property meaning to require that frames aren't > dropped > - Set MinimumRequests on SimplePipeline depending on device and usage of > converter > - Undid definition of default MinimumRequests on CameraSensor constructor > - Updated application-developer and pipeline-handler guides with new allocate() > API and MinimumRequests property > - Added handling for when allocate() returns less buffers than needed in cam and > the capture unit test > - Reworked buffer allocation handling in the raspberrypi pipeline handler > - Moved V4L2 compatibility layer changes to separate commit > - Added patch 10 to fix wrong file order in lc-compliance's meson.build > - Added requests_ member to SimpleCapture to hold ownership of queued > requests during capture > - Moved CameraHolder to new test_base.{cpp,h} files > - Fixed issue in UnbalancedStop test where requests cancelled due to stop() call > were failing the test > - Moved RequiredProperties test to property_test.cpp > - Moved CameraTests to new test_base.{cpp,h} files > > Changes in v7: > (thanks to Kieran and Jacopo) > - Renamed property from MinNumRequests to MinimumRequests > - Changed MinimumRequests property's description > - Added patch 11 to test if MinimumRequests is valid > > Changes in v6: > (thanks to Naushir) > - Fixed style issues > - Changed static_cast to unsigned int when comparing buffer count in > lc-compliance > - Added pipeline prefix to INTERNAL_BUFFER_COUNT and BUFFER_SLOT_COUNT constants > - Removed comment from Raspberrypi MinNumRequests setting > - Switched queueRequests()'s 'buffers' and 'requests' parameters order, since > 'requests' is an output variable > - Added comment to runCaptureSession() > > Changes in v5: > - Rebased on master (now that lc-compliance was refactored to use Googletest) > - Added patches 3, 5, 6 and 8 > - Fixed qcam to use at least two buffers > - Made sure that qcam allocates at least 2 buffers > > Changes in v4: > (thanks to Laurent and Niklas) > - Renamed QueueDepth property to MinNumRequests and better documented it > - Changed patch 6 to also remove bufferCount from android > - Added patch 3 to factor common code in lc-compliance > - Added patch 5 to remove pipeline dependency on bufferCount > > Changes in v3: > - Added patches 1 and 4 to add the QueueDepth property and remove bufferCount > - Made the count argument required in patch 2 > - Added previously missing changes to the gstreamer and V4L2 compatibility > layers > > Changes in v2: > - Renamed and reworded commits and series > - Dropped patches 2 and 3, which were hacks to test, and added patch 1 to add > count to FrameBufferAllocator > - Thanks to Niklas: > - Created new standalone test instead of looping over the other tests > > Nícolas F. R. A. Prado (17): > libcamera: property: Add MinimumRequests property > libcamera: framebuffer_allocator: Make allocate() require count > libcamera: pipeline: raspberrypi: Don't rely on bufferCount > libcamera: pipeline: ipu3: Don't rely on bufferCount > libcamera: pipeline: simple: Don't rely on bufferCount > libcamera: pipeline: rkisp1: Don't rely on bufferCount > libcamera: pipeline: vimc, uvcvideo: Don't rely on bufferCount > v4l2: Allocate buffers based on requested count and MinimumRequests > libcamera: stream: Remove bufferCount > lc-compliance: Fix source file ordering in meson.build > lc-compliance: Move buffer allocation to separate function > lc-compliance: Factor common capture code into SimpleCapture > lc-compliance: Move camera setup to CameraHolder class > lc-compliance: Move role to string conversion to its own function > lc-compliance: Add test to queue more requests than hardware depth > lc-compliance: Check that requests complete successfully > lc-compliance: Add test to ensure MinimumRequests is valid > > Paul Elder (1): > pipeline: rkisp1: Reorder headers to appease the linter > > .../guides/application-developer.rst | 9 +- > Documentation/guides/pipeline-handler.rst | 40 +++-- > include/libcamera/camera.h | 2 +- > include/libcamera/framebuffer_allocator.h | 2 +- > include/libcamera/internal/converter.h | 3 +- > .../internal/converter/converter_v4l2_m2m.h | 9 +- > include/libcamera/internal/pipeline_handler.h | 2 +- > include/libcamera/stream.h | 2 - > src/android/camera_stream.cpp | 2 +- > src/apps/cam/camera_session.cpp | 13 +- > src/apps/lc-compliance/capture_test.cpp | 92 ++++++++--- > src/apps/lc-compliance/meson.build | 4 +- > src/apps/lc-compliance/property_test.cpp | 24 +++ > src/apps/lc-compliance/simple_capture.cpp | 143 ++++++++++++------ > src/apps/lc-compliance/simple_capture.h | 25 ++- > src/apps/lc-compliance/test_base.cpp | 38 +++++ > src/apps/lc-compliance/test_base.h | 31 ++++ > src/apps/qcam/main_window.cpp | 10 +- > src/gstreamer/gstlibcameraallocator.cpp | 5 +- > src/libcamera/camera.cpp | 4 +- > .../converter/converter_v4l2_m2m.cpp | 15 +- > src/libcamera/framebuffer_allocator.cpp | 9 +- > src/libcamera/pipeline/ipu3/cio2.cpp | 5 +- > src/libcamera/pipeline/ipu3/cio2.h | 16 +- > src/libcamera/pipeline/ipu3/imgu.cpp | 12 +- > src/libcamera/pipeline/ipu3/imgu.h | 15 +- > src/libcamera/pipeline/ipu3/ipu3.cpp | 34 ++--- > .../pipeline/raspberrypi/raspberrypi.cpp | 95 ++++-------- > .../pipeline/raspberrypi/rpi_stream.cpp | 39 ++--- > .../pipeline/raspberrypi/rpi_stream.h | 24 ++- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 29 ++-- > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 5 +- > src/libcamera/pipeline/rkisp1/rkisp1_path.h | 4 +- > src/libcamera/pipeline/simple/simple.cpp | 72 +++++++-- > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 17 ++- > src/libcamera/pipeline/vimc/vimc.cpp | 17 ++- > src/libcamera/pipeline_handler.cpp | 1 + > src/libcamera/property_ids.yaml | 21 +++ > src/libcamera/stream.cpp | 12 +- > src/v4l2/v4l2_camera.cpp | 22 ++- > src/v4l2/v4l2_camera.h | 5 +- > src/v4l2/v4l2_camera_proxy.cpp | 10 +- > test/camera/buffer_import.cpp | 11 +- > test/camera/camera_reconfigure.cpp | 5 +- > test/camera/capture.cpp | 11 +- > test/camera/statemachine.cpp | 5 +- > test/fence.cpp | 6 +- > test/libtest/buffer_source.cpp | 4 +- > test/libtest/buffer_source.h | 2 +- > test/mapped-buffer.cpp | 5 +- > test/v4l2_videodevice/buffer_cache.cpp | 3 +- > 51 files changed, 667 insertions(+), 324 deletions(-) > create mode 100644 src/apps/lc-compliance/property_test.cpp > create mode 100644 src/apps/lc-compliance/test_base.cpp > create mode 100644 src/apps/lc-compliance/test_base.h > > -- > 2.35.1 >