1. 31 Aug, 2021 5 commits
    • Nícolas F. R. A. Prado's avatar
      Documentation: guides: pipeline-handler: Document internal queue pattern · 97ec8d19
      Nícolas F. R. A. Prado authored
      
      
      Pipeline handlers need to implement a common pattern of adding queued
      requests to an internal queue and queuing them to the video devices as
      the buffer slots there become available.
      
      Add this pattern to the vivid example in the pipeline-handler guide so
      it's clear that new pipeline handlers should also implement it.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      
      Cover-changes: 2
      - Added patch to document this pattern in the pipeline-handler guide
      
      Commit-changes: 2
      - New
      97ec8d19
    • Nícolas F. R. A. Prado's avatar
      libcamera: pipeline: simple: Add internal request queue · f9ef9665
      Nícolas F. R. A. Prado authored
      
      
      Add an internal queue that stores requests until there are internal
      buffers and V4L2 buffer slots available. This avoids the need to cancel
      requests when there is a shortage of said buffers.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      
      Cover-changes: 2
      - Added patch for the simple pipeline handler
      
      Commit-changes: 2
      - New
      
      Commit-notes:
      I wasn't able to test this patch as I don't have any of the target
      hardware.
      END
      f9ef9665
    • Nícolas F. R. A. Prado's avatar
      libcamera: pipeline: rkisp1: Add internal request queue · 72a87d1e
      Nícolas F. R. A. Prado authored
      
      
      Add an internal queue that stores requests until there are internal
      buffers and V4L2 buffer slots available. This avoids the need to cancel
      requests when there is a shortage of said buffers.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      Reviewed-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
      
      Commit-changes: 2
      - Moved cancellation of pending requests to after video devices stop
      72a87d1e
    • Nícolas F. R. A. Prado's avatar
      libcamera: pipeline: uvcvideo: Add internal request queue · c4df6452
      Nícolas F. R. A. Prado authored
      
      
      Add an internal queue that stores requests until there are V4L2 buffer
      slots available. This avoids the need to cancel requests when there is a
      shortage of said buffers.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      
      Commit-changes: 2
      - Added a counter to keep track of the number of available buffer slots
      - Moved cancellation of pending requests to after video devices stop
      - Added error log on failure to process controls
      c4df6452
    • Nícolas F. R. A. Prado's avatar
      libcamera: pipeline: vimc: Add internal request queue · bb70d41f
      Nícolas F. R. A. Prado authored
      
      
      Add an internal queue that stores requests until there are V4L2 buffer
      slots available. This avoids the need to cancel requests when there is a
      shortage of said buffers.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      
      Series-to: libcamera-devel@lists.libcamera.org
      Series-cc: kernel@collabora.com, André Almeida <andrealmeid@collabora.com>
      
      Series-version: 2
      
      Cover-changes: 2
      (thanks to Laurent)
      - Added a counter to keep track of the number of available buffer slots
        in the VIMC and UVC pipeline handlers
      
      Series-changes: 2
      - Moved processControls() from PipelineHandlerVimc to VimcCameraData
      - Moved cancellation of pending requests to after video devices stop
      
      Commit-changes: 2
      - Added a counter to keep track of the number of available buffer slots
      - Added error log on failure to process controls
      
      Cover-letter:
      libcamera: pipeline: Add internal request queue
      This series adds an internal request queue for the vimc, uvcvideo,
      rkisp1 and simple pipeline handlers. Each patch is independent of the
      others, but I've grouped them in this series because they're very
      similar, so it should ease the review.
      
      Additionally, patch 5 documents this pattern in the pipeline-handler
      guide so that future pipeline handlers also implement it.
      
      Note: starting in v2 this series actually depends on [1], due to the
      usage of buffer slots counters for the VIMC, UVC and simple pipeline
      handlers, which rely on constants defined in that series.
      
      Note: the simple pipeline handler patch wasn't tested since I don't have
      any of the hardware targeted by it.
      
      The patches here are based on these patches that added the same functionality
      for the ipu3 pipeline handler: 5a9d1921 ("libcamera: pipeline: ipu3: Try
      queuing pending requests if a buffer is available") and 89dae584
      ("libcamera: pipeline: ipu3: Store requests in the case a buffer shortage").
      
      With these patches applied, the lc-compliance test from [1] passes in
      all pipeline handlers, even if the number of buffer slots in them is
      lowered below 8.
      
      [1] https://lists.libcamera.org/pipermail/libcamera-devel/2021-August/023773.html
      
      Previous standalone versions of the patches: uvcvideo [2] and rkisp1 [3]
      
      [2] https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022029.html
      [3] https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022135.html
      END
      bb70d41f
  2. 24 Aug, 2021 17 commits
    • Nícolas F. R. A. Prado's avatar
      lc-compliance: Add test to ensure MinimumRequests is valid · 195f03d5
      Nícolas F. R. A. Prado authored
      
      
      Add a test in lc-compliance to check that the MinimumRequests property
      is set and valid, that is, greater than 0.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      
      Series-changes: 8
      - Moved RequiredProperties test to property_test.cpp
      - Moved CameraTests to new test_base.{cpp,h} files
      
      Cover-changes: 7
      - Added patch 11 to test if MinimumRequests is valid
      
      Commit-changes: 7
      - New
      195f03d5
    • Nícolas F. R. A. Prado's avatar
      lc-compliance: Check that requests complete successfully · 7ac314aa
      Nícolas F. R. A. Prado authored
      
      
      When a request fails to queue it is completed but with its status set to
      RequestCancelled. Add a check in the requestComplete callback to make
      sure that the request was completed successfully.
      
      For the SimpleCaptureUnbalanced test we need to do this check only if
      the capture isn't over yet, otherwise the few extra requests that get
      cancelled at the end, which is the normal behavior, will make the test
      fail.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      
      Series-changes: 8
      - Fixed issue in UnbalancedStop test where requests cancelled due to stop() call
        were failing the test
      
      Commit-changes: 8
      - Fixed formatting
      
      Commit-changes: 5
      - New
      7ac314aa
    • Nícolas F. R. A. Prado's avatar
      lc-compliance: Add test to queue more requests than hardware depth · c8ed4c74
      Nícolas F. R. A. Prado authored
      A pipeline handler should be able to handle an arbitrary number of
      simultaneous requests by submitting what it can to the video device and
      queuing the rest internally until resources are available. This isn't
      currently done by some pipeline handlers however [1].
      
      Add a new test to lc-compliance that submits a lot of requests at once
      to check if the pipeline handler is behaving well in this situation.
      
      [1] https://bugs.libcamera.org/show_bug.cgi?id=24
      
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      
      Commit-changes: 6
      - Made MAX_SIMULTANEOUS_REQUESTS a constexpr
      
      Commit-changes: 5
      - Updated to use googletest, and CameraHolder and roleToString from previous
        patches
      c8ed4c74
    • Nícolas F. R. A. Prado's avatar
      lc-compliance: Move role to string conversion to its own function · c81a4174
      Nícolas F. R. A. Prado authored
      
      
      The functions that generate the test name based on the parameters need
      to convert a StreamRole to a string. Move this to a separate function to
      avoid redundancy.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      Reviewed-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
      
      Commit-changes: 8
      - Made roleToString()'s map const and changed usage from [] operator to at()
      
      Commit-changes: 5
      - New
      c81a4174
    • Nícolas F. R. A. Prado's avatar
      lc-compliance: Move camera setup to CameraHolder class · 6fe92dbc
      Nícolas F. R. A. Prado authored
      
      
      Different base classes can be used for different setups on tests, but
      all of them will need to setup the camera for the test. To reuse that
      code, move it to a separate CameraHolder class that is inherited by test
      classes.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      Reviewed-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
      
      Series-changes: 8
      - Moved CameraHolder to new test_base.{cpp,h} files
      
      Commit-changes: 5
      - New
      6fe92dbc
    • Nícolas F. R. A. Prado's avatar
      lc-compliance: Factor common capture code into SimpleCapture · 61dc59e4
      Nícolas F. R. A. Prado authored
      
      
      Factor common code from SimpleCapture* subclasses into the SimpleCapture
      parent class in order to avoid duplicated code.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      
      Series-changes: 8
      - Added requests_ member to SimpleCapture to hold ownership of queued
        requests during capture
      
      Commit-changes: 8
      - Made queueRequest() virtual and removed SimpleCaptureBalanced::queueRequests()
      
      Series-changes: 6
      - Switched queueRequests()'s 'buffers' and 'requests' parameters order, since
        'requests' is an output variable
      - Added comment to runCaptureSession()
      
      Commit-changes: 6
      - Added missing blank line before return in captureCompleted()
      - Added blank line to runCaptureSession()
      61dc59e4
    • Nícolas F. R. A. Prado's avatar
      lc-compliance: Move buffer allocation to separate function · 880f60a2
      Nícolas F. R. A. Prado authored
      
      
      Move buffer allocation to its own function and with an optional count
      argument so tests can specify how many buffers to allocate. When count
      is omitted, allocate MinimumRequests buffers.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      
      Commit-changes: 8
      - Made SimpleCapture::allocateBuffers() a single function with an optional parameter
      
      Commit-changes: 5
      - New
      880f60a2
    • Nícolas F. R. A. Prado's avatar
      lc-compliance: Fix source file ordering in meson.build · c3b74e04
      Nícolas F. R. A. Prado authored
      
      
      The capture_test.cpp file was added in the source list of meson in the
      wrong place. Fix it so the list is alphabetically sorted.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      
      Cover-changes: 8
      - Added patch 10 to fix wrong file order in lc-compliance's meson.build
      
      Commit-changes: 8
      - New
      c3b74e04
    • Nícolas F. R. A. Prado's avatar
      libcamera: stream: Remove bufferCount · 9bebbda9
      Nícolas F. R. A. Prado authored
      
      
      Now that the number of buffers allocated by the FrameBufferAllocator
      helper is passed through FrameBufferAllocator::allocate() and the
      pipelines no longer use bufferCount for internal buffer or V4L2 buffer
      slots allocation, we no longer need to have bufferCount in the
      StreamConfiguration, so remove it.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      
      Commit-changes: 8
      - Updated the pipeline-handler guide to use MinimumRequests instead of
        bufferCount
      - Removed kNumInternalBuffers as it was unused
      
      Commit-changes: 6
      - Removed IPU3_BUFFER_COUNT as it was unused
      9bebbda9
    • Nícolas F. R. A. Prado's avatar
      v4l2: Allocate buffers based on requested count and MinimumRequests · f6439f50
      Nícolas F. R. A. Prado authored
      
      
      Currently the number of buffers allocated is based on bufferCount, which
      is hardcoded to 1. Instead allocate buffers based on the requested count
      and on the MinimumRequests property for the camera, which is accessed
      through a newly added getter.
      
      This allows the removal of bufferCount.
      
      While at it, fix a typo: s/interval/internal/.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      
      Cover-changes: 8
      - Moved V4L2 compatibility layer changes to separate commit
      
      Commit-changes: 8
      - New
      - Fixed typo: s/interval/internal/
      f6439f50
    • Nícolas F. R. A. Prado's avatar
      libcamera: pipeline: vimc, uvcvideo: Don't rely on bufferCount · 5daa3412
      Nícolas F. R. A. Prado authored
      
      
      Instead of using bufferCount as the number of V4L2 buffer slots to
      reserve in the vimc and uvcvideo pipeline handlers, use a reasonably
      high constant: 16. Overallocating isn't a problem as buffer slots are
      cheap. Having too few, on the other hand, could degrade performance. It
      is expected that this number will be more than enough for most, if not
      all, use cases.
      
      This makes way for removing bufferCount.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      
      Commit-changes: 8
      - New
      5daa3412
    • Nícolas F. R. A. Prado's avatar
      libcamera: pipeline: rkisp1: Don't rely on bufferCount · 13620417
      Nícolas F. R. A. Prado authored
      
      
      Currently the rkisp1 pipeline handler relies on bufferCount to decide on
      the number of parameter and statistics buffers to allocate internally
      and for the number of V4L2 buffer slots to reserve. Instead, the number
      of internal buffers should be the minimum required by the pipeline to
      keep the requests flowing, in order to avoid wasting memory, while the
      number of V4L2 buffer slots should be greater than the expected number
      of requests queued by the application, in order to avoid thrashing
      dmabuf mappings, which would degrade performance.
      
      Stop relying on bufferCount for these numbers and instead set them to
      appropriate, and independent, constants.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      
      Commit-changes: 8
      - New
      13620417
    • Nícolas F. R. A. Prado's avatar
      libcamera: pipeline: simple: Don't rely on bufferCount · 086d9de3
      Nícolas F. R. A. Prado authored
      
      
      Currently the simple pipeline handler relies on bufferCount to decide on
      the number of buffers to allocate internally when a converter is in use
      and for the number of V4L2 buffer slots to reserve. Instead, the number
      of internal buffers should be the minimum required by the pipeline to
      keep the requests flowing, in order to avoid wasting memory, while the
      number of V4L2 buffer slots should be greater than the expected number
      of requests queued by the application, in order to avoid thrashing
      dmabuf mappings, which would degrade performance.
      
      Stop relying on bufferCount for these numbers and instead set them to
      appropriate, and independent, constants.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      
      Commit-changes: 8
      - New
      086d9de3
    • Nícolas F. R. A. Prado's avatar
      libcamera: pipeline: ipu3: Don't rely on bufferCount · 558bcfb4
      Nícolas F. R. A. Prado authored
      
      
      Currently the ipu3 pipeline handler relies on bufferCount to decide on
      the number of V4L2 buffer slots to reserve as well as for the number of
      buffers to allocate internally for the CIO2 raw buffers and
      parameter-statistics ImgU buffer pairs. Instead, the number of internal
      buffers should be the minimum required by the pipeline to keep the
      requests flowing without frame drops, in order to avoid wasting memory,
      while the number of V4L2 buffer slots should be greater than the
      expected number of requests queued by the application, in order to avoid
      thrashing dmabuf mappings, which would degrade performance.
      
      Stop relying on bufferCount for these numbers and instead set them to
      appropriate, and independent, constants.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      
      Commit-changes: 8
      - New
      558bcfb4
    • Nícolas F. R. A. Prado's avatar
      libcamera: pipeline: raspberrypi: Don't rely on bufferCount · 74d943ed
      Nícolas F. R. A. Prado authored
      
      
      Currently the raspberrypi pipeline handler relies on bufferCount to
      decide on the number of buffers to allocate internally and for the
      number of V4L2 buffer slots to reserve. Instead, the number of internal
      buffers should be the minimum required by the pipeline to keep the
      requests flowing, in order to avoid wasting memory, while the number of
      V4L2 buffer slots should be greater than the expected number of requests
      queued, in order to avoid thrashing dmabuf mappings, which would degrade
      performance.
      
      Extend the Stream class to receive the number of internal buffers that
      should be allocated, and optionally the number of buffer slots to
      reserve. Use these new member variables to specify better suited numbers
      for internal buffers and buffer slots for each stream individually,
      which also allows us to remove bufferCount.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      
      Commit-changes: 8
      - New
      
      Series-changes: 8
      - Reworked buffer allocation handling in the raspberrypi pipeline handler
      74d943ed
    • Nícolas F. R. A. Prado's avatar
      libcamera: framebuffer_allocator: Make allocate() require count · e238ffea
      Nícolas F. R. A. Prado authored
      
      
      Make FrameBufferAllocator::allocate() require a 'count' argument for the
      number of buffers to be allocated.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      Reviewed-by: default avatarKieran Bingham <kieran.bingham@ideasonboard.com>
      Reviewed-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
      
      Series-changes: 8
      - 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
      
      Commit-changes: 8
      - Improved FrameBufferAllocator::allocate() description
      
      Commit-changes: 6
      - Changed static_cast to convert 'count' to unsigned int instead of
        'bufferCount' to int when comparing
      
      Series-changes: 5
      - Made sure that qcam allocates at least 2 buffers
      e238ffea
    • Nícolas F. R. A. Prado's avatar
      libcamera: property: Add MinimumRequests property · 6bfd3c5f
      Nícolas F. R. A. Prado authored
      
      
      The MinimumRequests property reports the minimum number of capture
      requests that the camera pipeline requires to have queued in order to
      sustain capture operations without frame drops. At this quantity,
      there's no guarantee that manual per-frame controls will apply
      correctly. The quantity needed for that may be added as a separate
      property in the future.
      Signed-off-by: Nícolas F. R. A. Prado's avatarNícolas F. R. A. Prado <nfraprado@collabora.com>
      Reviewed-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
      
      Series-to: libcamera-devel@lists.libcamera.org
      Series-cc: kernel@collabora.com, André Almeida <andrealmeid@collabora.com>
      
      Series-version: 8
      
      Cover-changes: 8
      (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)
      
      Series-changes: 8
      - 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
      
      Commit-changes: 8
      - Updated pipeline-handler guides with new MinimumRequests property
      
      Cover-changes: 7
      (thanks to Kieran and Jacopo)
      
      Series-changes: 7
      - Renamed property from MinNumRequests to MinimumRequests
      - Changed MinimumRequests property's description
      
      Cover-changes: 6
      (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
      
      Series-changes: 6
      - Removed comment from Raspberrypi MinNumRequests setting
      
      Commit-changes: 6
      - Removed an extra blank line
      
      Cover-changes: 5
      - 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
      
      Cover-changes: 4
      (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
      
      Cover-changes: 3
      - 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
      
      Cover-changes: 2
      - 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
      
      Cover-letter:
      lc-compliance: Add test to queue more requests than hardware depth
      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 1 through 9 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 1 adds the new MinimumRequests property to report the minimum number of
      requests needed by the pipeline handler.
      
      Patch 2 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 3-7 decouple the number of internal buffers and V4L2 buffer slots from
      bufferCount in each pipeline handler.
      
      Patch 8 reworks the V4L2 compatibility layer to not depend on bufferCount.
      
      Patch 9 removes bufferCount from the StreamConfiguration and everywhere it was
      still used, as it is no longer needed.
      
      Patch 10 fixes a file ordering issue in lc-compliance's meson.build.
      
      Patches 11-14 does some refactoring in lc-compliance in order to reduce code
      duplication.
      
      Patch 15 adds the test to lc-compliance.
      
      Patch 16 adds checks in lc-compliance to ensure that requests which failed to be
      enqueued are reported as test failure.
      
      Patch 17 adds another, very short, test to lc-compliance to make sure that the
      MinimumRequests property is set in the pipeline handler.
      
      I've run 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
      END
      6bfd3c5f
  3. 23 Aug, 2021 5 commits
  4. 21 Aug, 2021 2 commits
  5. 20 Aug, 2021 9 commits
  6. 19 Aug, 2021 2 commits
    • Umang Jain's avatar
      libcamera: ipc_pipe: Do not run memcpy with null arguments · 35583345
      Umang Jain authored
      
      
      IPCMessage::payload() converts the IPCMessage into an IPCUnixSocket
      payload. However, if IPCMessage is constructed with one of the
      following constructors -
      
      	IPCMessage::IPCMessage(),
      	IPCMessage::IPCMessage(uint32_t cmd)
      	IPCMessage::IPCMessage(const Header &header)
      
      The data_ vector of IPCMessage is empty and uninitialised. In that
      case, IPCMessage::payload will try to memcpy() an empty data_ vector
      which can lead to invoking memcpy() with a nullptr parameter, which
      is then identified by the address sanity checker.. Add a non-empty
      data_ vector check to avoid it.
      
      The issue is noticed by running a test manually, testing the vimc
      IPA code paths in isolated mode. It is only noticed when the test
      is compiled with -Db_sanitize=address,undefined meson built-in option.
      
      ipc_pipe.cpp:110:8: runtime error: null pointer passed as argument 2, which is declared to never be null
      Signed-off-by: default avatarUmang Jain <umang.jain@ideasonboard.com>
      Reviewed-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
      Reviewed-by: default avatarPaul Elder <paul.elder@ideasonboard.com>
      Reviewed-by: default avatarKieran Bingham <kieran.bingham@ideasonboard.com>
      35583345
    • Umang Jain's avatar
      libcamera: ipc_unixsocket: Do not run memcpy with null arguments · cdb70b5c
      Umang Jain authored
      
      
      In IPCUnixSocket, a payload can be sent/received with empty fd vector,
      which leads to passing a nullptr in memcpy() in both sendData()
      and recvData(). Add a null check for fd vector's data pointer
      to avoid invoking memcpy() with nullptr.
      
      The issue is noticed by running a test manually testing the vimc
      IPA code paths in isolated mode. It is only noticed when the test
      is compiled with -Db_sanitize=address,undefined meson built-in option.
      
      ipc_unixsocket.cpp:268:8: runtime error: null pointer passed as argument 2, which is declared to never be null
      ipc_unixsocket.cpp:312:8: runtime error: null pointer passed as argument 1, which is declared to never be null
      Signed-off-by: default avatarUmang Jain <umang.jain@ideasonboard.com>
      Reviewed-by: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
      Reviewed-by: default avatarPaul Elder <paul.elder@ideasonboard.com>
      cdb70b5c