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. > > Thank you, > Oleksandr >> 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. I would rewrite the last part as well: ...set to zero and any created buffers must be destroyed. 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 >>> + * >>> + * 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 >
