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?
Thank you!
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
