Re: [Xen-devel][PATCH v5 1/1] cameraif: add ABI for para-virtual camera

[Hans Verkuil <hverkuil@xs4all.nl>]

Hi Oleksandr,

Just one comment:

On 3/12/19 9:20 AM, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> 
> This is the ABI for the two halves of a para-virtualized
> camera driver which extends Xen's reach multimedia capabilities even
> farther enabling it for video conferencing, In-Vehicle Infotainment,
> high definition maps etc.
> 
> The initial goal is to support most needed functionality with the
> final idea to make it possible to extend the protocol if need be:
> 
> 1. Provide means for base virtual device configuration:
>  - pixel formats
>  - resolutions
>  - frame rates
> 2. Support basic camera controls:
>  - contrast
>  - brightness
>  - hue
>  - saturation
> 3. Support streaming control
> 
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> ---
>  xen/include/public/io/cameraif.h | 1370 ++++++++++++++++++++++++++++++
>  1 file changed, 1370 insertions(+)
>  create mode 100644 xen/include/public/io/cameraif.h
> 
> diff --git a/xen/include/public/io/cameraif.h b/xen/include/public/io/cameraif.h
> new file mode 100644
> index 000000000000..1ae4c51ea758
> --- /dev/null
> +++ b/xen/include/public/io/cameraif.h
> @@ -0,0 +1,1370 @@

<snip>

> +/*
> + * Request camera buffer's layout:
> + *         0                1                 2               3        octet
> + * +----------------+----------------+----------------+----------------+
> + * |               id                | _BUF_GET_LAYOUT|   reserved     | 4
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 8
> + * +----------------+----------------+----------------+----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 64
> + * +----------------+----------------+----------------+----------------+
> + *
> + * See response format for this request.
> + *
> + *
> + * Request number of buffers to be used:
> + *         0                1                 2               3        octet
> + * +----------------+----------------+----------------+----------------+
> + * |               id                | _OP_BUF_REQUEST|   reserved     | 4
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 8
> + * +----------------+----------------+----------------+----------------+
> + * |    num_bufs    |                     reserved                     | 12
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 16
> + * +----------------+----------------+----------------+----------------+
> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
> + * +----------------+----------------+----------------+----------------+
> + * |                             reserved                              | 64
> + * +----------------+----------------+----------------+----------------+
> + *
> + * num_bufs - uint8_t, desired number of buffers to be used.
> + *
> + * If num_bufs is not zero then the backend validates the requested number of
> + * buffers and responds with the number of buffers allowed for this frontend.
> + * Frontend is responsible for checking the corresponding response in order to
> + * see if the values reported back by the backend do match the desired ones
> + * and can be accepted.
> + * Frontend is allowed to send multiple XENCAMERA_OP_BUF_REQUEST requests
> + * before sending XENCAMERA_OP_STREAM_START request to update or tune the
> + * final configuration.
> + * Frontend is not allowed to change the number of buffers and/or camera
> + * configuration after the streaming has started.

This last sentence isn't quite right, and I missed that when reviewing the
proposed text during the v4 discussions.

The bit about not being allowed to change the number of buffers when streaming
has started is correct.

But the camera configuration is more strict: you can't change the camera
configuration after this request unless you call this again with num_bufs = 0.

The camera configuration changes the buffer size, so once the buffers are
allocated you can no longer change the camera config. It is unrelated to streaming.

Regards,

	Hans

> + *
> + * If num_bufs is 0 and streaming has not started yet, then the backend will
> + * free all previously allocated buffers (if any).
> + * Trying to call this if streaming is in progress will result in an error.
> + *
> + * If camera reconfiguration is required then the streaming must be stopped
> + * and this request must be sent with num_bufs set to zero and finally
> + * buffers destroyed.
> + *
> + * Please note, that the number of buffers in this request must not exceed
> + * the value configured in XenStore.max-buffers.
> + *
> + * See response format for this request.
> + */

<snip>

Regards,

	Hans