[libcamera-devel,v5,0/6] Introduce UVC hotplugging support
mbox series

Message ID 20200616194523.23268-1-email@uajain.com
Headers show
Series
  • Introduce UVC hotplugging support
Related show

Message

Umang Jain June 16, 2020, 7:45 p.m. UTC
v4->v5:
  - Rework "libcamera: CameraManager: Drop the vector of created
    PipelineHandlers" commit to guard against the possibility of
    pipeline handler getting deleted in PipelineHandler::disconnect().
    Add a _lot_ of documentation to it via commit message and a comment.
  - Make sure no use-after-free errors are present when the unit-test
    runs under valgrind. Fold the patch given by Laurent in previous
    review to properly address and document it.
  - Provide more past context to "libcamera: CameraManager: Drop the
    vector of created PipelineHandlers" in commit message, to 'why'
    pipes_ vector is not needed anymore.
  - Few typos fixes.

v3->v4:
  - Introduce one additional commit to eliminate Pipehandlers' vector
    from CameraManager. It was leaving behind a leaked reference which
    was causing unclean unbind->bind operation (due to dangling dirs)
    for the hotplug unit test to run.
  - Simplify hotplug unit test with use of std::ofstream.
  - Change CameraManager::Private::addCamera() signature to accept
    Camera arg as pass-by-value instead of pass-by-reference.
  - qcam: Finalize HotplugEvent naming with ::HotPlug and ::HotUnplug
  - Few small improvements in commit messages and rebase. 

Umang Jain (6):
  libcamera: CameraManager: Drop the vector of created PipelineHandlers
  libcamera: camera_manager: Refactor pipelines creation into separate
    function
  libcamera: device_enumerator: Emit a signal when new devices are added
  libcamera: camera_manager: Introduce signals when a camera is added or
    removed
  qcam: main_window: Introduce initial hotplug support
  tests: Introduce hotplug hot-unplug unit test

 include/libcamera/camera_manager.h            |   6 +-
 .../libcamera/internal/device_enumerator.h    |   4 +
 src/libcamera/camera_manager.cpp              |  65 ++++++---
 src/libcamera/device_enumerator.cpp           |  13 ++
 src/libcamera/pipeline_handler.cpp            |  20 ++-
 src/qcam/main_window.cpp                      |  76 +++++++++++
 src/qcam/main_window.h                        |   6 +
 test/hotplug-cameras.cpp                      | 128 ++++++++++++++++++
 test/meson.build                              |   1 +
 9 files changed, 298 insertions(+), 21 deletions(-)
 create mode 100644 test/hotplug-cameras.cpp

Comments

Laurent Pinchart June 16, 2020, 9:51 p.m. UTC | #1
Hi Umang,

On Tue, Jun 16, 2020 at 07:45:32PM +0000, Umang Jain wrote:
> v4->v5:
>   - Rework "libcamera: CameraManager: Drop the vector of created
>     PipelineHandlers" commit to guard against the possibility of
>     pipeline handler getting deleted in PipelineHandler::disconnect().
>     Add a _lot_ of documentation to it via commit message and a comment.
>   - Make sure no use-after-free errors are present when the unit-test
>     runs under valgrind. Fold the patch given by Laurent in previous
>     review to properly address and document it.
>   - Provide more past context to "libcamera: CameraManager: Drop the
>     vector of created PipelineHandlers" in commit message, to 'why'
>     pipes_ vector is not needed anymore.
>   - Few typos fixes.
> 
> v3->v4:
>   - Introduce one additional commit to eliminate Pipehandlers' vector
>     from CameraManager. It was leaving behind a leaked reference which
>     was causing unclean unbind->bind operation (due to dangling dirs)
>     for the hotplug unit test to run.
>   - Simplify hotplug unit test with use of std::ofstream.
>   - Change CameraManager::Private::addCamera() signature to accept
>     Camera arg as pass-by-value instead of pass-by-reference.
>   - qcam: Finalize HotplugEvent naming with ::HotPlug and ::HotUnplug
>   - Few small improvements in commit messages and rebase. 
> 
> Umang Jain (6):
>   libcamera: CameraManager: Drop the vector of created PipelineHandlers
>   libcamera: camera_manager: Refactor pipelines creation into separate
>     function
>   libcamera: device_enumerator: Emit a signal when new devices are added
>   libcamera: camera_manager: Introduce signals when a camera is added or
>     removed
>   qcam: main_window: Introduce initial hotplug support
>   tests: Introduce hotplug hot-unplug unit test

