| Message ID | 20200112010212.2609025-1-niklas.soderlund@ragnatech.se |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, On Sun, Jan 12, 2020 at 02:01:40AM +0100, Niklas Söderlund wrote: > Hi, > > This series reworks the buffer API across the whole library. The two > main reasons for the rework is > > - The current buffer API is cumbersome to work with as the variations > between internally allocated buffers (from a V4L2 video device) and > externally (from other source in the system or other V4l2 video > device) is slightly different. > > - V4L2 concepts such as buffer index have "leaked" into the application > facing interface which makes the API less intuitive to work with as > one needs to know more about V4L2 and its limitations to use. > > As changing the buffer API touches most parts of the library this series > is unfortunately quite large and some patches are also quite large. I > have really tried to break things apart as best I could. > > The series starts by adding the new FrameBuffer interface building > blocks and then slowly proceeds to replace the existing API with the new > one. The series is tested on all upstream pipelines and IPAs without any > regressions. > > One note on 18/32, there is a temporary hack which is introduced in this > patch that allows both the Buffer and FrameBuffer API to function side > by side. That is i the V4L2VideoDevice is setup using the Buffer API > slots related to Buffer will be emitted while if it is setup with the > FrameBuffer API slots related to FrameBuffers are emitted. This hack is > removed later in 27/32 where the Buffer API is completely removed. > > I have incorporated all prototype patches published by Laurent on top of > this series. Either in their original form or squashed if the code is > introduced in this patch-set (FileDescriptor). Big thanks to Laurent for > this work. Fully reviewed \o/ There are just a handful of minor comments, I think you can address them and push the result to master. Please remember to remove the changelog from individual patches before doing so. Thank you for the great work ! There's more needed as I think we can improve the interface towards pipeline handlers, but that can be done on top. > Laurent Pinchart (1): > v4l2: camera_proxy: Call V4L2Camera::getBufferFd() directly > > Niklas Söderlund (31): > libcamera: request: remove prepare() > libcamera: Add FileDescriptor to help pass numerical fds around > test: file_descriptor: Add test > v4l2: Rename FrameMetadata to V4L2FrameMetadata > v4l2: camera: Handle memory mapping of buffers directly > libcamera: buffer: Add FrameMetadata container for metadata > information > libcamera: buffer: Add FrameBuffer interface > ipa: Switch to FrameBuffer interface > libcamera: buffer: Switch from Plane to FrameBuffer::Plane > libcamera: buffers: Remove Plane class > libcamera: buffer: Drop private function setRequest() > libcamera: v4l2_videodevice: Align which type variable is used in > queueBuffer() > libcamera: v4l2_videodevice: Extract exportDmabufFd() > libcamera: request: In addBuffer() do not fetch stream from Buffer > libcamera: buffer: Move captured metadata to FrameMetadata > libcamera: v4l2_videodevice: Add V4L2BufferCache to deal with index > mapping > libcamera: v4l2_videodevice: Add FrameBuffer interface > test: v4l2_videodevice: Switch to FrameBuffer interface > test: camera: buffer_import: Update to FrameBuffer restrictions > libcamera: pipeline: rkisp1: Destroy frame information before > completing request > libcamera: pipeline: rkisp1: Switch to FrameBuffer interface for stat > and param > libcamera: pipeline: ipu3: Switch to FrameBuffer interface for cio2 > and stat > libcamera: pipeline: Add FrameBuffer handlers > libcamera: allocator: Add FrameBufferAllocator to help applications > allocate buffers > libcamera: Switch to FrameBuffer interface > libcamera: v4l2_videodevice: Remove Buffer interface > libcamera: Remove dead code after switch to FrameBuffer > cam: Cache buffer memory mapping > qcam: Cache buffer memory mapping > libcamera: pipeline: Remove explicit buffer handling > libcamera: camera: Remove the prepared state > > include/ipa/ipa_interface.h | 2 +- > include/libcamera/buffer.h | 113 ++--- > include/libcamera/camera.h | 13 +- > include/libcamera/file_descriptor.h | 47 ++ > include/libcamera/framebuffer_allocator.h | 45 ++ > include/libcamera/meson.build | 2 + > include/libcamera/request.h | 16 +- > include/libcamera/stream.h | 23 - > src/android/camera_device.cpp | 43 +- > src/cam/buffer_writer.cpp | 33 +- > src/cam/buffer_writer.h | 8 +- > src/cam/capture.cpp | 64 +-- > src/cam/capture.h | 5 +- > src/ipa/libipa/ipa_interface_wrapper.cpp | 9 +- > src/ipa/rkisp1/rkisp1.cpp | 40 +- > src/libcamera/buffer.cpp | 411 +++++------------ > src/libcamera/camera.cpp | 184 +++----- > src/libcamera/file_descriptor.cpp | 203 +++++++++ > src/libcamera/framebuffer_allocator.cpp | 215 +++++++++ > src/libcamera/include/pipeline_handler.h | 13 +- > src/libcamera/include/v4l2_videodevice.h | 66 ++- > src/libcamera/ipa_context_wrapper.cpp | 8 +- > src/libcamera/ipa_interface.cpp | 7 +- > src/libcamera/meson.build | 2 + > src/libcamera/pipeline/ipu3/ipu3.cpp | 277 +++++------- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 192 ++++---- > src/libcamera/pipeline/uvcvideo.cpp | 40 +- > src/libcamera/pipeline/vimc.cpp | 40 +- > src/libcamera/pipeline_handler.cpp | 75 ++- > src/libcamera/request.cpp | 60 +-- > src/libcamera/stream.cpp | 248 +--------- > src/libcamera/v4l2_videodevice.cpp | 426 ++++++++++++------ > src/qcam/main_window.cpp | 86 ++-- > src/qcam/main_window.h | 7 +- > src/v4l2/v4l2_camera.cpp | 63 +-- > src/v4l2/v4l2_camera.h | 44 +- > src/v4l2/v4l2_camera_proxy.cpp | 46 +- > src/v4l2/v4l2_camera_proxy.h | 2 +- > src/v4l2/v4l2_compat_manager.cpp | 2 +- > test/camera/buffer_import.cpp | 382 ++++------------ > test/camera/capture.cpp | 48 +- > test/camera/statemachine.cpp | 89 +--- > test/file-descriptor.cpp | 208 +++++++++ > test/ipa/ipa_wrappers_test.cpp | 41 +- > test/meson.build | 1 + > test/v4l2_videodevice/buffer_sharing.cpp | 34 +- > test/v4l2_videodevice/capture_async.cpp | 18 +- > test/v4l2_videodevice/request_buffers.cpp | 11 +- > test/v4l2_videodevice/stream_on_off.cpp | 6 +- > test/v4l2_videodevice/v4l2_m2mdevice.cpp | 40 +- > test/v4l2_videodevice/v4l2_videodevice_test.h | 2 +- > 51 files changed, 2101 insertions(+), 1959 deletions(-) > create mode 100644 include/libcamera/file_descriptor.h > create mode 100644 include/libcamera/framebuffer_allocator.h > create mode 100644 src/libcamera/file_descriptor.cpp > create mode 100644 src/libcamera/framebuffer_allocator.cpp > create mode 100644 test/file-descriptor.cpp
Hi, This series is now pushed to master with Laurent's comments addressed. On 2020-01-12 02:01:40 +0100, Niklas Söderlund wrote: > Hi, > > This series reworks the buffer API across the whole library. The two > main reasons for the rework is > > - The current buffer API is cumbersome to work with as the variations > between internally allocated buffers (from a V4L2 video device) and > externally (from other source in the system or other V4l2 video > device) is slightly different. > > - V4L2 concepts such as buffer index have "leaked" into the application > facing interface which makes the API less intuitive to work with as > one needs to know more about V4L2 and its limitations to use. > > As changing the buffer API touches most parts of the library this series > is unfortunately quite large and some patches are also quite large. I > have really tried to break things apart as best I could. > > The series starts by adding the new FrameBuffer interface building > blocks and then slowly proceeds to replace the existing API with the new > one. The series is tested on all upstream pipelines and IPAs without any > regressions. > > One note on 18/32, there is a temporary hack which is introduced in this > patch that allows both the Buffer and FrameBuffer API to function side > by side. That is i the V4L2VideoDevice is setup using the Buffer API > slots related to Buffer will be emitted while if it is setup with the > FrameBuffer API slots related to FrameBuffers are emitted. This hack is > removed later in 27/32 where the Buffer API is completely removed. > > I have incorporated all prototype patches published by Laurent on top of > this series. Either in their original form or squashed if the code is > introduced in this patch-set (FileDescriptor). Big thanks to Laurent for > this work. > > Laurent Pinchart (1): > v4l2: camera_proxy: Call V4L2Camera::getBufferFd() directly > > Niklas Söderlund (31): > libcamera: request: remove prepare() > libcamera: Add FileDescriptor to help pass numerical fds around > test: file_descriptor: Add test > v4l2: Rename FrameMetadata to V4L2FrameMetadata > v4l2: camera: Handle memory mapping of buffers directly > libcamera: buffer: Add FrameMetadata container for metadata > information > libcamera: buffer: Add FrameBuffer interface > ipa: Switch to FrameBuffer interface > libcamera: buffer: Switch from Plane to FrameBuffer::Plane > libcamera: buffers: Remove Plane class > libcamera: buffer: Drop private function setRequest() > libcamera: v4l2_videodevice: Align which type variable is used in > queueBuffer() > libcamera: v4l2_videodevice: Extract exportDmabufFd() > libcamera: request: In addBuffer() do not fetch stream from Buffer > libcamera: buffer: Move captured metadata to FrameMetadata > libcamera: v4l2_videodevice: Add V4L2BufferCache to deal with index > mapping > libcamera: v4l2_videodevice: Add FrameBuffer interface > test: v4l2_videodevice: Switch to FrameBuffer interface > test: camera: buffer_import: Update to FrameBuffer restrictions > libcamera: pipeline: rkisp1: Destroy frame information before > completing request > libcamera: pipeline: rkisp1: Switch to FrameBuffer interface for stat > and param > libcamera: pipeline: ipu3: Switch to FrameBuffer interface for cio2 > and stat > libcamera: pipeline: Add FrameBuffer handlers > libcamera: allocator: Add FrameBufferAllocator to help applications > allocate buffers > libcamera: Switch to FrameBuffer interface > libcamera: v4l2_videodevice: Remove Buffer interface > libcamera: Remove dead code after switch to FrameBuffer > cam: Cache buffer memory mapping > qcam: Cache buffer memory mapping > libcamera: pipeline: Remove explicit buffer handling > libcamera: camera: Remove the prepared state > > include/ipa/ipa_interface.h | 2 +- > include/libcamera/buffer.h | 113 ++--- > include/libcamera/camera.h | 13 +- > include/libcamera/file_descriptor.h | 47 ++ > include/libcamera/framebuffer_allocator.h | 45 ++ > include/libcamera/meson.build | 2 + > include/libcamera/request.h | 16 +- > include/libcamera/stream.h | 23 - > src/android/camera_device.cpp | 43 +- > src/cam/buffer_writer.cpp | 33 +- > src/cam/buffer_writer.h | 8 +- > src/cam/capture.cpp | 64 +-- > src/cam/capture.h | 5 +- > src/ipa/libipa/ipa_interface_wrapper.cpp | 9 +- > src/ipa/rkisp1/rkisp1.cpp | 40 +- > src/libcamera/buffer.cpp | 411 +++++------------ > src/libcamera/camera.cpp | 184 +++----- > src/libcamera/file_descriptor.cpp | 203 +++++++++ > src/libcamera/framebuffer_allocator.cpp | 215 +++++++++ > src/libcamera/include/pipeline_handler.h | 13 +- > src/libcamera/include/v4l2_videodevice.h | 66 ++- > src/libcamera/ipa_context_wrapper.cpp | 8 +- > src/libcamera/ipa_interface.cpp | 7 +- > src/libcamera/meson.build | 2 + > src/libcamera/pipeline/ipu3/ipu3.cpp | 277 +++++------- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 192 ++++---- > src/libcamera/pipeline/uvcvideo.cpp | 40 +- > src/libcamera/pipeline/vimc.cpp | 40 +- > src/libcamera/pipeline_handler.cpp | 75 ++- > src/libcamera/request.cpp | 60 +-- > src/libcamera/stream.cpp | 248 +--------- > src/libcamera/v4l2_videodevice.cpp | 426 ++++++++++++------ > src/qcam/main_window.cpp | 86 ++-- > src/qcam/main_window.h | 7 +- > src/v4l2/v4l2_camera.cpp | 63 +-- > src/v4l2/v4l2_camera.h | 44 +- > src/v4l2/v4l2_camera_proxy.cpp | 46 +- > src/v4l2/v4l2_camera_proxy.h | 2 +- > src/v4l2/v4l2_compat_manager.cpp | 2 +- > test/camera/buffer_import.cpp | 382 ++++------------ > test/camera/capture.cpp | 48 +- > test/camera/statemachine.cpp | 89 +--- > test/file-descriptor.cpp | 208 +++++++++ > test/ipa/ipa_wrappers_test.cpp | 41 +- > test/meson.build | 1 + > test/v4l2_videodevice/buffer_sharing.cpp | 34 +- > test/v4l2_videodevice/capture_async.cpp | 18 +- > test/v4l2_videodevice/request_buffers.cpp | 11 +- > test/v4l2_videodevice/stream_on_off.cpp | 6 +- > test/v4l2_videodevice/v4l2_m2mdevice.cpp | 40 +- > test/v4l2_videodevice/v4l2_videodevice_test.h | 2 +- > 51 files changed, 2101 insertions(+), 1959 deletions(-) > create mode 100644 include/libcamera/file_descriptor.h > create mode 100644 include/libcamera/framebuffer_allocator.h > create mode 100644 src/libcamera/file_descriptor.cpp > create mode 100644 src/libcamera/framebuffer_allocator.cpp > create mode 100644 test/file-descriptor.cpp > > -- > 2.24.1 >
Hi, This series reworks the buffer API across the whole library. The two main reasons for the rework is - The current buffer API is cumbersome to work with as the variations between internally allocated buffers (from a V4L2 video device) and externally (from other source in the system or other V4l2 video device) is slightly different. - V4L2 concepts such as buffer index have "leaked" into the application facing interface which makes the API less intuitive to work with as one needs to know more about V4L2 and its limitations to use. As changing the buffer API touches most parts of the library this series is unfortunately quite large and some patches are also quite large. I have really tried to break things apart as best I could. The series starts by adding the new FrameBuffer interface building blocks and then slowly proceeds to replace the existing API with the new one. The series is tested on all upstream pipelines and IPAs without any regressions. One note on 18/32, there is a temporary hack which is introduced in this patch that allows both the Buffer and FrameBuffer API to function side by side. That is i the V4L2VideoDevice is setup using the Buffer API slots related to Buffer will be emitted while if it is setup with the FrameBuffer API slots related to FrameBuffers are emitted. This hack is removed later in 27/32 where the Buffer API is completely removed. I have incorporated all prototype patches published by Laurent on top of this series. Either in their original form or squashed if the code is introduced in this patch-set (FileDescriptor). Big thanks to Laurent for this work. Laurent Pinchart (1): v4l2: camera_proxy: Call V4L2Camera::getBufferFd() directly Niklas Söderlund (31): libcamera: request: remove prepare() libcamera: Add FileDescriptor to help pass numerical fds around test: file_descriptor: Add test v4l2: Rename FrameMetadata to V4L2FrameMetadata v4l2: camera: Handle memory mapping of buffers directly libcamera: buffer: Add FrameMetadata container for metadata information libcamera: buffer: Add FrameBuffer interface ipa: Switch to FrameBuffer interface libcamera: buffer: Switch from Plane to FrameBuffer::Plane libcamera: buffers: Remove Plane class libcamera: buffer: Drop private function setRequest() libcamera: v4l2_videodevice: Align which type variable is used in queueBuffer() libcamera: v4l2_videodevice: Extract exportDmabufFd() libcamera: request: In addBuffer() do not fetch stream from Buffer libcamera: buffer: Move captured metadata to FrameMetadata libcamera: v4l2_videodevice: Add V4L2BufferCache to deal with index mapping libcamera: v4l2_videodevice: Add FrameBuffer interface test: v4l2_videodevice: Switch to FrameBuffer interface test: camera: buffer_import: Update to FrameBuffer restrictions libcamera: pipeline: rkisp1: Destroy frame information before completing request libcamera: pipeline: rkisp1: Switch to FrameBuffer interface for stat and param libcamera: pipeline: ipu3: Switch to FrameBuffer interface for cio2 and stat libcamera: pipeline: Add FrameBuffer handlers libcamera: allocator: Add FrameBufferAllocator to help applications allocate buffers libcamera: Switch to FrameBuffer interface libcamera: v4l2_videodevice: Remove Buffer interface libcamera: Remove dead code after switch to FrameBuffer cam: Cache buffer memory mapping qcam: Cache buffer memory mapping libcamera: pipeline: Remove explicit buffer handling libcamera: camera: Remove the prepared state include/ipa/ipa_interface.h | 2 +- include/libcamera/buffer.h | 113 ++--- include/libcamera/camera.h | 13 +- include/libcamera/file_descriptor.h | 47 ++ include/libcamera/framebuffer_allocator.h | 45 ++ include/libcamera/meson.build | 2 + include/libcamera/request.h | 16 +- include/libcamera/stream.h | 23 - src/android/camera_device.cpp | 43 +- src/cam/buffer_writer.cpp | 33 +- src/cam/buffer_writer.h | 8 +- src/cam/capture.cpp | 64 +-- src/cam/capture.h | 5 +- src/ipa/libipa/ipa_interface_wrapper.cpp | 9 +- src/ipa/rkisp1/rkisp1.cpp | 40 +- src/libcamera/buffer.cpp | 411 +++++------------ src/libcamera/camera.cpp | 184 +++----- src/libcamera/file_descriptor.cpp | 203 +++++++++ src/libcamera/framebuffer_allocator.cpp | 215 +++++++++ src/libcamera/include/pipeline_handler.h | 13 +- src/libcamera/include/v4l2_videodevice.h | 66 ++- src/libcamera/ipa_context_wrapper.cpp | 8 +- src/libcamera/ipa_interface.cpp | 7 +- src/libcamera/meson.build | 2 + src/libcamera/pipeline/ipu3/ipu3.cpp | 277 +++++------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 192 ++++---- src/libcamera/pipeline/uvcvideo.cpp | 40 +- src/libcamera/pipeline/vimc.cpp | 40 +- src/libcamera/pipeline_handler.cpp | 75 ++- src/libcamera/request.cpp | 60 +-- src/libcamera/stream.cpp | 248 +--------- src/libcamera/v4l2_videodevice.cpp | 426 ++++++++++++------ src/qcam/main_window.cpp | 86 ++-- src/qcam/main_window.h | 7 +- src/v4l2/v4l2_camera.cpp | 63 +-- src/v4l2/v4l2_camera.h | 44 +- src/v4l2/v4l2_camera_proxy.cpp | 46 +- src/v4l2/v4l2_camera_proxy.h | 2 +- src/v4l2/v4l2_compat_manager.cpp | 2 +- test/camera/buffer_import.cpp | 382 ++++------------ test/camera/capture.cpp | 48 +- test/camera/statemachine.cpp | 89 +--- test/file-descriptor.cpp | 208 +++++++++ test/ipa/ipa_wrappers_test.cpp | 41 +- test/meson.build | 1 + test/v4l2_videodevice/buffer_sharing.cpp | 34 +- test/v4l2_videodevice/capture_async.cpp | 18 +- test/v4l2_videodevice/request_buffers.cpp | 11 +- test/v4l2_videodevice/stream_on_off.cpp | 6 +- test/v4l2_videodevice/v4l2_m2mdevice.cpp | 40 +- test/v4l2_videodevice/v4l2_videodevice_test.h | 2 +- 51 files changed, 2101 insertions(+), 1959 deletions(-) create mode 100644 include/libcamera/file_descriptor.h create mode 100644 include/libcamera/framebuffer_allocator.h create mode 100644 src/libcamera/file_descriptor.cpp create mode 100644 src/libcamera/framebuffer_allocator.cpp create mode 100644 test/file-descriptor.cpp