Message ID | 20210805222436.6263-1-laurent.pinchart@ideasonboard.com |
---|---|
Headers | show |
Series |
|
Related | show |
Hi Martin, Would you be able to test this series on the Librem5 ? I'd like to make sure it doesn't cause any regression. On Fri, Aug 06, 2021 at 01:24:26AM +0300, Laurent Pinchart wrote: > Hello, > > Another day, another patch series that has been sitting in my tree and > that I'd like to stop rebasing. > > The series reworks the simple pipeline handler to correctly support > streaming from multiple cameras concurrently. The reason I haven't > posted it yet is that the libcamera core gets doesn't support this > feature yet. It's easy to work around by removing some checks for > testing, but will take more time to fix correctly. > > The simple pipeline handler already supports registering multiple > cameras, and relies on the libcamera core blocking concurrent access to > different cameras backed by the same media device. It thus doesn't > implement internally mutual exclusion between cameras that share > hardware resources, but, based on the assumption that a single camera > will be used at a time, implements camera operation around a global > active camera pointer. > > Patch 01/10 is the only libcamera core patch. It extends the MediaEntity > class to expose the type of userspace interface that the entity > implements. Patches 02/10 to 06/10 then rework the way the pipeline > handler stores data about pipelines and accesses video devices and > subdevs. > > Patch 02/10 extends the SimpleCameraData entity representation to store > more information about graph traversal, and patch 04/10 stores the > entity corresponding to the video node with all other pipeline entities > in SimpleCameraData. Patches 03/10, 05/10 and 06/10 generalize handling > of video devices and subdev by opening them all together at match time > and storing them in a single container in the SimplePipelineHandler > class. > > With this done, path 07/10 adds an entity reservation mechanism in the > SimplePipelineHandler class, to handle mutual exclusion when multiple > cameras use the same entities. With this change, the simple pipeline > handler handles concurrent access to cameras internally, without > depending on mutual exclusion implemented by the libcamera core anymore. > This doesn't result in any change of behaviour, but paves the way to > concurrent usage of multiple cameras that don't share entities once the > restriction in the libcamera core will be lifted. > > Patch 08/10 continues in that direction by moving the converter from the > SimplePipelineHandler class to the SimpleCameraData class. This allows > usage of the converter by multiple cameras concurrently. Patch 09/10 > then moves the bufferReady handler to the SimpleCameraData class as > well, to handle buffer completion in the correct context without relying > on the global active camera pointer. > > Finally, patch 10/10 removes the active camera pointer as it's not used > anymore. > > This series has only been partially tested, as the libcamera core > doesn't correctly support concurrent streaming from cameras belonging to > the same pipeline handler. The code was tested with a single camera and > didn't introduce any regression, and was also tested on i.MX8MP with a > few hacks in the libcamera core to lift the restrictions and a few > out-of-tree patches to support the i.MX8MP (those are not ready for > submission yet). I would thus understand a reluctance to get this merged > already, even if I believe that the absence of regression, combined with > the cleaner (in my opinion) implementation of the simple pipeline > handler are good enough reasons by themselves to already merge this. > > Laurent Pinchart (10): > libcamera: media_object: Expose entity type > libcamera: pipeline: simple: Add sink and source pads to entity data > libcamera: pipeline: simple: Delay opening of video device until > init() > libcamera: pipeline: simple: Store video node entity in camera data > libcamera: pipeline: simple: Store all entity devices in common map > libcamera: pipeline: simple: Open all video devices at match() time > libcamera: pipeline: simple: Add pipeline pad reservation mechanism > libcamera: pipeline: simple: Move converter to SimpleCameraData > libcamera: pipeline: simple: Move bufferReady handler to > SimpleCameraData > libcamera: pipeline: simple: Remove > SimplePipelineHandler::activeCamera_ > > include/libcamera/internal/media_object.h | 11 +- > src/libcamera/media_device.cpp | 9 +- > src/libcamera/media_object.cpp | 50 ++- > src/libcamera/pipeline/simple/simple.cpp | 506 ++++++++++++++-------- > 4 files changed, 377 insertions(+), 199 deletions(-)
Am Sonntag, dem 08.08.2021 um 18:07 +0300 schrieb Laurent Pinchart: > Hi Martin, > > Would you be able to test this series on the Librem5 ? I'd like to > make > sure it doesn't cause any regression. hi Laurent, could you send me the patches directly and tell me where they apply? thanks, martin > > On Fri, Aug 06, 2021 at 01:24:26AM +0300, Laurent Pinchart wrote: > > Hello, > > > > Another day, another patch series that has been sitting in my tree > > and > > that I'd like to stop rebasing. > > > > The series reworks the simple pipeline handler to correctly support > > streaming from multiple cameras concurrently. The reason I haven't > > posted it yet is that the libcamera core gets doesn't support this > > feature yet. It's easy to work around by removing some checks for > > testing, but will take more time to fix correctly. > > > > The simple pipeline handler already supports registering multiple > > cameras, and relies on the libcamera core blocking concurrent > > access to > > different cameras backed by the same media device. It thus doesn't > > implement internally mutual exclusion between cameras that share > > hardware resources, but, based on the assumption that a single > > camera > > will be used at a time, implements camera operation around a global > > active camera pointer. > > > > Patch 01/10 is the only libcamera core patch. It extends the > > MediaEntity > > class to expose the type of userspace interface that the entity > > implements. Patches 02/10 to 06/10 then rework the way the pipeline > > handler stores data about pipelines and accesses video devices and > > subdevs. > > > > Patch 02/10 extends the SimpleCameraData entity representation to > > store > > more information about graph traversal, and patch 04/10 stores the > > entity corresponding to the video node with all other pipeline > > entities > > in SimpleCameraData. Patches 03/10, 05/10 and 06/10 generalize > > handling > > of video devices and subdev by opening them all together at match > > time > > and storing them in a single container in the SimplePipelineHandler > > class. > > > > With this done, path 07/10 adds an entity reservation mechanism in > > the > > SimplePipelineHandler class, to handle mutual exclusion when > > multiple > > cameras use the same entities. With this change, the simple > > pipeline > > handler handles concurrent access to cameras internally, without > > depending on mutual exclusion implemented by the libcamera core > > anymore. > > This doesn't result in any change of behaviour, but paves the way > > to > > concurrent usage of multiple cameras that don't share entities once > > the > > restriction in the libcamera core will be lifted. > > > > Patch 08/10 continues in that direction by moving the converter > > from the > > SimplePipelineHandler class to the SimpleCameraData class. This > > allows > > usage of the converter by multiple cameras concurrently. Patch > > 09/10 > > then moves the bufferReady handler to the SimpleCameraData class as > > well, to handle buffer completion in the correct context without > > relying > > on the global active camera pointer. > > > > Finally, patch 10/10 removes the active camera pointer as it's not > > used > > anymore. > > > > This series has only been partially tested, as the libcamera core > > doesn't correctly support concurrent streaming from cameras > > belonging to > > the same pipeline handler. The code was tested with a single camera > > and > > didn't introduce any regression, and was also tested on i.MX8MP > > with a > > few hacks in the libcamera core to lift the restrictions and a few > > out-of-tree patches to support the i.MX8MP (those are not ready for > > submission yet). I would thus understand a reluctance to get this > > merged > > already, even if I believe that the absence of regression, combined > > with > > the cleaner (in my opinion) implementation of the simple pipeline > > handler are good enough reasons by themselves to already merge > > this. > > > > Laurent Pinchart (10): > > libcamera: media_object: Expose entity type > > libcamera: pipeline: simple: Add sink and source pads to entity > > data > > libcamera: pipeline: simple: Delay opening of video device until > > init() > > libcamera: pipeline: simple: Store video node entity in camera > > data > > libcamera: pipeline: simple: Store all entity devices in common > > map > > libcamera: pipeline: simple: Open all video devices at match() > > time > > libcamera: pipeline: simple: Add pipeline pad reservation > > mechanism > > libcamera: pipeline: simple: Move converter to SimpleCameraData > > libcamera: pipeline: simple: Move bufferReady handler to > > SimpleCameraData > > libcamera: pipeline: simple: Remove > > SimplePipelineHandler::activeCamera_ > > > > include/libcamera/internal/media_object.h | 11 +- > > src/libcamera/media_device.cpp | 9 +- > > src/libcamera/media_object.cpp | 50 ++- > > src/libcamera/pipeline/simple/simple.cpp | 506 ++++++++++++++---- > > ---- > > 4 files changed, 377 insertions(+), 199 deletions(-) >
Hi Martin, On Tue, Aug 24, 2021 at 04:57:15PM +0200, Martin Kepplinger wrote: > Am Sonntag, dem 08.08.2021 um 18:07 +0300 schrieb Laurent Pinchart: > > Hi Martin, > > > > Would you be able to test this series on the Librem5 ? I'd like to make > > sure it doesn't cause any regression. > > hi Laurent, could you send me the patches directly and tell me where > they apply? Better, I can provide a branch :-) https://git.libcamera.org/libcamera/pinchartl/libcamera.git/log/?h=simple/multicam > > On Fri, Aug 06, 2021 at 01:24:26AM +0300, Laurent Pinchart wrote: > > > Hello, > > > > > > Another day, another patch series that has been sitting in my tree and > > > that I'd like to stop rebasing. > > > > > > The series reworks the simple pipeline handler to correctly support > > > streaming from multiple cameras concurrently. The reason I haven't > > > posted it yet is that the libcamera core gets doesn't support this > > > feature yet. It's easy to work around by removing some checks for > > > testing, but will take more time to fix correctly. > > > > > > The simple pipeline handler already supports registering multiple > > > cameras, and relies on the libcamera core blocking concurrent access to > > > different cameras backed by the same media device. It thus doesn't > > > implement internally mutual exclusion between cameras that share > > > hardware resources, but, based on the assumption that a single camera > > > will be used at a time, implements camera operation around a global > > > active camera pointer. > > > > > > Patch 01/10 is the only libcamera core patch. It extends the MediaEntity > > > class to expose the type of userspace interface that the entity > > > implements. Patches 02/10 to 06/10 then rework the way the pipeline > > > handler stores data about pipelines and accesses video devices and > > > subdevs. > > > > > > Patch 02/10 extends the SimpleCameraData entity representation to store > > > more information about graph traversal, and patch 04/10 stores the > > > entity corresponding to the video node with all other pipeline entities > > > in SimpleCameraData. Patches 03/10, 05/10 and 06/10 generalize handling > > > of video devices and subdev by opening them all together at match time > > > and storing them in a single container in the SimplePipelineHandler > > > class. > > > > > > With this done, path 07/10 adds an entity reservation mechanism in the > > > SimplePipelineHandler class, to handle mutual exclusion when multiple > > > cameras use the same entities. With this change, the simple pipeline > > > handler handles concurrent access to cameras internally, without > > > depending on mutual exclusion implemented by the libcamera core anymore. > > > This doesn't result in any change of behaviour, but paves the way to > > > concurrent usage of multiple cameras that don't share entities once the > > > restriction in the libcamera core will be lifted. > > > > > > Patch 08/10 continues in that direction by moving the converter from the > > > SimplePipelineHandler class to the SimpleCameraData class. This allows > > > usage of the converter by multiple cameras concurrently. Patch> 09/10 > > > then moves the bufferReady handler to the SimpleCameraData class as > > > well, to handle buffer completion in the correct context without relying > > > on the global active camera pointer. > > > > > > Finally, patch 10/10 removes the active camera pointer as it's not used > > > anymore. > > > > > > This series has only been partially tested, as the libcamera core > > > doesn't correctly support concurrent streaming from cameras belonging to > > > the same pipeline handler. The code was tested with a single camera and > > > didn't introduce any regression, and was also tested on i.MX8MP with a > > > few hacks in the libcamera core to lift the restrictions and a few > > > out-of-tree patches to support the i.MX8MP (those are not ready for > > > submission yet). I would thus understand a reluctance to get this merged > > > already, even if I believe that the absence of regression, combined with > > > the cleaner (in my opinion) implementation of the simple pipeline > > > handler are good enough reasons by themselves to already merge this. > > > > > > Laurent Pinchart (10): > > > libcamera: media_object: Expose entity type > > > libcamera: pipeline: simple: Add sink and source pads to entity data > > > libcamera: pipeline: simple: Delay opening of video device until init() > > > libcamera: pipeline: simple: Store video node entity in camera data > > > libcamera: pipeline: simple: Store all entity devices in common map > > > libcamera: pipeline: simple: Open all video devices at match() time > > > libcamera: pipeline: simple: Add pipeline pad reservation mechanism > > > libcamera: pipeline: simple: Move converter to SimpleCameraData > > > libcamera: pipeline: simple: Move bufferReady handler to SimpleCameraData > > > libcamera: pipeline: simple: Remove SimplePipelineHandler::activeCamera_ > > > > > > include/libcamera/internal/media_object.h | 11 +- > > > src/libcamera/media_device.cpp | 9 +- > > > src/libcamera/media_object.cpp | 50 ++- > > > src/libcamera/pipeline/simple/simple.cpp | 506 ++++++++++++++---- > > > ---- > > > 4 files changed, 377 insertions(+), 199 deletions(-)
Am Mittwoch, dem 25.08.2021 um 04:34 +0300 schrieb Laurent Pinchart: > Hi Martin, > > On Tue, Aug 24, 2021 at 04:57:15PM +0200, Martin Kepplinger wrote: > > Am Sonntag, dem 08.08.2021 um 18:07 +0300 schrieb Laurent Pinchart: > > > Hi Martin, > > > > > > Would you be able to test this series on the Librem5 ? I'd like > > > to make > > > sure it doesn't cause any regression. > > > > hi Laurent, could you send me the patches directly and tell me > > where > > they apply? > > Better, I can provide a branch :-) > > https://git.libcamera.org/libcamera/pinchartl/libcamera.git/log/?h=simple/multicam > > > hi Laurent, using the above branch, the cam application streams the highest mode (by default) just like the libcamera master branch. I don't see any difference. Do you want to have anything else in this regard? thanks, martin
Hi Martin, On Mon, Aug 30, 2021 at 03:43:32PM +0200, Martin Kepplinger wrote: > Am Mittwoch, dem 25.08.2021 um 04:34 +0300 schrieb Laurent Pinchart: > > On Tue, Aug 24, 2021 at 04:57:15PM +0200, Martin Kepplinger wrote: > > > Am Sonntag, dem 08.08.2021 um 18:07 +0300 schrieb Laurent Pinchart: > > > > Hi Martin, > > > > > > > > Would you be able to test this series on the Librem5 ? I'd like to make > > > > sure it doesn't cause any regression. > > > > > > hi Laurent, could you send me the patches directly and tell me > > > where > > > they apply? > > > > Better, I can provide a branch :-) > > > > https://git.libcamera.org/libcamera/pinchartl/libcamera.git/log/?h=simple/multicam > > hi Laurent, > > using the above branch, the cam application streams the highest mode > (by default) just like the libcamera master branch. I don't see any > difference. > > Do you want to have anything else in this regard? Thank you for testing. This is all I need. Can I add a Tested-by: Martin Kepplinger <martin.kepplinger@puri.sm> to all the patches in the series ?
Am Montag, dem 30.08.2021 um 17:38 +0300 schrieb Laurent Pinchart: > Hi Martin, > > On Mon, Aug 30, 2021 at 03:43:32PM +0200, Martin Kepplinger wrote: > > Am Mittwoch, dem 25.08.2021 um 04:34 +0300 schrieb Laurent > > Pinchart: > > > On Tue, Aug 24, 2021 at 04:57:15PM +0200, Martin Kepplinger > > > wrote: > > > > Am Sonntag, dem 08.08.2021 um 18:07 +0300 schrieb Laurent > > > > Pinchart: > > > > > Hi Martin, > > > > > > > > > > Would you be able to test this series on the Librem5 ? I'd > > > > > like to make > > > > > sure it doesn't cause any regression. > > > > > > > > hi Laurent, could you send me the patches directly and tell me > > > > where > > > > they apply? > > > > > > Better, I can provide a branch :-) > > > > > > https://git.libcamera.org/libcamera/pinchartl/libcamera.git/log/?h=simple/multicam > > > > hi Laurent, > > > > using the above branch, the cam application streams the highest > > mode > > (by default) just like the libcamera master branch. I don't see any > > difference. > > > > Do you want to have anything else in this regard? > > Thank you for testing. This is all I need. Can I add a > > Tested-by: Martin Kepplinger <martin.kepplinger@puri.sm> > > to all the patches in the series ? > yes, you can. thanks, martin