Re: [Xen-devel] [PATCH v3 1/1] cameraif: add ABI for para-virtual camera
Hi Oleksandr, This is looking a lot better than v2. I do have a few remaining comments about some things that are a bit unclear to me. On 12/12/18 10:49 AM, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko > > 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 > --- > xen/include/public/io/cameraif.h | 1374 ++ > 1 file changed, 1374 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 ..9aae0f47743b > --- /dev/null > +++ b/xen/include/public/io/cameraif.h > @@ -0,0 +1,1374 @@ > +/** > + * cameraif.h > + * > + * Unified camera device I/O interface for Xen guest OSes. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > copy > + * of this software and associated documentation files (the "Software"), to > + * deal in the Software without restriction, including without limitation the > + * rights to use, copy, modify, merge, publish, distribute, sublicense, > and/or > + * sell copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > THE > + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > + * DEALINGS IN THE SOFTWARE. > + * > + * Copyright (C) 2018 EPAM Systems Inc. > + * > + * Author: Oleksandr Andrushchenko > + */ > + > +#ifndef __XEN_PUBLIC_IO_CAMERAIF_H__ > +#define __XEN_PUBLIC_IO_CAMERAIF_H__ > + > +#include "ring.h" > +#include "../grant_table.h" > + > +/* > + > ** > + * Protocol version > + > ** > + */ > +#define XENCAMERA_PROTOCOL_VERSION "1" > + > +/* > + > ** > + * Feature and Parameter Negotiation > + > ** > + * > + * Front->back notifications: when enqueuing a new request, sending a > + * notification can be made conditional on xencamera_req (i.e., the generic > + * hold-off mechanism provided by the ring macros). Backends must set > + * xencamera_req appropriately (e.g., using RING_FINAL_CHECK_FOR_REQUESTS()). > + * > + * Back->front notifications: when enqueuing a new response, sending a > + * notification can be made conditional on xencamera_resp (i.e., the generic > + * hold-off mechanism provided by the ring macros). Frontends must set > + * xencamera_resp appropriately (e.g., using > RING_FINAL_CHECK_FOR_RESPONSES()). > + * > + * The two halves of a para-virtual camera driver utilize nodes within > + * XenStore to communicate capabilities and to negotiate operating > parameters. > + * This section enumerates these nodes which reside in the respective front > and > + * backend portions of XenStore, following the XenBus convention. > + * > + * All data in XenStore is stored as strings. Nodes specifying numeric > + * values are encoded in decimal. Integer value ranges listed below are > + * expressed as fixed sized integer types capable of storing the conversion > + * of a properly formatted node string, without loss of information. > + * > + > ** > + *Example configuration > + > ** > + * > + * This is an example of backend and frontend configuration: > + * > + *- Backend > --- > + * >
Re: [Xen-devel] [PATCH v3 1/1] cameraif: add ABI for para-virtual camera
On 1/14/19 10:23 AM, Oleksandr Andrushchenko wrote: > Hello, Hans! > Could you please take a look at my answers below and kindly let me know > if we are ready for (final?) v4 from your point of view. Looks good, so I'm looking forward to a v4. Regards, Hans > > Konrad, Xen-devel - do you have any objections/comments on this? > > Thank you, > Oleksandr > > On 12/17/18 9:37 AM, Oleksandr Andrushchenko wrote: >> Hello, Hans! >> >> Thank you for reviewing, please find my answers inline >> >> On 12/14/18 2:14 PM, Hans Verkuil wrote: >>> Hi Oleksandr, >>> >>> This is looking a lot better than v2. I do have a few remaining comments >>> about >>> some things that are a bit unclear to me. >>> >>> On 12/12/18 10:49 AM, Oleksandr Andrushchenko wrote: >>>> From: Oleksandr Andrushchenko >>>> >>>> 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 >>>> --- >>>> xen/include/public/io/cameraif.h | 1374 ++ >>>> 1 file changed, 1374 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 ..9aae0f47743b >>>> --- /dev/null >>>> +++ b/xen/include/public/io/cameraif.h >>>> @@ -0,0 +1,1374 @@ >>>> +/** >>>> + * cameraif.h >>>> + * >>>> + * Unified camera device I/O interface for Xen guest OSes. >>>> + * >>>> + * Permission is hereby granted, free of charge, to any person obtaining >>>> a copy >>>> + * of this software and associated documentation files (the "Software"), >>>> to >>>> + * deal in the Software without restriction, including without limitation >>>> the >>>> + * rights to use, copy, modify, merge, publish, distribute, sublicense, >>>> and/or >>>> + * sell copies of the Software, and to permit persons to whom the >>>> Software is >>>> + * furnished to do so, subject to the following conditions: >>>> + * >>>> + * The above copyright notice and this permission notice shall be >>>> included in >>>> + * all copies or substantial portions of the Software. >>>> + * >>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >>>> EXPRESS OR >>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >>>> MERCHANTABILITY, >>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT >>>> SHALL THE >>>> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER >>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING >>>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER >>>> + * DEALINGS IN THE SOFTWARE. >>>> + * >>>> + * Copyright (C) 2018 EPAM Systems Inc. >>>> + * >>>> + * Author: Oleksandr Andrushchenko >>>> + */ >>>> + >>>> +#ifndef __XEN_PUBLIC_IO_CAMERAIF_H__ >>>> +#define __XEN_PUBLIC_IO_CAMERAIF_H__ >>>> + >>>> +#include "ring.h" >>>> +#include "../grant_table.h" >>>> + >>>> +/* >>>> + >>>> ** >>>> + * Protocol version >>>> + >>>&g
Re: [Xen-devel] [PATCH v4 1/1] cameraif: add ABI for para-virtual camera
Hi Oleksandr, Just two remaining comments: On 1/15/19 10:38 AM, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko > > 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 > --- > xen/include/public/io/cameraif.h | 1364 ++ > 1 file changed, 1364 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 ..246eb2457f40 > --- /dev/null > +++ b/xen/include/public/io/cameraif.h > @@ -0,0 +1,1364 @@ > +/* > + > ** > + * EVENT CODES > + > ** > + */ > +#define XENCAMERA_EVT_FRAME_AVAIL 0x00 > +#define XENCAMERA_EVT_CTRL_CHANGE 0x01 > + > +/* Resolution has changed. */ > +#define XENCAMERA_EVT_CFG_FLG_RESOL(1 << 0) I think this flag is a left-over from v2 and should be removed. > + * Request number of buffers to be used: > + * 01 2 3octet > + * +++++ > + * | 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. This is > + * limited to the value configured in XenStore.max-buffers. > + * Passing zero num_bufs in this request (after streaming has stopped > + * and all buffers destroyed) unblocks camera configuration changes. I think the phrase 'unblocks camera configuration changes' is confusing. In v3 this sentence came after the third note below, and so it made sense in that context, but now the order has been reversed and it became hard to understand. I'm not sure what the best approach is to fix this. One option is to remove the third note and integrate it somehow in the sentence above. Or perhaps do away with the 'notes' at all and just write a more extensive documentation for this op. I leave that up to you. > + * > + * See response format for this request. > + * > + * Notes: > + * - frontend must check 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 may send multiple XENCAMERA_OP_BUF_REQUEST requests before > + *sending XENCAMERA_OP_STREAM_START request to update or tune the > + *configuration. > + * - after this request camera configuration cannot be changed, unless camera configuration -> the camera configuration > + *streaming is stopped and buffers destroyed > + */ Regards, Hans ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/1] cameraif: add ABI for para-virtual camera
Hi Oleksandr, On 09/03/2018 12:16 PM, Oleksandr Andrushchenko wrote: > On 08/21/2018 08:54 AM, Oleksandr Andrushchenko wrote: >> On 08/14/2018 11:30 AM, Juergen Gross wrote: >>> On 31/07/18 11:31, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko 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 4. Support zero-copying use-cases Signed-off-by: Oleksandr Andrushchenko >>> Some style issues below... >> Will fix all the below, thank you! >> >> I would like to draw some attention of the Linux/V4L community to this >> protocol as the plan is that once it is accepted for Xen we plan to >> upstream a Linux camera front-end kernel driver which will be based >> on this work and will be a V4L2 device driver (this is why I have sent >> this patch not only to Xen, but to the corresponding Linux mailing list >> as well) > ping Sorry, this got buried in my mailbox, I only came across it today. I'll try to review this this week, if not, just ping me again. I had one high-level question, though: What types of hardware do you intend to target? This initial version targets (very) simple webcams, but what about HDMI or SDTV receivers? Or hardware codecs? Or complex embedded video pipelines? In other words, where are you planning to draw the line? Even with just simple cameras there is a difference between regular UVC webcams and cameras used with embedded systems: for the latter you often need to provide more control w.r.t. white-balancing etc., things that a UVC webcam will generally do for you in the webcam's firmware. Regards, Hans ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/1] cameraif: add ABI for para-virtual camera
Hi Oleksandr, Sorry for the delay in reviewing, I missed this patch until you pinged me, and I was very busy after that as well. On 07/31/2018 11:31 AM, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko > > 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 > 4. Support zero-copying use-cases > > Signed-off-by: Oleksandr Andrushchenko > --- > xen/include/public/io/cameraif.h | 981 +++ > 1 file changed, 981 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 ..bdc6a1262fcf > --- /dev/null > +++ b/xen/include/public/io/cameraif.h > @@ -0,0 +1,981 @@ > +/** > + * cameraif.h > + * > + * Unified camera device I/O interface for Xen guest OSes. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > copy > + * of this software and associated documentation files (the "Software"), to > + * deal in the Software without restriction, including without limitation the > + * rights to use, copy, modify, merge, publish, distribute, sublicense, > and/or > + * sell copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > THE > + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > + * DEALINGS IN THE SOFTWARE. > + * > + * Copyright (C) 2018 EPAM Systems Inc. > + * > + * Author: Oleksandr Andrushchenko > + */ Use SPDX tag instead of copying the license text. > + > +#ifndef __XEN_PUBLIC_IO_CAMERAIF_H__ > +#define __XEN_PUBLIC_IO_CAMERAIF_H__ > + > +#include "ring.h" > +#include "../grant_table.h" > + > +/* > + > ** > + * Protocol version > + > ** > + */ > +#define XENCAMERA_PROTOCOL_VERSION "1" > + > +/* > + > ** > + * Feature and Parameter Negotiation > + > ** > + * > + * Front->back notifications: when enqueuing a new request, sending a > + * notification can be made conditional on xencamera_req (i.e., the generic > + * hold-off mechanism provided by the ring macros). Backends must set > + * xencamera_req appropriately (e.g., using RING_FINAL_CHECK_FOR_REQUESTS()). > + * > + * Back->front notifications: when enqueuing a new response, sending a > + * notification can be made conditional on xencamera_resp (i.e., the generic > + * hold-off mechanism provided by the ring macros). Frontends must set > + * xencamera_resp appropriately (e.g., using > RING_FINAL_CHECK_FOR_RESPONSES()). > + * > + * The two halves of a para-virtual camera driver utilize nodes within > + * XenStore to communicate capabilities and to negotiate operating > parameters. > + * This section enumerates these nodes which reside in the respective front > and > + * backend portions of XenStore, following the XenBus convention. > + * > + * All data in XenStore is stored as strings. Nodes specifying numeric > + * values are encoded in decimal. Integer value ranges listed below are > + * expressed as fixed sized integer types capable of storing the conversion > + * of a properly formatted node string, without loss of information. > + * > + > ** > + *Example configuration > + > ** > + * > + * This is an example of backend and frontend configuration: > + * > + *--
Re: [Xen-devel] [PATCH 1/1] cameraif: add ABI for para-virtual camera
On 09/04/2018 08:56 AM, Oleksandr Andrushchenko wrote: > On 09/03/2018 06:25 PM, Hans Verkuil wrote: >> Hi Oleksandr, >> >> On 09/03/2018 12:16 PM, Oleksandr Andrushchenko wrote: >>> On 08/21/2018 08:54 AM, Oleksandr Andrushchenko wrote: >>>> On 08/14/2018 11:30 AM, Juergen Gross wrote: >>>>> On 31/07/18 11:31, Oleksandr Andrushchenko wrote: >>>>>> From: Oleksandr Andrushchenko >>>>>> >>>>>> 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 >>>>>> 4. Support zero-copying use-cases >>>>>> >>>>>> Signed-off-by: Oleksandr Andrushchenko >>>>>> >>>>> Some style issues below... >>>> Will fix all the below, thank you! >>>> >>>> I would like to draw some attention of the Linux/V4L community to this >>>> protocol as the plan is that once it is accepted for Xen we plan to >>>> upstream a Linux camera front-end kernel driver which will be based >>>> on this work and will be a V4L2 device driver (this is why I have sent >>>> this patch not only to Xen, but to the corresponding Linux mailing list >>>> as well) >>> ping >> Sorry, this got buried in my mailbox, I only came across it today. I'll try >> to review this this week, if not, just ping me again. > Thank you for your time >> >> I had one high-level question, though: >> >> What types of hardware do you intend to target? This initial version targets >> (very) simple webcams, but what about HDMI or SDTV receivers? Or hardware >> codecs? Or complex embedded video pipelines? >> >> In other words, where are you planning to draw the line? >> >> Even with just simple cameras there is a difference between regular UVC >> webcams and cameras used with embedded systems: for the latter you often >> need to provide more control w.r.t. white-balancing etc., things that a >> UVC webcam will generally do for you in the webcam's firmware. > The use-cases we want to implement are mostly in automotive/embedded domain, > so there are many performance restrictions apply. > We are not targeting virtualizing very complex hardware and have no > intention > to make a 1:1 mapping of the real hardware: for that one can pass-through > a real HW device to a virtual machine (VM). The goal is to share a single > camera device to multiple virtual machines, no codecs, receivers etc. > > Controlling the same HW device from different VMs doesn't look feasible: > what if the same control is set to different values from different VMs? You can do this, actually: in V4L2 you can get an event when another process changes a control, and update your own GUI/internal state accordingly. So in this case if one VM changes a control, an event is sent to all others that the control has changed value. > Of course, this can be achieved if the corresponding backend can > post-process > original camera image with GPU, for example, thus applying different filters > for different VMs effectively emulating camera controls. > But this requires additional CPU/GPU power which we try to avoid. > > System partitioning (camera and controls assignment) is done at > configuration > time (remember we are in automotive/embedded world, so most of the time > the set > of VMs requiring cameras is known at this stage and the configuration > remains > static at run-time). So, when para-virtualized (PV) approach is used then we > only implement very basic controls (those found in the protocol), so one can > assign set of controls (all or some) to one of the VMs (main or mission > critical > VM or whatever) allowing that VM to adjusts those for all VMs at once. > For other > VMs think of it
Re: [Xen-devel] [PATCH 1/1] cameraif: add ABI for para-virtual camera
Hi Oleksandr, On 09/10/2018 09:16 AM, Oleksandr Andrushchenko wrote: > Hi, Hans! > > On 09/09/2018 01:31 PM, Hans Verkuil wrote: >> Hi Oleksandr, >> >> Sorry for the delay in reviewing, I missed this patch until you pinged me, >> and >> I was very busy after that as well. > I do appreciate you spending time on this! >> >> On 07/31/2018 11:31 AM, Oleksandr Andrushchenko wrote: >>> From: Oleksandr Andrushchenko >>> >>> 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 >>> 4. Support zero-copying use-cases >>> >>> Signed-off-by: Oleksandr Andrushchenko >>> --- >>> xen/include/public/io/cameraif.h | 981 +++ >>> 1 file changed, 981 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 ..bdc6a1262fcf >>> --- /dev/null >>> +++ b/xen/include/public/io/cameraif.h >>> @@ -0,0 +1,981 @@ >>> +/** >>> + * cameraif.h >>> + * >>> + * Unified camera device I/O interface for Xen guest OSes. >>> + * >>> + * Permission is hereby granted, free of charge, to any person obtaining a >>> copy >>> + * of this software and associated documentation files (the "Software"), to >>> + * deal in the Software without restriction, including without limitation >>> the >>> + * rights to use, copy, modify, merge, publish, distribute, sublicense, >>> and/or >>> + * sell copies of the Software, and to permit persons to whom the Software >>> is >>> + * furnished to do so, subject to the following conditions: >>> + * >>> + * The above copyright notice and this permission notice shall be included >>> in >>> + * all copies or substantial portions of the Software. >>> + * >>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS >>> OR >>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, >>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL >>> THE >>> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER >>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING >>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER >>> + * DEALINGS IN THE SOFTWARE. >>> + * >>> + * Copyright (C) 2018 EPAM Systems Inc. >>> + * >>> + * Author: Oleksandr Andrushchenko >>> + */ >> Use SPDX tag instead of copying the license text. > This is yet a Xen header which belongs to Xen project and > all the rest of the protocols have the same license header. > If Xen community decides to use SPDX then I'll definitely follow. Ah, yes, I was reviewing this as a kernel header, I hadn't realized that this isn't a kernel header. Since it isn't a kernel header, you can disregard my comments about style and naming conventions, since you have your own. > > Konrad, do you think this is the right time for such a move? >> >>> + >>> +#ifndef __XEN_PUBLIC_IO_CAMERAIF_H__ >>> +#define __XEN_PUBLIC_IO_CAMERAIF_H__ >>> + >>> +#include "ring.h" >>> +#include "../grant_table.h" >>> + >>> +/* >>> + >>> ** >>> + * Protocol version >>> + >>> ** >>> + */ >>> +#define XENCAMERA_PROTOCOL_VERSION "1" >>> + >>>
Re: [Xen-devel] [PATCH 1/1] cameraif: add ABI for para-virtual camera
On 09/10/2018 07:59 AM, Oleksandr Andrushchenko wrote: > Hi, Hans! > > On 09/09/2018 01:42 PM, Hans Verkuil wrote: >> On 09/04/2018 08:56 AM, Oleksandr Andrushchenko wrote: >>> On 09/03/2018 06:25 PM, Hans Verkuil wrote: >>>> Hi Oleksandr, >>>> >>>> On 09/03/2018 12:16 PM, Oleksandr Andrushchenko wrote: >>>>> On 08/21/2018 08:54 AM, Oleksandr Andrushchenko wrote: >>>>>> On 08/14/2018 11:30 AM, Juergen Gross wrote: >>>>>>> On 31/07/18 11:31, Oleksandr Andrushchenko wrote: >>>>>>>> From: Oleksandr Andrushchenko >>>>>>>> >>>>>>>> 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 >>>>>>>> 4. Support zero-copying use-cases >>>>>>>> >>>>>>>> Signed-off-by: Oleksandr Andrushchenko >>>>>>>> >>>>>>> Some style issues below... >>>>>> Will fix all the below, thank you! >>>>>> >>>>>> I would like to draw some attention of the Linux/V4L community to this >>>>>> protocol as the plan is that once it is accepted for Xen we plan to >>>>>> upstream a Linux camera front-end kernel driver which will be based >>>>>> on this work and will be a V4L2 device driver (this is why I have sent >>>>>> this patch not only to Xen, but to the corresponding Linux mailing list >>>>>> as well) >>>>> ping >>>> Sorry, this got buried in my mailbox, I only came across it today. I'll try >>>> to review this this week, if not, just ping me again. >>> Thank you for your time >>>> I had one high-level question, though: >>>> >>>> What types of hardware do you intend to target? This initial version >>>> targets >>>> (very) simple webcams, but what about HDMI or SDTV receivers? Or hardware >>>> codecs? Or complex embedded video pipelines? >>>> >>>> In other words, where are you planning to draw the line? >>>> >>>> Even with just simple cameras there is a difference between regular UVC >>>> webcams and cameras used with embedded systems: for the latter you often >>>> need to provide more control w.r.t. white-balancing etc., things that a >>>> UVC webcam will generally do for you in the webcam's firmware. >>> The use-cases we want to implement are mostly in automotive/embedded domain, >>> so there are many performance restrictions apply. >>> We are not targeting virtualizing very complex hardware and have no >>> intention >>> to make a 1:1 mapping of the real hardware: for that one can pass-through >>> a real HW device to a virtual machine (VM). The goal is to share a single >>> camera device to multiple virtual machines, no codecs, receivers etc. >>> >>> Controlling the same HW device from different VMs doesn't look feasible: >>> what if the same control is set to different values from different VMs? >> You can do this, actually: in V4L2 you can get an event when another process >> changes a control, and update your own GUI/internal state accordingly. >> >> So in this case if one VM changes a control, an event is sent to all others >> that the control has changed value. > Well, technically this can be done by introducing one more > event for such a notification. But, from system partitioning > POV, I am still not convinced this should be done: I would prefer > that a single V
Re: [Xen-devel] [PATCH 1/1] cameraif: add ABI for para-virtual camera
On 09/10/2018 10:24 AM, Oleksandr Andrushchenko wrote:
> On 09/10/2018 10:53 AM, Hans Verkuil wrote:
>> Hi Oleksandr,
>>
>> On 09/10/2018 09:16 AM, Oleksandr Andrushchenko wrote:
>>>> I suspect that you likely will want to support such sources eventually, so
>>>> it pays to design this with that in mind.
>>> Again, I think that this is the backend to hide these
>>> use-cases from the frontend.
>> I'm not sure you can: say you are playing a bluray connected to the system
>> with HDMI, then if there is a resolution change, what do you do? You can tear
>> everything down and build it up again, or you can just tell frontends that
>> something changed and that they have to look at the new vcamera
>> configuration.
>>
>> The latter seems to be more sensible to me. It is really not much that you
>> need to do: all you really need is an event signalling that something
>> changed.
>> In V4L2 that's the V4L2_EVENT_SOURCE_CHANGE.
> well, this complicates things a lot as I'll have to
> re-allocate buffers - right?
Right. Different resolutions means different sized buffers and usually lots of
changes throughout the whole video pipeline, which in this case can even
go into multiple VMs.
One additional thing to keep in mind for the future: V4L2_EVENT_SOURCE_CHANGE
has a flags field that tells userspace what changed. Right now that is just the
resolution, but in the future you can expect flags for cases where just the
colorspace information changes, but not the resolution.
Which reminds me of two important missing pieces of information in your
protocol:
1) You need to communicate the colorspace data:
- colorspace
- xfer_func
- ycbcr_enc/hsv_enc (unlikely you ever want to support HSV pixelformats, so I
think you can ignore hsv_enc)
- quantization
See
https://hverkuil.home.xs4all.nl/spec/uapi/v4l/pixfmt-v4l2.html#c.v4l2_pix_format
and the links to the colorspace sections in the V4L2 spec for details).
This information is part of the format, it is reported by the driver.
2) If you support interlaced formats and V4L2_FIELD_ALTERNATE (i.e.
each buffer contains a single field), then you need to be able to tell
userspace whether the dequeued buffer contains a top or bottom field.
Also, what to do with dropped frames/fields: V4L2 has a sequence counter and
timestamp that can help detecting that. You probably need something similar.
> But anyways, I can add
> #define XENCAMERA_EVT_CFG_CHANGE 0x01
> in the protocol, so we can address this use-case
>>
>>> 1. set format command:
>>>* pixel_format - uint32_t, pixel format to be used, FOURCC code.
>>>* width - uint32_t, width in pixels.
>>>* height - uint32_t, height in pixels.
>>>
>>> 2. Set frame rate command:
>>>+ * frame_rate_numer - uint32_t, numerator of the frame rate.
>>>+ * frame_rate_denom - uint32_t, denominator of the frame rate.
>>>
>>> 3. Set/request num bufs:
>>>* num_bufs - uint8_t, desired number of buffers to be used.
>> I like this much better. 1+2 could be combined, but 3 should definitely
>> remain
>> separate.
> ok, then 1+2 combined + 3 separate.
> Do you think we can still name 1+2 as "set_format" or "set_config"
> will fit better?
set_format is closer to S_FMT as used in V4L2, so I have a slight preference
for that, but it is really up to you.
>>
>>>>> + *
>>>>> + * See response format for this request.
>>>>> + *
>>>>> + * Notes:
>>>>> + * - frontend must check 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 may send multiple XENCAMERA_OP_SET_CONFIG requests before
>>>>> + *sending XENCAMERA_OP_STREAM_START request to update or tune the
>>>>> + *configuration.
>>>>> + */
>>>>> +struct xencamera_config {
>>>>> +uint32_t pixel_format;
>>>>> +uint32_t width;
>>>>> +uint32_t height;
>>>>> +uint32_t frame_rate_nom;
>>>>> +uint32_t frame_rate_denom;
>>>>> +uint8_t num_bufs;
>>>>> +};
>>>>> +
>>>>> +/*
>>>>> + * Request buffer details - request camera buffer's memory layout.
>>>>> + * detailed description:
>>>>> + * 01 2
Re: [Xen-devel] [PATCH 1/1] cameraif: add ABI for para-virtual camera
On 09/10/2018 11:52 AM, Oleksandr Andrushchenko wrote: > On 09/10/2018 12:04 PM, Hans Verkuil wrote: >> On 09/10/2018 10:24 AM, Oleksandr Andrushchenko wrote: >>> On 09/10/2018 10:53 AM, Hans Verkuil wrote: >>>> Hi Oleksandr, >>>> >>>> On 09/10/2018 09:16 AM, Oleksandr Andrushchenko wrote: >> >> >>>>>> I suspect that you likely will want to support such sources eventually, >>>>>> so >>>>>> it pays to design this with that in mind. >>>>> Again, I think that this is the backend to hide these >>>>> use-cases from the frontend. >>>> I'm not sure you can: say you are playing a bluray connected to the system >>>> with HDMI, then if there is a resolution change, what do you do? You can >>>> tear >>>> everything down and build it up again, or you can just tell frontends that >>>> something changed and that they have to look at the new vcamera >>>> configuration. >>>> >>>> The latter seems to be more sensible to me. It is really not much that you >>>> need to do: all you really need is an event signalling that something >>>> changed. >>>> In V4L2 that's the V4L2_EVENT_SOURCE_CHANGE. >>> well, this complicates things a lot as I'll have to >>> re-allocate buffers - right? >> Right. Different resolutions means different sized buffers and usually lots >> of >> changes throughout the whole video pipeline, which in this case can even >> go into multiple VMs. >> >> One additional thing to keep in mind for the future: V4L2_EVENT_SOURCE_CHANGE >> has a flags field that tells userspace what changed. Right now that is just >> the >> resolution, but in the future you can expect flags for cases where just the >> colorspace information changes, but not the resolution. >> >> Which reminds me of two important missing pieces of information in your >> protocol: >> >> 1) You need to communicate the colorspace data: >> >> - colorspace >> - xfer_func >> - ycbcr_enc/hsv_enc (unlikely you ever want to support HSV pixelformats, so I >>think you can ignore hsv_enc) >> - quantization >> >> See >> https://hverkuil.home.xs4all.nl/spec/uapi/v4l/pixfmt-v4l2.html#c.v4l2_pix_format >> and the links to the colorspace sections in the V4L2 spec for details). >> >> This information is part of the format, it is reported by the driver. > I'll take a look and think what can be put and how into the protocol, > do you think I'll have to implement all the above for > this stage? Yes. Without it VMs will have no way of knowing how to reproduce the right colors. They don't *have* to use this information, but it should be there. For cameras this isn't all that important, for SDTV/HDTV sources this becomes more relevant (esp. the quantization and ycbcr_enc information) and for sources with BT.2020/HDR formats this is critical. The vivid driver can actually reproduce all combinations, so that's a good driver to test this with. > >> >> 2) If you support interlaced formats and V4L2_FIELD_ALTERNATE (i.e. >> each buffer contains a single field), then you need to be able to tell >> userspace whether the dequeued buffer contains a top or bottom field. > I think at the first stage we can assume that interlaced > formats are not supported and add such support later if need be. Frankly I consider that a smart move :-) Interlaced formats are awful... You just have to keep this in mind if you ever have to add support for this. >> >> Also, what to do with dropped frames/fields: V4L2 has a sequence counter and >> timestamp that can help detecting that. You probably need something similar. > Ok, this can be reported as part of XENCAMERA_EVT_FRAME_AVAIL event >> >>> But anyways, I can add >>> #define XENCAMERA_EVT_CFG_CHANGE 0x01 >>> in the protocol, so we can address this use-case >> >> >>>>> 1. set format command: >>>>> * pixel_format - uint32_t, pixel format to be used, FOURCC code. >>>>> * width - uint32_t, width in pixels. >>>>> * height - uint32_t, height in pixels. >>>>> >>>>> 2. Set frame rate command: >>>>> + * frame_rate_numer - uint32_t, numerator of the frame rate. >>>>> + * frame_rate_denom - uint32_t, denominator of the frame rate. >>>>> >>>>> 3. Set/request num bufs: >>>>> * num_bufs - uint8_t, desired number of buffers to be used. &
Re: [Xen-devel] [PATCH 1/1] cameraif: add ABI for para-virtual camera
On 09/10/2018 01:49 PM, Oleksandr Andrushchenko wrote: > On 09/10/2018 02:09 PM, Hans Verkuil wrote: >> On 09/10/2018 11:52 AM, Oleksandr Andrushchenko wrote: >>> On 09/10/2018 12:04 PM, Hans Verkuil wrote: >>>> On 09/10/2018 10:24 AM, Oleksandr Andrushchenko wrote: >>>>> On 09/10/2018 10:53 AM, Hans Verkuil wrote: >>>>>> Hi Oleksandr, >>>>>> >>>>>> On 09/10/2018 09:16 AM, Oleksandr Andrushchenko wrote: >>>> >>>> >>>>>>>> I suspect that you likely will want to support such sources >>>>>>>> eventually, so >>>>>>>> it pays to design this with that in mind. >>>>>>> Again, I think that this is the backend to hide these >>>>>>> use-cases from the frontend. >>>>>> I'm not sure you can: say you are playing a bluray connected to the >>>>>> system >>>>>> with HDMI, then if there is a resolution change, what do you do? You can >>>>>> tear >>>>>> everything down and build it up again, or you can just tell frontends >>>>>> that >>>>>> something changed and that they have to look at the new vcamera >>>>>> configuration. >>>>>> >>>>>> The latter seems to be more sensible to me. It is really not much that >>>>>> you >>>>>> need to do: all you really need is an event signalling that something >>>>>> changed. >>>>>> In V4L2 that's the V4L2_EVENT_SOURCE_CHANGE. >>>>> well, this complicates things a lot as I'll have to >>>>> re-allocate buffers - right? >>>> Right. Different resolutions means different sized buffers and usually >>>> lots of >>>> changes throughout the whole video pipeline, which in this case can even >>>> go into multiple VMs. >>>> >>>> One additional thing to keep in mind for the future: >>>> V4L2_EVENT_SOURCE_CHANGE >>>> has a flags field that tells userspace what changed. Right now that is >>>> just the >>>> resolution, but in the future you can expect flags for cases where just the >>>> colorspace information changes, but not the resolution. >>>> >>>> Which reminds me of two important missing pieces of information in your >>>> protocol: >>>> >>>> 1) You need to communicate the colorspace data: >>>> >>>> - colorspace >>>> - xfer_func >>>> - ycbcr_enc/hsv_enc (unlikely you ever want to support HSV pixelformats, >>>> so I >>>> think you can ignore hsv_enc) >>>> - quantization >>>> >>>> See >>>> https://hverkuil.home.xs4all.nl/spec/uapi/v4l/pixfmt-v4l2.html#c.v4l2_pix_format >>>> and the links to the colorspace sections in the V4L2 spec for details). >>>> >>>> This information is part of the format, it is reported by the driver. >>> I'll take a look and think what can be put and how into the protocol, >>> do you think I'll have to implement all the above for >>> this stage? >> Yes. Without it VMs will have no way of knowing how to reproduce the right >> colors. >> They don't *have* to use this information, but it should be there. For >> cameras >> this isn't all that important, for SDTV/HDTV sources this becomes more >> relevant >> (esp. the quantization and ycbcr_enc information) and for sources with >> BT.2020/HDR >> formats this is critical. > ok, then I'll add the following to the set_config request/response: > > uint32_t colorspace; > uint32_t xfer_func; > uint32_t ycbcr_enc; > uint32_t quantization; > > With this respect, I will need to put some OS agnostic constants > into the protocol, so if backend and frontend are not Linux/V4L2 > based they can still talk to each other. > I see that V4L2 already defines constants for the above: [1], [2], [3], [4]. > > Do you think I can define the same replacing V4L2_ prefix > with XENCAMERA_, e.g. V4L2_XFER_FUNC_SRGB -> XENCAMERA_XFER_FUNC_SRGB? Yes. > > Do I need to define all those or there can be some subset of the > above for my simpler use-case? Most of these defines directly map to standards. I would skip the following defines: V4L2_COLORSPACE_DEFAULT (not applicable) V4L2_COLORSPACE_470_SYSTEM_* (rarely used, if received by the HW the Xen backend shoul
Re: [Xen-devel] [PATCH 1/1] cameraif: add ABI for para-virtual camera
On 09/11/2018 08:52 AM, Oleksandr Andrushchenko wrote: > Hi, Hans! > > On 09/10/2018 03:26 PM, Hans Verkuil wrote: >> On 09/10/2018 01:49 PM, Oleksandr Andrushchenko wrote: >>> On 09/10/2018 02:09 PM, Hans Verkuil wrote: >>>> On 09/10/2018 11:52 AM, Oleksandr Andrushchenko wrote: >>>>> On 09/10/2018 12:04 PM, Hans Verkuil wrote: >>>>>> On 09/10/2018 10:24 AM, Oleksandr Andrushchenko wrote: >>>>>>> On 09/10/2018 10:53 AM, Hans Verkuil wrote: >>>>>>>> Hi Oleksandr, >>>>>>>> >>>>>>>> On 09/10/2018 09:16 AM, Oleksandr Andrushchenko wrote: >>>>>> >>>>>> >>>>>>>>>> I suspect that you likely will want to support such sources >>>>>>>>>> eventually, so >>>>>>>>>> it pays to design this with that in mind. >>>>>>>>> Again, I think that this is the backend to hide these >>>>>>>>> use-cases from the frontend. >>>>>>>> I'm not sure you can: say you are playing a bluray connected to the >>>>>>>> system >>>>>>>> with HDMI, then if there is a resolution change, what do you do? You >>>>>>>> can tear >>>>>>>> everything down and build it up again, or you can just tell frontends >>>>>>>> that >>>>>>>> something changed and that they have to look at the new vcamera >>>>>>>> configuration. >>>>>>>> >>>>>>>> The latter seems to be more sensible to me. It is really not much that >>>>>>>> you >>>>>>>> need to do: all you really need is an event signalling that something >>>>>>>> changed. >>>>>>>> In V4L2 that's the V4L2_EVENT_SOURCE_CHANGE. >>>>>>> well, this complicates things a lot as I'll have to >>>>>>> re-allocate buffers - right? >>>>>> Right. Different resolutions means different sized buffers and usually >>>>>> lots of >>>>>> changes throughout the whole video pipeline, which in this case can even >>>>>> go into multiple VMs. >>>>>> >>>>>> One additional thing to keep in mind for the future: >>>>>> V4L2_EVENT_SOURCE_CHANGE >>>>>> has a flags field that tells userspace what changed. Right now that is >>>>>> just the >>>>>> resolution, but in the future you can expect flags for cases where just >>>>>> the >>>>>> colorspace information changes, but not the resolution. >>>>>> >>>>>> Which reminds me of two important missing pieces of information in your >>>>>> protocol: >>>>>> >>>>>> 1) You need to communicate the colorspace data: >>>>>> >>>>>> - colorspace >>>>>> - xfer_func >>>>>> - ycbcr_enc/hsv_enc (unlikely you ever want to support HSV pixelformats, >>>>>> so I >>>>>> think you can ignore hsv_enc) >>>>>> - quantization >>>>>> >>>>>> See >>>>>> https://hverkuil.home.xs4all.nl/spec/uapi/v4l/pixfmt-v4l2.html#c.v4l2_pix_format >>>>>> and the links to the colorspace sections in the V4L2 spec for details). >>>>>> >>>>>> This information is part of the format, it is reported by the driver. >>>>> I'll take a look and think what can be put and how into the protocol, >>>>> do you think I'll have to implement all the above for >>>>> this stage? >>>> Yes. Without it VMs will have no way of knowing how to reproduce the right >>>> colors. >>>> They don't *have* to use this information, but it should be there. For >>>> cameras >>>> this isn't all that important, for SDTV/HDTV sources this becomes more >>>> relevant >>>> (esp. the quantization and ycbcr_enc information) and for sources with >>>> BT.2020/HDR >>>> formats this is critical. >>> ok, then I'll add the following to the set_config request/response: >>> >>> uint32_t colorspace; >>> uint32_t xfer_func; >>> uint32_t ycbcr_enc; >>> uint32_t quantization; > Yet another question here: are the above (color space, xfer etc.) and > display aspect ratio defined per pixel_format or per pixel_format + > resolution? > > If per pixel_format then > > .../vcamera/1/formats/YUYV/display-aspect-ratio = "59/58" > > or if per resolution > > .../vcamera/1/formats/YUYV/640x480/display-aspect-ratio = "59/58" They are totally independent of resolution or pixelformat, with the exception of ycbcr_enc which is of course ignored for RGB pixelformats. They are set by the driver, never by the application. For HDMI sources these values can change depending on what source is connected, so they are not fixed and you need to query them whenever a new source is connected. In fact, then can change midstream, but we do not have good support for that at the moment. Regards, Hans ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/1] cameraif: add ABI for para-virtual camera
On 09/11/18 09:14, Oleksandr Andrushchenko wrote: > On 09/11/2018 10:04 AM, Hans Verkuil wrote: >> On 09/11/2018 08:52 AM, Oleksandr Andrushchenko wrote: >>> Hi, Hans! >>> >>> On 09/10/2018 03:26 PM, Hans Verkuil wrote: >>>> On 09/10/2018 01:49 PM, Oleksandr Andrushchenko wrote: >>>>> On 09/10/2018 02:09 PM, Hans Verkuil wrote: >>>>>> On 09/10/2018 11:52 AM, Oleksandr Andrushchenko wrote: >>>>>>> On 09/10/2018 12:04 PM, Hans Verkuil wrote: >>>>>>>> On 09/10/2018 10:24 AM, Oleksandr Andrushchenko wrote: >>>>>>>>> On 09/10/2018 10:53 AM, Hans Verkuil wrote: >>>>>>>>>> Hi Oleksandr, >>>>>>>>>> >>>>>>>>>> On 09/10/2018 09:16 AM, Oleksandr Andrushchenko wrote: >>>>>>>> >>>>>>>> >>>>>>>>>>>> I suspect that you likely will want to support such sources >>>>>>>>>>>> eventually, so >>>>>>>>>>>> it pays to design this with that in mind. >>>>>>>>>>> Again, I think that this is the backend to hide these >>>>>>>>>>> use-cases from the frontend. >>>>>>>>>> I'm not sure you can: say you are playing a bluray connected to the >>>>>>>>>> system >>>>>>>>>> with HDMI, then if there is a resolution change, what do you do? You >>>>>>>>>> can tear >>>>>>>>>> everything down and build it up again, or you can just tell >>>>>>>>>> frontends that >>>>>>>>>> something changed and that they have to look at the new vcamera >>>>>>>>>> configuration. >>>>>>>>>> >>>>>>>>>> The latter seems to be more sensible to me. It is really not much >>>>>>>>>> that you >>>>>>>>>> need to do: all you really need is an event signalling that >>>>>>>>>> something changed. >>>>>>>>>> In V4L2 that's the V4L2_EVENT_SOURCE_CHANGE. >>>>>>>>> well, this complicates things a lot as I'll have to >>>>>>>>> re-allocate buffers - right? >>>>>>>> Right. Different resolutions means different sized buffers and usually >>>>>>>> lots of >>>>>>>> changes throughout the whole video pipeline, which in this case can >>>>>>>> even >>>>>>>> go into multiple VMs. >>>>>>>> >>>>>>>> One additional thing to keep in mind for the future: >>>>>>>> V4L2_EVENT_SOURCE_CHANGE >>>>>>>> has a flags field that tells userspace what changed. Right now that is >>>>>>>> just the >>>>>>>> resolution, but in the future you can expect flags for cases where >>>>>>>> just the >>>>>>>> colorspace information changes, but not the resolution. >>>>>>>> >>>>>>>> Which reminds me of two important missing pieces of information in >>>>>>>> your protocol: >>>>>>>> >>>>>>>> 1) You need to communicate the colorspace data: >>>>>>>> >>>>>>>> - colorspace >>>>>>>> - xfer_func >>>>>>>> - ycbcr_enc/hsv_enc (unlikely you ever want to support HSV >>>>>>>> pixelformats, so I >>>>>>>> think you can ignore hsv_enc) >>>>>>>> - quantization >>>>>>>> >>>>>>>> See >>>>>>>> https://hverkuil.home.xs4all.nl/spec/uapi/v4l/pixfmt-v4l2.html#c.v4l2_pix_format >>>>>>>> and the links to the colorspace sections in the V4L2 spec for details). >>>>>>>> >>>>>>>> This information is part of the format, it is reported by the driver. >>>>>>> I'll take a look and think what can be put and how into the protocol, >>>>>>> do you think I'll have to implement all the above for >>>>>>> this stage? >>>>>> Yes. Without it VMs will have no way of knowing how to reproduce
Re: [Xen-devel] [PATCH v2 1/1] cameraif: add ABI for para-virtual camera
On 09/11/2018 10:29 AM, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko > > 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 > 4. Support zero-copying use-cases > > Signed-off-by: Oleksandr Andrushchenko > --- > xen/include/public/io/cameraif.h | 1263 ++ > 1 file changed, 1263 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 ..38b9b3741e75 > --- /dev/null > +++ b/xen/include/public/io/cameraif.h > @@ -0,0 +1,1263 @@ > +/** > + * cameraif.h > + * > + * Unified camera device I/O interface for Xen guest OSes. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > copy > + * of this software and associated documentation files (the "Software"), to > + * deal in the Software without restriction, including without limitation the > + * rights to use, copy, modify, merge, publish, distribute, sublicense, > and/or > + * sell copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > THE > + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > + * DEALINGS IN THE SOFTWARE. > + * > + * Copyright (C) 2018 EPAM Systems Inc. > + * > + * Author: Oleksandr Andrushchenko > + */ > + > +#ifndef __XEN_PUBLIC_IO_CAMERAIF_H__ > +#define __XEN_PUBLIC_IO_CAMERAIF_H__ > + > +#include "ring.h" > +#include "../grant_table.h" > + > +/* > + > ** > + * Protocol version > + > ** > + */ > +#define XENCAMERA_PROTOCOL_VERSION "1" > + > +/* > + > ** > + * Feature and Parameter Negotiation > + > ** > + * > + * Front->back notifications: when enqueuing a new request, sending a > + * notification can be made conditional on xencamera_req (i.e., the generic > + * hold-off mechanism provided by the ring macros). Backends must set > + * xencamera_req appropriately (e.g., using RING_FINAL_CHECK_FOR_REQUESTS()). > + * > + * Back->front notifications: when enqueuing a new response, sending a > + * notification can be made conditional on xencamera_resp (i.e., the generic > + * hold-off mechanism provided by the ring macros). Frontends must set > + * xencamera_resp appropriately (e.g., using > RING_FINAL_CHECK_FOR_RESPONSES()). > + * > + * The two halves of a para-virtual camera driver utilize nodes within > + * XenStore to communicate capabilities and to negotiate operating > parameters. > + * This section enumerates these nodes which reside in the respective front > and > + * backend portions of XenStore, following the XenBus convention. > + * > + * All data in XenStore is stored as strings. Nodes specifying numeric > + * values are encoded in decimal. Integer value ranges listed below are > + * expressed as fixed sized integer types capable of storing the conversion > + * of a properly formatted node string, without loss of information. > + * > + > ** > + *Example configuration > + > ** > + * > + * This is an example of backend and frontend configuration: > + * > + *- Backend > --- > + * > + * /local/domain/0/backend/vcamera/1/0/frontend-id = "1" > + * /local/domain/0/backend/vcamera/1/
Re: [Xen-devel] [PATCH v2 1/1] cameraif: add ABI for para-virtual camera
On 09/11/18 10:29, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko > > 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 > 4. Support zero-copying use-cases > > Signed-off-by: Oleksandr Andrushchenko > --- > xen/include/public/io/cameraif.h | 1263 ++ > 1 file changed, 1263 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 ..38b9b3741e75 > --- /dev/null > +++ b/xen/include/public/io/cameraif.h > @@ -0,0 +1,1263 @@ > +/** > + * cameraif.h > + * > + * Unified camera device I/O interface for Xen guest OSes. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > copy > + * of this software and associated documentation files (the "Software"), to > + * deal in the Software without restriction, including without limitation the > + * rights to use, copy, modify, merge, publish, distribute, sublicense, > and/or > + * sell copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > THE > + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > + * DEALINGS IN THE SOFTWARE. > + * > + * Copyright (C) 2018 EPAM Systems Inc. > + * > + * Author: Oleksandr Andrushchenko > + */ > + > +#ifndef __XEN_PUBLIC_IO_CAMERAIF_H__ > +#define __XEN_PUBLIC_IO_CAMERAIF_H__ > + > +#include "ring.h" > +#include "../grant_table.h" > + > +/* > + > ** > + * Protocol version > + > ** > + */ > +#define XENCAMERA_PROTOCOL_VERSION "1" > + > +/* > + > ** > + * Feature and Parameter Negotiation > + > ** > + * > + * Front->back notifications: when enqueuing a new request, sending a > + * notification can be made conditional on xencamera_req (i.e., the generic > + * hold-off mechanism provided by the ring macros). Backends must set > + * xencamera_req appropriately (e.g., using RING_FINAL_CHECK_FOR_REQUESTS()). > + * > + * Back->front notifications: when enqueuing a new response, sending a > + * notification can be made conditional on xencamera_resp (i.e., the generic > + * hold-off mechanism provided by the ring macros). Frontends must set > + * xencamera_resp appropriately (e.g., using > RING_FINAL_CHECK_FOR_RESPONSES()). > + * > + * The two halves of a para-virtual camera driver utilize nodes within > + * XenStore to communicate capabilities and to negotiate operating > parameters. > + * This section enumerates these nodes which reside in the respective front > and > + * backend portions of XenStore, following the XenBus convention. > + * > + * All data in XenStore is stored as strings. Nodes specifying numeric > + * values are encoded in decimal. Integer value ranges listed below are > + * expressed as fixed sized integer types capable of storing the conversion > + * of a properly formatted node string, without loss of information. > + * > + > ** > + *Example configuration > + > ** > + * > + * This is an example of backend and frontend configuration: > + * > + *- Backend > --- > + * > + * /local/domain/0/backend/vcamera/1/0/frontend-id = "1" > + * /local/domain/0/backend/vcamera/1/0/fro
Re: [Xen-devel] [PATCH v2 1/1] cameraif: add ABI for para-virtual camera
On 09/12/18 12:09, Oleksandr Andrushchenko wrote: > Hi, Hans! > > Thank you for valuable comments and valid concerns! > > On 09/12/2018 10:52 AM, Hans Verkuil wrote: >> On 09/11/2018 10:29 AM, Oleksandr Andrushchenko wrote: >>> From: Oleksandr Andrushchenko >>> >>> 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 >>> 4. Support zero-copying use-cases >>> >>> Signed-off-by: Oleksandr Andrushchenko >>> --- >>> xen/include/public/io/cameraif.h | 1263 ++ >>> 1 file changed, 1263 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 ..38b9b3741e75 >>> --- /dev/null >>> +++ b/xen/include/public/io/cameraif.h >>> @@ -0,0 +1,1263 @@ >>> +/** >>> + * cameraif.h >>> + * >>> + * Unified camera device I/O interface for Xen guest OSes. >>> + * >>> + * Permission is hereby granted, free of charge, to any person obtaining a >>> copy >>> + * of this software and associated documentation files (the "Software"), to >>> + * deal in the Software without restriction, including without limitation >>> the >>> + * rights to use, copy, modify, merge, publish, distribute, sublicense, >>> and/or >>> + * sell copies of the Software, and to permit persons to whom the Software >>> is >>> + * furnished to do so, subject to the following conditions: >>> + * >>> + * The above copyright notice and this permission notice shall be included >>> in >>> + * all copies or substantial portions of the Software. >>> + * >>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS >>> OR >>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, >>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL >>> THE >>> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER >>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING >>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER >>> + * DEALINGS IN THE SOFTWARE. >>> + * >>> + * Copyright (C) 2018 EPAM Systems Inc. >>> + * >>> + * Author: Oleksandr Andrushchenko >>> + */ >>> + >>> +#ifndef __XEN_PUBLIC_IO_CAMERAIF_H__ >>> +#define __XEN_PUBLIC_IO_CAMERAIF_H__ >>> + >>> +#include "ring.h" >>> +#include "../grant_table.h" >>> + >>> +/* >>> + >>> ** >>> + * Protocol version >>> + >>> ** >>> + */ >>> +#define XENCAMERA_PROTOCOL_VERSION "1" >>> + >>> +/* >>> + >>> ** >>> + * Feature and Parameter Negotiation >>> + >>> ** >>> + * >>> + * Front->back notifications: when enqueuing a new request, sending a >>> + * notification can be made conditional on xencamera_req (i.e., the generic >>> + * hold-off mechanism provided by the ring macros). Backends must set >>> + * xencamera_req appropriately (e.g., using >>> RING_FINAL_CHECK_FOR_REQUESTS()). >>> + * >>> + *
Re: [Xen-devel] [PATCH v2 1/1] cameraif: add ABI for para-virtual camera
On 09/12/18 15:02, Oleksandr Andrushchenko wrote:
> On 09/12/2018 03:25 PM, Hans Verkuil wrote:
>>>>> + * formats
>>>>> + * Values:
>>>>> + *
>>>>> + * Formats are organized as a set of directories one per each
>>>>> + * supported pixel format. The name of the directory is the
>>>>> + * corresponding FOURCC string label. The next level of
>>>>> + * the directory under represents supported resolutions.
>>>> So how will this work for a pixelformat like V4L2_PIX_FMT_ARGB555X?
>>>>
>>>> As mentioned before, we display such formats as '-BE', i.e. char[7].
>>> ok, then I'll change this to char[7] and put a note on big-endian:
>>>
>>>* If format represents a big-endian FOURCC code, then "-BE"
>>>* suffix must be added, case insensitive.
>> Since the fourcc is case-sensitive, I'd keep -BE case sensitive as well.
>> Your decision, though.
> hm, I'm a little bit confused here... One of the previous comments was...
>
> >> + * Formats are organized as a set of directories one per each
> >> + * supported pixel format. The name of the directory is an
> upper case
> >> + * string of the corresponding FOURCC string label. The next
> level of
> >> + * the directory under represents supported resolutions.
>
> >Lower-case characters are also use in pixelformats, so I'd just keep
> this as-is.
> >
> >In addition it is common to set bit 31 of the fourcc to 1 if the format is
> >big-endian (see v4l2_fourcc_be macro). When v4l utilities print this
> format we
> >add a -BE suffix, so V4L2_PIX_FMT_ARGB555X becomes "AR15-BE". You
> might want to
> >keep that convention.
>
> So, finally, I'll put upper case constraint here for fourcc and "-BE"?
> Did I miss something here?
Easiest is to look at videodev2.h. Let me take two examples:
#define V4L2_PIX_FMT_ARGB555 v4l2_fourcc('A', 'R', '1', '5') /* 16
ARGB-1-5-5-5 */
#define V4L2_PIX_FMT_ARGB555X v4l2_fourcc_be('A', 'R', '1', '5') /* 16
ARGB-5-5-5 BE */
The fourcc macros are defined as:
#define v4l2_fourcc(a, b, c, d)\
((__u32)(a) | ((__u32)(b) << 8) | ((__u32)(c) << 16) | ((__u32)(d) <<
24))
#define v4l2_fourcc_be(a, b, c, d) (v4l2_fourcc(a, b, c, d) | (1 << 31))
The characters can be any printable character, but currently we only use
a-z, A-Z, 0-9 and space (' ').
For big-endian formats we also set bit 31 (i.e. bit 7 of the last character)
to indicate this.
In our v4l2 utilities we use this function when we want to print a fourcc:
std::string fcc2s(__u32 val)
{
std::string s;
s += val & 0x7f;
s += (val >> 8) & 0x7f;
s += (val >> 16) & 0x7f;
s += (val >> 24) & 0x7f;
if (val & (1 << 31))
s += "-BE";
return s;
}
So the four characters (with bit 7 masked out) and the -BE suffix
if bit 7 was set for the fourth character.
So for your protocol, if you want to specify the fourcc, then I
assume dealing with characters with bit 7 set is a pain as well,
and in that case you are better off using the same scheme that we
do.
And to match the formats, applications should remember that the
string is case-sensitive, so 'abcd' != 'Abcd'.
Note that there can be spaces:
#define V4L2_PIX_FMT_Y16 v4l2_fourcc('Y', '1', '6', ' ') /* 16 Greyscale
*/
#define V4L2_PIX_FMT_Y16_BE v4l2_fourcc_be('Y', '1', '6', ' ') /* 16
Greyscale BE */
So that would be:
/local/domain/1/device/vcamera/0/formats/Y16 /1200x720/frame-rates = "15/2"
and:
/local/domain/1/device/vcamera/0/formats/Y16 -BE/1200x720/frame-rates = "15/2"
Not sure if that will fly for you.
Currently if there are spaced, then they are at the end, but I don't think
we can guarantee that for all time.
>
>>>> I assume the pixelformats you use here are based on the V4L2_PIX_FMT_
>>>> fourccs?
>>>>
>>>> Note that there is no real standard for fourcc values, so if you want to
>>>> support a Windows backend as well, then you'll need mappings from whatever
>>>> Windows uses to the V4L2 fourccs.
>>>>
>>>> The V4L2_PIX_FMT_ fourccs are entirely V4L2 specific.
>>>>
>>>> So you have to define here whose fourccs you are using.
>>> I thought that [1] defines all these val
Re: [Xen-devel] [PATCH v2 1/1] cameraif: add ABI for para-virtual camera
On 09/12/18 16:15, Oleksandr Andrushchenko wrote:
> On 09/12/2018 04:38 PM, Hans Verkuil wrote:
>> On 09/12/18 15:02, Oleksandr Andrushchenko wrote:
>>> On 09/12/2018 03:25 PM, Hans Verkuil wrote:
>>>>>>> + * formats
>>>>>>> + * Values:
>>>>>>> + *
>>>>>>> + * Formats are organized as a set of directories one per each
>>>>>>> + * supported pixel format. The name of the directory is the
>>>>>>> + * corresponding FOURCC string label. The next level of
>>>>>>> + * the directory under represents supported resolutions.
>>>>>> So how will this work for a pixelformat like V4L2_PIX_FMT_ARGB555X?
>>>>>>
>>>>>> As mentioned before, we display such formats as '-BE', i.e. char[7].
>>>>> ok, then I'll change this to char[7] and put a note on big-endian:
>>>>>
>>>>> * If format represents a big-endian FOURCC code, then "-BE"
>>>>> * suffix must be added, case insensitive.
>>>> Since the fourcc is case-sensitive, I'd keep -BE case sensitive as well.
>>>> Your decision, though.
>>> hm, I'm a little bit confused here... One of the previous comments was...
>>>
>>> >> + * Formats are organized as a set of directories one per each
>>> >> + * supported pixel format. The name of the directory is an
>>> upper case
>>> >> + * string of the corresponding FOURCC string label. The next
>>> level of
>>> >> + * the directory under represents supported
>>> resolutions.
>>>
>>> >Lower-case characters are also use in pixelformats, so I'd just keep
>>> this as-is.
>>> >
>>> >In addition it is common to set bit 31 of the fourcc to 1 if the format
>>> is
>>> >big-endian (see v4l2_fourcc_be macro). When v4l utilities print this
>>> format we
>>> >add a -BE suffix, so V4L2_PIX_FMT_ARGB555X becomes "AR15-BE". You
>>> might want to
>>> >keep that convention.
>>>
>>> So, finally, I'll put upper case constraint here for fourcc and "-BE"?
>>> Did I miss something here?
>> Easiest is to look at videodev2.h. Let me take two examples:
>>
>> #define V4L2_PIX_FMT_ARGB555 v4l2_fourcc('A', 'R', '1', '5') /* 16
>> ARGB-1-5-5-5 */
>> #define V4L2_PIX_FMT_ARGB555X v4l2_fourcc_be('A', 'R', '1', '5') /* 16
>> ARGB-5-5-5 BE */
>>
>> The fourcc macros are defined as:
>>
>> #define v4l2_fourcc(a, b, c, d)\
>> ((__u32)(a) | ((__u32)(b) << 8) | ((__u32)(c) << 16) | ((__u32)(d)
>> << 24))
>> #define v4l2_fourcc_be(a, b, c, d) (v4l2_fourcc(a, b, c, d) | (1 << 31))
>>
>> The characters can be any printable character, but currently we only use
>> a-z, A-Z, 0-9 and space (' ').
>>
>> For big-endian formats we also set bit 31 (i.e. bit 7 of the last character)
>> to indicate this.
>>
>> In our v4l2 utilities we use this function when we want to print a fourcc:
>>
>> std::string fcc2s(__u32 val)
>> {
>> std::string s;
>>
>> s += val & 0x7f;
>> s += (val >> 8) & 0x7f;
>> s += (val >> 16) & 0x7f;
>> s += (val >> 24) & 0x7f;
>> if (val & (1 << 31))
>> s += "-BE";
>> return s;
>> }
>>
>> So the four characters (with bit 7 masked out) and the -BE suffix
>> if bit 7 was set for the fourth character.
>>
>> So for your protocol, if you want to specify the fourcc, then I
>> assume dealing with characters with bit 7 set is a pain as well,
>> and in that case you are better off using the same scheme that we
>> do.
>>
>> And to match the formats, applications should remember that the
>> string is case-sensitive, so 'abcd' != 'Abcd'.
> Ah, got it, so I'll just make sure that "-BE" part
> is case sensitive, e.g. remove "case insensitive":
>
> * formats
> * Values:
> *
> * Formats are organized as a set of directories one per each
> * supported pixel format. The name of the directory is the
> * corresponding FOURCC string label. The next l
Re: [Xen-devel] [PATCH v4 1/1] cameraif: add ABI for para-virtual camera
On 2/5/19 9:48 AM, Oleksandr Andrushchenko wrote: > On 1/23/19 10:14 AM, Oleksandr Andrushchenko wrote: >> Any comments from Xen community? >> Konrad? > While I am still looking forward to any comments from Xen community... >> >> On 1/15/19 4:44 PM, Hans Verkuil wrote: >>> Hi Oleksandr, >>> >>> Just two remaining comments: >>> >>> On 1/15/19 10:38 AM, Oleksandr Andrushchenko wrote: >>>> From: Oleksandr Andrushchenko >>>> >>>> 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 >>>> --- >>>> xen/include/public/io/cameraif.h | 1364 ++ >>>> 1 file changed, 1364 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 ..246eb2457f40 >>>> --- /dev/null >>>> +++ b/xen/include/public/io/cameraif.h >>>> @@ -0,0 +1,1364 @@ >>> >>> >>>> +/* >>>> + >>>> ** >>>> + * EVENT CODES >>>> + >>>> ** >>>> + */ >>>> +#define XENCAMERA_EVT_FRAME_AVAIL 0x00 >>>> +#define XENCAMERA_EVT_CTRL_CHANGE 0x01 >>>> + >>>> +/* Resolution has changed. */ >>>> +#define XENCAMERA_EVT_CFG_FLG_RESOL (1 << 0) >>> I think this flag is a left-over from v2 and should be removed. >>> >>> >>> >>>> + * 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. This is >>>> + * limited to the value configured in XenStore.max-buffers. >>>> + * Passing zero num_bufs in this request (after streaming has stopped >>>> + * and all buffers destroyed) unblocks camera configuration changes. >>> I think the phrase 'unblocks camera configuration changes' is confusing. >>> >>> In v3 this sentence came after the third note below, and so it made sense >>> in that context, but now the order has been reversed and it became hard to >>> understand. >>> >>> I'm not sure what the best approach is to fix thi
Re: [Xen-devel] [PATCH v4 1/1] cameraif: add ABI for para-virtual camera
On 2/5/19 11:44 AM, Oleksandr Andrushchenko wrote: > On 2/5/19 11:34 AM, Hans Verkuil wrote: >> On 2/5/19 9:48 AM, Oleksandr Andrushchenko wrote: >>> On 1/23/19 10:14 AM, Oleksandr Andrushchenko wrote: >>>> Any comments from Xen community? >>>> Konrad? >>> While I am still looking forward to any comments from Xen community... >>>> On 1/15/19 4:44 PM, Hans Verkuil wrote: >>>>> Hi Oleksandr, >>>>> >>>>> Just two remaining comments: >>>>> >>>>> On 1/15/19 10:38 AM, Oleksandr Andrushchenko wrote: >>>>>> From: Oleksandr Andrushchenko >>>>>> >>>>>> 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 >>>>>> --- >>>>>> xen/include/public/io/cameraif.h | 1364 >>>>>> ++ >>>>>> 1 file changed, 1364 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 ..246eb2457f40 >>>>>> --- /dev/null >>>>>> +++ b/xen/include/public/io/cameraif.h >>>>>> @@ -0,0 +1,1364 @@ >>>>> >>>>> >>>>>> +/* >>>>>> + >>>>>> ** >>>>>> + * EVENT CODES >>>>>> + >>>>>> ** >>>>>> + */ >>>>>> +#define XENCAMERA_EVT_FRAME_AVAIL 0x00 >>>>>> +#define XENCAMERA_EVT_CTRL_CHANGE 0x01 >>>>>> + >>>>>> +/* Resolution has changed. */ >>>>>> +#define XENCAMERA_EVT_CFG_FLG_RESOL (1 << 0) >>>>> I think this flag is a left-over from v2 and should be removed. >>>>> >>>>> >>>>> >>>>>> + * 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 >>>>>&g
Re: [Xen-devel] [PATCH v4 1/1] cameraif: add ABI for para-virtual camera
On 2/5/19 12:44 PM, Oleksandr Andrushchenko wrote: > On 2/5/19 12:53 PM, Hans Verkuil wrote: >> On 2/5/19 11:44 AM, Oleksandr Andrushchenko wrote: >>> On 2/5/19 11:34 AM, Hans Verkuil wrote: >>>> On 2/5/19 9:48 AM, Oleksandr Andrushchenko wrote: >>>>> On 1/23/19 10:14 AM, Oleksandr Andrushchenko wrote: >>>>>> Any comments from Xen community? >>>>>> Konrad? >>>>> While I am still looking forward to any comments from Xen community... >>>>>> On 1/15/19 4:44 PM, Hans Verkuil wrote: >>>>>>> Hi Oleksandr, >>>>>>> >>>>>>> Just two remaining comments: >>>>>>> >>>>>>> On 1/15/19 10:38 AM, Oleksandr Andrushchenko wrote: >>>>>>>> From: Oleksandr Andrushchenko >>>>>>>> >>>>>>>> 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 >>>>>>>> >>>>>>>> --- >>>>>>>> xen/include/public/io/cameraif.h | 1364 >>>>>>>> ++ >>>>>>>> 1 file changed, 1364 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 ..246eb2457f40 >>>>>>>> --- /dev/null >>>>>>>> +++ b/xen/include/public/io/cameraif.h >>>>>>>> @@ -0,0 +1,1364 @@ >>>>>>> >>>>>>> >>>>>>>> +/* >>>>>>>> + >>>>>>>> ** >>>>>>>> + * EVENT CODES >>>>>>>> + >>>>>>>> ** >>>>>>>> + */ >>>>>>>> +#define XENCAMERA_EVT_FRAME_AVAIL 0x00 >>>>>>>> +#define XENCAMERA_EVT_CTRL_CHANGE 0x01 >>>>>>>> + >>>>>>>> +/* Resolution has changed. */ >>>>>>>> +#define XENCAMERA_EVT_CFG_FLG_RESOL (1 << 0) >>>>>>> I think this flag is a left-over from v2 and should be removed. >>>>>>> >>>>>>> >>>>>>> >>>>>>>> + * Request number of buffers to be used: >>>>>>>> + * 0 1 2 3 >>>>>>>> octet >>>>>>>> + * >>>>>>>> +++++ >>>>>>>> + * | id | _OP_BUF_REQUEST| reserved >>>>>>>> | 4 >>>>>>>> + * >>>>>>>> +++++ >>>>>>>> + * | reserved >>>>>>>> | 8 >>>>>>>> + * >>>>>>>> ++
Re: [Xen-devel] [PATCH v4 1/1] cameraif: add ABI for para-virtual camera
On 2/5/19 1:30 PM, Oleksandr Andrushchenko wrote: >> Sorry for paying so much attention to this, but I think it is important that >> this is documented precisely. > Thank you for helping with this - your comments are really > important and make the description precise. Ok, so finally: > > * 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. > * > * If num_bufs is 0 and streaming has not started yet, then the backend may > * free all previously allocated buffers (if any) or do nothing. I would rephrase this: * If num_bufs is 0 and streaming has not started yet, then the backend will * free all previously allocated buffers (if any). The previous text suggested that the backend might choose not to free the allocated buffers, but that's not the case. > * 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. With that small change the text looks good to me. Regards, Hans ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 11/41] media/v4l2-core/mm: convert put_page() to put_user_page*()
On 8/7/19 3:33 AM, john.hubb...@gmail.com wrote:
> From: John Hubbard
>
> For pages that were retained via get_user_pages*(), release those pages
> via the new put_user_page*() routines, instead of via put_page() or
> release_pages().
>
> This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
> ("mm: introduce put_user_page*(), placeholder versions").
>
> Cc: Mauro Carvalho Chehab
> Cc: Kees Cook
> Cc: Hans Verkuil
> Cc: Sakari Ailus
> Cc: Jan Kara
> Cc: Robin Murphy
> Cc: Souptick Joarder
> Cc: Dan Williams
> Cc: linux-me...@vger.kernel.org
> Signed-off-by: John Hubbard
Acked-by: Hans Verkuil
> ---
> drivers/media/v4l2-core/videobuf-dma-sg.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c
> b/drivers/media/v4l2-core/videobuf-dma-sg.c
> index 66a6c6c236a7..d6eeb437ec19 100644
> --- a/drivers/media/v4l2-core/videobuf-dma-sg.c
> +++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
> @@ -349,8 +349,7 @@ int videobuf_dma_free(struct videobuf_dmabuf *dma)
> BUG_ON(dma->sglen);
>
> if (dma->pages) {
> - for (i = 0; i < dma->nr_pages; i++)
> - put_page(dma->pages[i]);
> + put_user_pages(dma->pages, dma->nr_pages);
> kfree(dma->pages);
> dma->pages = NULL;
> }
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 10/41] media/ivtv: convert put_page() to put_user_page*()
On 8/7/19 3:33 AM, john.hubb...@gmail.com wrote:
> From: John Hubbard
>
> For pages that were retained via get_user_pages*(), release those pages
> via the new put_user_page*() routines, instead of via put_page() or
> release_pages().
>
> This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
> ("mm: introduce put_user_page*(), placeholder versions").
>
> Cc: Andy Walls
> Cc: Mauro Carvalho Chehab
> Cc: linux-me...@vger.kernel.org
> Signed-off-by: John Hubbard
Acked-by: Hans Verkuil
Regards,
Hans
> ---
> drivers/media/pci/ivtv/ivtv-udma.c | 14 --
> drivers/media/pci/ivtv/ivtv-yuv.c | 11 +++
> 2 files changed, 7 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/media/pci/ivtv/ivtv-udma.c
> b/drivers/media/pci/ivtv/ivtv-udma.c
> index 5f8883031c9c..7c7f33c2412b 100644
> --- a/drivers/media/pci/ivtv/ivtv-udma.c
> +++ b/drivers/media/pci/ivtv/ivtv-udma.c
> @@ -92,7 +92,7 @@ int ivtv_udma_setup(struct ivtv *itv, unsigned long
> ivtv_dest_addr,
> {
> struct ivtv_dma_page_info user_dma;
> struct ivtv_user_dma *dma = &itv->udma;
> - int i, err;
> + int err;
>
> IVTV_DEBUG_DMA("ivtv_udma_setup, dst: 0x%08x\n", (unsigned
> int)ivtv_dest_addr);
>
> @@ -119,8 +119,7 @@ int ivtv_udma_setup(struct ivtv *itv, unsigned long
> ivtv_dest_addr,
> IVTV_DEBUG_WARN("failed to map user pages, returned %d instead
> of %d\n",
> err, user_dma.page_count);
> if (err >= 0) {
> - for (i = 0; i < err; i++)
> - put_page(dma->map[i]);
> + put_user_pages(dma->map, err);
> return -EINVAL;
> }
> return err;
> @@ -130,9 +129,7 @@ int ivtv_udma_setup(struct ivtv *itv, unsigned long
> ivtv_dest_addr,
>
> /* Fill SG List with new values */
> if (ivtv_udma_fill_sg_list(dma, &user_dma, 0) < 0) {
> - for (i = 0; i < dma->page_count; i++) {
> - put_page(dma->map[i]);
> - }
> + put_user_pages(dma->map, dma->page_count);
> dma->page_count = 0;
> return -ENOMEM;
> }
> @@ -153,7 +150,6 @@ int ivtv_udma_setup(struct ivtv *itv, unsigned long
> ivtv_dest_addr,
> void ivtv_udma_unmap(struct ivtv *itv)
> {
> struct ivtv_user_dma *dma = &itv->udma;
> - int i;
>
> IVTV_DEBUG_INFO("ivtv_unmap_user_dma\n");
>
> @@ -170,9 +166,7 @@ void ivtv_udma_unmap(struct ivtv *itv)
> ivtv_udma_sync_for_cpu(itv);
>
> /* Release User Pages */
> - for (i = 0; i < dma->page_count; i++) {
> - put_page(dma->map[i]);
> - }
> + put_user_pages(dma->map, dma->page_count);
> dma->page_count = 0;
> }
>
> diff --git a/drivers/media/pci/ivtv/ivtv-yuv.c
> b/drivers/media/pci/ivtv/ivtv-yuv.c
> index cd2fe2d444c0..2c61a11d391d 100644
> --- a/drivers/media/pci/ivtv/ivtv-yuv.c
> +++ b/drivers/media/pci/ivtv/ivtv-yuv.c
> @@ -30,7 +30,6 @@ static int ivtv_yuv_prep_user_dma(struct ivtv *itv, struct
> ivtv_user_dma *dma,
> struct yuv_playback_info *yi = &itv->yuv_info;
> u8 frame = yi->draw_frame;
> struct yuv_frame_info *f = &yi->new_frame_info[frame];
> - int i;
> int y_pages, uv_pages;
> unsigned long y_buffer_offset, uv_buffer_offset;
> int y_decode_height, uv_decode_height, y_size;
> @@ -81,8 +80,7 @@ static int ivtv_yuv_prep_user_dma(struct ivtv *itv, struct
> ivtv_user_dma *dma,
>uv_pages, uv_dma.page_count);
>
> if (uv_pages >= 0) {
> - for (i = 0; i < uv_pages; i++)
> - put_page(dma->map[y_pages + i]);
> + put_user_pages(&dma->map[y_pages], uv_pages);
> rc = -EFAULT;
> } else {
> rc = uv_pages;
> @@ -93,8 +91,7 @@ static int ivtv_yuv_prep_user_dma(struct ivtv *itv, struct
> ivtv_user_dma *dma,
>y_pages, y_dma.page_count);
> }
> if (y_pages >= 0) {
> - for (i = 0; i < y_pages; i++)
> - put_page(dma->map[i]);
> + put_user_pages(dma->map, y_pages);
> /*
>* Inherit the -EFAULT from rc's
>
Re: [Xen-devel] [PATCH v5 1/1] cameraif: add ABI for para-virtual camera
Hi Oleksandr, Just one comment: On 3/12/19 9:20 AM, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko > > 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 > --- > 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 ..1ae4c51ea758 > --- /dev/null > +++ b/xen/include/public/io/cameraif.h > @@ -0,0 +1,1370 @@ > +/* > + * Request camera buffer's layout: > + * 01 2 3octet > + * +++++ > + * | id| _BUF_GET_LAYOUT| reserved | 4 > + * +++++ > + * | reserved | 8 > + * +++++ > + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| > + * +++++ > + * | reserved | 64 > + * +++++ > + * > + * See response format for this request. > + * > + * > + * Request number of buffers to be used: > + * 01 2 3octet > + * +++++ > + * | 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.ma
Re: [Xen-devel] [PATCH v5 1/1] cameraif: add ABI for para-virtual camera
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 >>> >>> 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 >>> --- >>> 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 ..1ae4c51ea758 >>> --- /dev/null >>> +++ b/xen/include/public/io/cameraif.h >>> @@ -0,0 +1,1370 @@ >> >> >>> +/* >>> + * Request camera buffer's layout: >>> + * 01 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: >>> + * 01 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 streamin
Re: [Xen-devel] [PATCH v5 1/1] cameraif: add ABI for para-virtual camera
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 >>>>> >>>>> 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 >>>>> --- >>>>>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 ..1ae4c51ea758 >>>>> --- /dev/null >>>>> +++ b/xen/include/public/io/cameraif.h >>>>> @@ -0,0 +1,1370 @@ >>>> >>>> >>>>> +/* >>>>> + * Request camera buffer's layout: >>>>> + * 01 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: >>>>> + * 01 2 3 >>>>> octet >>>>> + * +++++ >>>>> + * | id| _OP_BUF_REQUEST| reserved | >>>>> 4 >>>>> + * +++++ >>>>> + * | reserved | >>>>> 8 >>>>> + * +++++ >>>>> + * |num_bufs| reserved | >>>>> 12 >>>>> + * +++++ >>>>> + * | reserved | >>>>> 16 >>>>> + * +++++ >>>>> + * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/| >>>>> + * +++++ >>>>> + * | reserved | >>>>> 64 >>>>> + * +++++ >>>>
Re: [Xen-devel] [PATCH v6 1/1] cameraif: add ABI for para-virtual camera
On 3/22/19 8:37 AM, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko > > 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 Looks good! Reviewed-by: Hans Verkuil Thank you for all your work on this. Regards, Hans > --- > xen/include/public/io/cameraif.h | 1374 ++ > 1 file changed, 1374 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 ..acbcbf3bd411 > --- /dev/null > +++ b/xen/include/public/io/cameraif.h > @@ -0,0 +1,1374 @@ > +/** > + * cameraif.h > + * > + * Unified camera device I/O interface for Xen guest OSes. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > copy > + * of this software and associated documentation files (the "Software"), to > + * deal in the Software without restriction, including without limitation the > + * rights to use, copy, modify, merge, publish, distribute, sublicense, > and/or > + * sell copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > THE > + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > + * DEALINGS IN THE SOFTWARE. > + * > + * Copyright (C) 2018-2019 EPAM Systems Inc. > + * > + * Author: Oleksandr Andrushchenko > + */ > + > +#ifndef __XEN_PUBLIC_IO_CAMERAIF_H__ > +#define __XEN_PUBLIC_IO_CAMERAIF_H__ > + > +#include "ring.h" > +#include "../grant_table.h" > + > +/* > + > ** > + * Protocol version > + > ** > + */ > +#define XENCAMERA_PROTOCOL_VERSION "1" > + > +/* > + > ** > + * Feature and Parameter Negotiation > + > ** > + * > + * Front->back notifications: when enqueuing a new request, sending a > + * notification can be made conditional on xencamera_req (i.e., the generic > + * hold-off mechanism provided by the ring macros). Backends must set > + * xencamera_req appropriately (e.g., using RING_FINAL_CHECK_FOR_REQUESTS()). > + * > + * Back->front notifications: when enqueuing a new response, sending a > + * notification can be made conditional on xencamera_resp (i.e., the generic > + * hold-off mechanism provided by the ring macros). Frontends must set > + * xencamera_resp appropriately (e.g., using > RING_FINAL_CHECK_FOR_RESPONSES()). > + * > + * The two halves of a para-virtual camera driver utilize nodes within > + * XenStore to communicate capabilities and to negotiate operating > parameters. > + * This section enumerates these nodes which reside in the respective front > and > + * backend portions of XenStore, following the XenBus convention. > + * > + * All data in XenStore is stored as strings. Nodes specifying numeric > + * values are encoded in decimal. Integer value ranges listed below are > + * expressed as fixed sized integer types capable of storing the conversion > + * of a properly formatted node string, without loss of information. > + * > + >
