[libcamera-devel] [RFC PATCH 2/2] android: camera_request: Lifetime of a Camera3RequestDescriptor
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Nov 23 01:30:25 CET 2021
Hi Umang,
Thank you for the patch.
On Fri, Nov 19, 2021 at 06:45:06PM +0530, Umang Jain wrote:
> This commit provides a sketch regarding Camera3RequestDescriptor
> which is aids tracking each capture reuquest placed by the android
s/which is/which/
s/reuquest/request/
> framework to libcamera HAL.
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> src/android/camera_request.cpp | 97 ++++++++++++++++++++++++++++++++++
> 1 file changed, 97 insertions(+)
>
> diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp
> index 4e017792..824b667d 100644
> --- a/src/android/camera_request.cpp
> +++ b/src/android/camera_request.cpp
> @@ -18,6 +18,9 @@ using namespace libcamera;
> *
> * A utility class that groups information about a capture request to be later
> * reused at request complete time to notify the framework.
> + *
> + * Also, refer to the Camera3RequestDescriptor's lifetime diagram at the end of
> + * this file.
I was going to say that "end of this file" won't mean much after
compiling the documentation with Doxygen, but we don't use Doxygen for
the HAL :-) I would however move the diagram here, regardless of that.
> */
>
> Camera3RequestDescriptor::Camera3RequestDescriptor(
> @@ -105,3 +108,97 @@ Camera3RequestDescriptor::StreamBuffer::StreamBuffer(StreamBuffer &&) = default;
>
> Camera3RequestDescriptor::StreamBuffer &
> Camera3RequestDescriptor::StreamBuffer::operator=(Camera3RequestDescriptor::StreamBuffer &&) = default;
> +
> +/*******************************************************************************
> + * Lifetime of a Camera3RequestDescriptor tracking a capture request placed by
> + * Android Framework
> + *******************************************************************************
> + *
> + *
> + * Android Framework
> + * │
> + * │ ┌──────────────────────────────────┐
> + * │ │camera3_capture_request_t │
> + * │ │ │
> + * │ │Requested output streams │
> + * │ │ stream1 stream2 stream3 ... │
> + * │ └──────────────────────────────────┘
> + * ▼
> + * ┌─────────────────────────────────────────────────────────────┐
> + * │ libcamera HAL │
> + * ├─────────────────────────────────────────────────────────────┤
> + * │ CameraDevice │
> + * │ │
> + * │ processCaptureRequest(camera3_capture_request_t request) │
> + * │ │
> + * │ - Create Camera3RequestDescriptor tracking this request │
> + * │ - Streams requiring post-processing is stored as a │
s/is stored/are stored/
s/as a map Camera3Requestdescriptor::pendingStreamsToProcess/in the pendingStreamsToProcess map/
> + * │ map Camera3Requestdescriptor::pendingStreamsToProcess │
> + * │ - Add this Camera3RequestDescriptor to descriptors' queue │
> + * │ CameraDevice::descriptors_ │
> + * │ │ ┌─────────────────────────┐
> + * │ - Queue the capture request to libcamera core ────────────┤►│libcamera core │
> + * │ │ ├─────────────────────────┤
> + * │ │ │- Capture from Camera │
> + * │ │ │ │
> + * │ │ │- Emit │
> + * │ │ │ Camera::requestComplete│
> + * │ requestCompleted(Request *request) ◄───────────────────────┼─┼──── │
> + * │ │ │ │
> + * │ - Check request completion status │ └─────────────────────────┘
> + * │ │
> + * │ - If(pendingStreamsToProcess > 0) │
s/If(/if /
> + * │ Queue all entries from pendingStreamsToProcess │
> + * │ else │ │
> + * │ completeDescriptor() │ └──────────────────────┐
> + * │ │ │
> + * │ ┌──────────────────────────┴───┬──────────────────┐ │
> + * │ │ │ │ │
> + * │ ┌──────────▼────────────┐ ┌───────────▼─────────┐ ▼ │
> + * │ │CameraStream1 │ │CameraStream2 │ .... │
> + * │ ├┬───┬───┬──────────────┤ ├┬───┬───┬────────────┤ │
> + * │ ││ │ │ │ ││ │ │ │ │
> + * │ │▼───▼───▼──────────────┤ │▼───▼───▼────────────┤ │
> + * │ │PostProcessorWorker ├─ │PostProcessorWorker │ │
Is the "├─" there on purpose ?
> + * │ │ │ │ │ │
> + * │ │ xxxxxxxxxxxxxxxxxxx │ │ xxxxxxxxxxxxxxxxxxx │ │
> + * │ │ x PostProcessor x │ │ x PostProcessor x │ │
> + * │ │ x process() x │ │ x process() x │ │
> + * │ │ x x │ │ x x │ │
> + * │ │ x Emit x │ │ x Emit x │ │
> + * │ │ x processComplete x │ │ x processComplete x │ │
> + * │ │ x x │ │ x x │ │
> + * │ │ xxxxxxxxxxxxxxx│xxx │ │ xxxxxxxxxxxxxxx│xxx │ │
> + * │ │ │ │ │ │ │ │
> + * │ │ │ │ │ │ │ │
> + * │ └────────────────┼──────┘ └────────────────┼────┘ │
> + * │ │ │ │
> + * │ │ │ │
> + * │ │ │ │
> + * │ ▼ ▼ │
> + * │ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx │
> + * │ x CameraDevice x x CameraDevice x │
> + * │ x x x x │
> + * │ x streamProcessingComplete() x x streamProcessingComplete() x │
> + * │ x x x x │
> + * │ x - Check and set buffer status x x - Check and set buffer status x │
> + * │ x - Remove post-processing entry x x - Remove post-processing entry x │
> + * │ x from pendingStreamsToProcess x x from pendingStreamsToProcess x │
> + * │ x x x x │
> + * │ x - If(pendingStreamsToProcess.empty()x x - If(pendingStreamsToProcess.empty()x │
s/If(/if (/
s/empty()/empty())/
> + * │ x completeDescriptor x x completeDescriptor x │
> + * │ x x x x │
> + * │ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx │
You could remove the text from the second box to avoid the duplication,
and just write ....
> + * │ │
> + * └────────────────────────────────────────────────────────────────────────────────────┘
> + *
> + *
> + *
> + *
> + *
> + *
I'd keep a single blank line only.
> + * xxxxxxxxxxxxxx
> + * x x - PostProcessorWorker's thread
> + * x x
> + * xxxxxxxxxxxxxx
Could this be easier to read with a double-line box instead of x's ?
╔═══════╗
║ ║
║ ║
║ ║
╚═══════╝
(and for completeness, if you need the other characters)
╔═══╦═══╗
║ ║ ║
╠═══╬═══╣
║ ║ ║
╚═══╩═══╝
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> + */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list