All patches pushed to master. Thank you !

>  include/libcamera/camera_manager.h            |   6 +-
>  .../libcamera/internal/device_enumerator.h    |   4 +
>  src/libcamera/camera_manager.cpp              |  65 ++++++---
>  src/libcamera/device_enumerator.cpp           |  13 ++
>  src/libcamera/pipeline_handler.cpp            |  20 ++-
>  src/qcam/main_window.cpp                      |  76 +++++++++++
>  src/qcam/main_window.h                        |   6 +
>  test/hotplug-cameras.cpp                      | 128 ++++++++++++++++++
>  test/meson.build                              |   1 +
>  9 files changed, 298 insertions(+), 21 deletions(-)
>  create mode 100644 test/hotplug-cameras.cpp
Umang Jain June 17, 2020, 6:10 a.m. UTC | #2
Hi Laurent, Kieran,

On 6/17/20 3:21 AM, Laurent Pinchart wrote:
> Hi Umang,
>
> On Tue, Jun 16, 2020 at 07:45:32PM +0000, Umang Jain wrote:
>> v4->v5:
>>    - Rework "libcamera: CameraManager: Drop the vector of created
>>      PipelineHandlers" commit to guard against the possibility of
>>      pipeline handler getting deleted in PipelineHandler::disconnect().
>>      Add a _lot_ of documentation to it via commit message and a comment.
>>    - Make sure no use-after-free errors are present when the unit-test
>>      runs under valgrind. Fold the patch given by Laurent in previous
>>      review to properly address and document it.
>>    - Provide more past context to "libcamera: CameraManager: Drop the
>>      vector of created PipelineHandlers" in commit message, to 'why'
>>      pipes_ vector is not needed anymore.
>>    - Few typos fixes.
>>
>> v3->v4:
>>    - Introduce one additional commit to eliminate Pipehandlers' vector
>>      from CameraManager. It was leaving behind a leaked reference which
>>      was causing unclean unbind->bind operation (due to dangling dirs)
>>      for the hotplug unit test to run.
>>    - Simplify hotplug unit test with use of std::ofstream.
>>    - Change CameraManager::Private::addCamera() signature to accept
>>      Camera arg as pass-by-value instead of pass-by-reference.
>>    - qcam: Finalize HotplugEvent naming with ::HotPlug and ::HotUnplug
>>    - Few small improvements in commit messages and rebase.
>>
>> Umang Jain (6):
>>    libcamera: CameraManager: Drop the vector of created PipelineHandlers
>>    libcamera: camera_manager: Refactor pipelines creation into separate
>>      function
>>    libcamera: device_enumerator: Emit a signal when new devices are added
>>    libcamera: camera_manager: Introduce signals when a camera is added or
>>      removed
>>    qcam: main_window: Introduce initial hotplug support
>>    tests: Introduce hotplug hot-unplug unit test
> All patches pushed to master. Thank you !
Thank you for all the reviews and hand-holding through various hairy bits :)
>
>>   include/libcamera/camera_manager.h            |   6 +-
>>   .../libcamera/internal/device_enumerator.h    |   4 +
>>   src/libcamera/camera_manager.cpp              |  65 ++++++---
>>   src/libcamera/device_enumerator.cpp           |  13 ++
>>   src/libcamera/pipeline_handler.cpp            |  20 ++-
>>   src/qcam/main_window.cpp                      |  76 +++++++++++
>>   src/qcam/main_window.h                        |   6 +
>>   test/hotplug-cameras.cpp                      | 128 ++++++++++++++++++
>>   test/meson.build                              |   1 +
>>   9 files changed, 298 insertions(+), 21 deletions(-)
>>   create mode 100644 test/hotplug-cameras.cpp