On 3/12/19 10:35 AM, Oleksandr Andrushchenko wrote: > On 3/12/19 11:30 AM, Hans Verkuil wrote: >> On 3/12/19 10:08 AM, Oleksandr Andrushchenko wrote: >>> On 3/12/19 10:58 AM, Hans Verkuil wrote: >>>> Hi Oleksandr, >>>> >>>> Just one comment: >>>> >>>> On 3/12/19 9:20 AM, Oleksandr Andrushchenko wrote: >>>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx> >>>>> >>>>> 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@xxxxxxxx> >>>>> --- >>>>> 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. >>> Can you please give me a hint of what would be the right thing to put in? >> How about this: >> >> Frontend is not allowed to change the camera configuration after this call with >> a non-zero value of num_bufs. If camera reconfiguration is required then this >> request must be sent with num_bufs set to zero and any created buffers must be >> destroyed first. >> >> Frontend is not allowed to change the number of buffers after the streaming has started. > Sounds great, so I'll replace: > > "Frontend is not allowed to change the number of buffers and/or camera > configuration after the streaming has started." > > with: > > "Frontend is not allowed to change the camera configuration after this > call with > a non-zero value of num_bufs. If camera reconfiguration is required > then this > request must be sent with num_bufs set to zero and any created buffers > must be > destroyed first. > > Frontend is not allowed to change the number of buffers after the > streaming has started. > " > > Are these all the changes you see at the moment? Also this change below... >>>>> + * >>>>> + * 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. >> I would rewrite the last part as well: >> >> ...set to zero and any created buffers must be destroyed. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ And note my note below :-) >> >> >> Note that "any created buffers must be destroyed" is something that you need to >> check for in your code if I am not mistaken. ^^^^^^^^^^^^^^^^^ Regards, Hans
