| Message ID | 20230127154322.29019-1-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
Quoting Naushir Patuck via libcamera-devel (2023-01-27 15:43:07) > Hi, > > Version 6 of this series addresses the following: > > - Switch the sense of the hint back to how it was in v4. This hint is now called MandatorySteam. > - Move the MandatorySteam hint validation into Camera::queueRequest. > - Remove the "return_newest_frames" config option. As discussed in the discussion for v5, it's a bit redundant. > - Added a bit more documentation for the Unicam buffer config options. > - The biggest changes come in patches 12/15 - 15/15 where I've added multi-stream support to lc-compliance. With that, I've added tests for dual stream captures, optional streams, and use of stream hints. > > Hopefully I've captured all the changes from the feedback, but if I've missed anything, please do let me know. Several of these patches could already apply, but that's breaking the series apart: As I'm aiming to make a release tomorrow - would you prefer to keep this whole series together? or cherry-pick suitable commits (which helps reduce this series anyway?) I think already these could be applied: [v6,02/15] libcamera: pipeline: Add a platform configuration file helper [v6,03/15] pipeline: raspberrypi: Add a pipeline config structure [v6,04/15] pipeline: raspberrypi: Reorder startup drop frame initialisation ... [v6,06/15] libcamera: pipeline: build: Add pipeline_data_dir variable [v6,07/15] pipeline: raspberrypi: Read config parameters from a file ... [v6,10/15] pipeline: raspberrypi: Add a parameter to disable startup drop frames ? > > One thing to note - lc-compliance does not run correctly on the Raspberry Pi platform. The reason for this is detailed in [1]. In order to test the lc-compliance app you need to make the following change: > This certainly needs some more consideration and whatever we do - lc-compliance should be updated (or left) to make sure that the correct behaviour is validated. > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index 9920f6bb01a7..656e0f623c44 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -1135,6 +1135,8 @@ void PipelineHandlerRPi::stopDevice(Camera *camera) > data->bayerQueue_ = {}; > data->embeddedQueue_ = {}; > > + data->freeBuffers(); > + > /* Stop the IPA. */ > data->ipa_->stop(); > > This is something that we do need to address at some point... > > Regards, > Naush > > [1] https://lists.libcamera.org/pipermail/libcamera-devel/2022-December/036136.html > > Naushir Patuck (15): > libcamera: stream: Add stream hints to StreamConfiguration > libcamera: pipeline: Add a platform configuration file helper > pipeline: raspberrypi: Add a pipeline config structure > pipeline: raspberrypi: Reorder startup drop frame initialisation > pipeline: raspberrypi: Handle MandatoryStream hints for Unicam Image > libcamera: pipeline: build: Add pipeline_data_dir variable > pipeline: raspberrypi: Read config parameters from a file > pipeline: raspberrypi: Handle MandatoryStream hints for ISP Output0 > libcamera: camera: Validate MandatoryStream in queueRequest() > pipeline: raspberrypi: Add a parameter to disable startup drop frames > pipeline: raspberrypi: Add minimal memory usage config file > libcamera: apps: lcc: Make rolesMap global > libcamera: apps: lcc: Add multi-stream capture test framework > libcamera: apps: lcc: Add optional stream tests > libcamera: apps: lcc: Add stream hints test > > Documentation/environment_variables.rst | 5 + > include/libcamera/internal/pipeline_handler.h | 3 + > include/libcamera/stream.h | 8 + > src/apps/lc-compliance/capture_test.cpp | 136 ++++++++- > src/apps/lc-compliance/meson.build | 1 + > src/apps/lc-compliance/multi_capture.cpp | 288 ++++++++++++++++++ > src/apps/lc-compliance/multi_capture.h | 87 ++++++ > src/libcamera/camera.cpp | 12 + > src/libcamera/pipeline/meson.build | 3 + > .../pipeline/raspberrypi/data/example.yaml | 37 +++ > .../pipeline/raspberrypi/data/meson.build | 9 + > .../raspberrypi/data/minimal_mem.yaml | 37 +++ > .../pipeline/raspberrypi/meson.build | 2 + > .../pipeline/raspberrypi/raspberrypi.cpp | 190 ++++++++++-- > src/libcamera/pipeline_handler.cpp | 57 ++++ > src/libcamera/stream.cpp | 25 ++ > 16 files changed, 862 insertions(+), 38 deletions(-) > create mode 100644 src/apps/lc-compliance/multi_capture.cpp > create mode 100644 src/apps/lc-compliance/multi_capture.h > create mode 100644 src/libcamera/pipeline/raspberrypi/data/example.yaml > create mode 100644 src/libcamera/pipeline/raspberrypi/data/meson.build > create mode 100644 src/libcamera/pipeline/raspberrypi/data/minimal_mem.yaml > > -- > 2.25.1 >
Hi Kieran, On Mon, 30 Jan 2023 at 14:04, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Quoting Naushir Patuck via libcamera-devel (2023-01-27 15:43:07) > > Hi, > > > > Version 6 of this series addresses the following: > > > > - Switch the sense of the hint back to how it was in v4. This hint is now called MandatorySteam. > > - Move the MandatorySteam hint validation into Camera::queueRequest. > > - Remove the "return_newest_frames" config option. As discussed in the discussion for v5, it's a bit redundant. > > - Added a bit more documentation for the Unicam buffer config options. > > - The biggest changes come in patches 12/15 - 15/15 where I've added multi-stream support to lc-compliance. With that, I've added tests for dual stream captures, optional streams, and use of stream hints. > > > > Hopefully I've captured all the changes from the feedback, but if I've missed anything, please do let me know. > > Several of these patches could already apply, but that's breaking the > series apart: > > As I'm aiming to make a release tomorrow - would you prefer to keep this > whole series together? or cherry-pick suitable commits (which helps > reduce this series anyway?) Sorry to not reply earlier, I'm away from the office this week. If you have not yet made the release, I am happy to split up the series and get a few things in earlier. > > I think already these could be applied: > > [v6,02/15] libcamera: pipeline: Add a platform configuration file helper > [v6,03/15] pipeline: raspberrypi: Add a pipeline config structure > [v6,04/15] pipeline: raspberrypi: Reorder startup drop frame initialisation > ... > [v6,06/15] libcamera: pipeline: build: Add pipeline_data_dir variable > [v6,07/15] pipeline: raspberrypi: Read config parameters from a file > ... > [v6,10/15] pipeline: raspberrypi: Add a parameter to disable startup drop frames > > ? > > > > > One thing to note - lc-compliance does not run correctly on the Raspberry Pi platform. The reason for this is detailed in [1]. In order to test the lc-compliance app you need to make the following change: > > > > This certainly needs some more consideration and whatever we do - > lc-compliance should be updated (or left) to make sure that the correct > behaviour is validated. Indeed. I have a "fix" for lc-compliance to work with the RPi pipeline handler correctly, but of course this will (likely) break ipu3 and rkisp so I cannot check it in. Regards, Naush > > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > index 9920f6bb01a7..656e0f623c44 100644 > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > @@ -1135,6 +1135,8 @@ void PipelineHandlerRPi::stopDevice(Camera *camera) > > data->bayerQueue_ = {}; > > data->embeddedQueue_ = {}; > > > > + data->freeBuffers(); > > + > > /* Stop the IPA. */ > > data->ipa_->stop(); > > > > This is something that we do need to address at some point... > > > > Regards, > > Naush > > > > [1] https://lists.libcamera.org/pipermail/libcamera-devel/2022-December/036136.html > > > > Naushir Patuck (15): > > libcamera: stream: Add stream hints to StreamConfiguration > > libcamera: pipeline: Add a platform configuration file helper > > pipeline: raspberrypi: Add a pipeline config structure > > pipeline: raspberrypi: Reorder startup drop frame initialisation > > pipeline: raspberrypi: Handle MandatoryStream hints for Unicam Image > > libcamera: pipeline: build: Add pipeline_data_dir variable > > pipeline: raspberrypi: Read config parameters from a file > > pipeline: raspberrypi: Handle MandatoryStream hints for ISP Output0 > > libcamera: camera: Validate MandatoryStream in queueRequest() > > pipeline: raspberrypi: Add a parameter to disable startup drop frames > > pipeline: raspberrypi: Add minimal memory usage config file > > libcamera: apps: lcc: Make rolesMap global > > libcamera: apps: lcc: Add multi-stream capture test framework > > libcamera: apps: lcc: Add optional stream tests > > libcamera: apps: lcc: Add stream hints test > > > > Documentation/environment_variables.rst | 5 + > > include/libcamera/internal/pipeline_handler.h | 3 + > > include/libcamera/stream.h | 8 + > > src/apps/lc-compliance/capture_test.cpp | 136 ++++++++- > > src/apps/lc-compliance/meson.build | 1 + > > src/apps/lc-compliance/multi_capture.cpp | 288 ++++++++++++++++++ > > src/apps/lc-compliance/multi_capture.h | 87 ++++++ > > src/libcamera/camera.cpp | 12 + > > src/libcamera/pipeline/meson.build | 3 + > > .../pipeline/raspberrypi/data/example.yaml | 37 +++ > > .../pipeline/raspberrypi/data/meson.build | 9 + > > .../raspberrypi/data/minimal_mem.yaml | 37 +++ > > .../pipeline/raspberrypi/meson.build | 2 + > > .../pipeline/raspberrypi/raspberrypi.cpp | 190 ++++++++++-- > > src/libcamera/pipeline_handler.cpp | 57 ++++ > > src/libcamera/stream.cpp | 25 ++ > > 16 files changed, 862 insertions(+), 38 deletions(-) > > create mode 100644 src/apps/lc-compliance/multi_capture.cpp > > create mode 100644 src/apps/lc-compliance/multi_capture.h > > create mode 100644 src/libcamera/pipeline/raspberrypi/data/example.yaml > > create mode 100644 src/libcamera/pipeline/raspberrypi/data/meson.build > > create mode 100644 src/libcamera/pipeline/raspberrypi/data/minimal_mem.yaml > > > > -- > > 2.25.1 > >
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 9920f6bb01a7..656e0f623c44 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1135,6 +1135,8 @@ void PipelineHandlerRPi::stopDevice(Camera *camera) data->bayerQueue_ = {}; data->embeddedQueue_ = {}; + data->freeBuffers(); + /* Stop the IPA. */ data->ipa_->stop();