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

2018-09-11 Thread Oleksandr Andrushchenko

On 09/11/2018 10:52 AM, Hans Verkuil wrote:

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 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.

Ah, great, then I'll define colorspace, xfer_func, quantization
and display aspect ratio as part of virtual camera device configuration
(as vcamera represents a single source) and ycbcr_enc as a part
of pixel format configuration (one ycbcr_enc per each
pixel format)

Does this sound ok?

Uh, no :-)

ycbcr_enc is not tied to specific pixel formats. The Y'CbCr encoding tells
you how the Y'CbCr values were derived from the R'G'B' values. So this only
makes sense if you are in fact receiving Y'CbCr pixels, otherwise you just
ignore it.

It's up to you what value to assign to ycbcr_enc in that case: V4L2 doesn't
have any hard requirements for that AFAIK, although it will most likely be
set to 0 (V4L2_YCBCR_ENC_DEFAULT).

Thank you for the explanation

Regards,

Hans

Thank you,
Oleksandr

___
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

2018-09-11 Thread Hans Verkuil
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 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.
> Ah, great, then I'll define colorspace, xfer_func, quantization
> and display aspect ratio as part of virtual camera device configuration
> (as vcamera represents a single source) and ycbcr_enc as a part
> of pixel format configuration (one ycbcr_enc per each
> pixel format)
> 
> Does this sound ok?

Uh, no :-)

ycbcr_enc is not tied to specific pixel formats. The Y'CbCr encoding 

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

2018-09-11 Thread Oleksandr Andrushchenko

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 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.

Ah, great, then I'll define colorspace, xfer_func, quantization
and display aspect ratio as part of virtual camera device configuration
(as vcamera represents a single source) and ycbcr_enc as a part
of pixel format configuration (one ycbcr_enc per each
pixel format)

Does this sound ok?


Regards,

Hans

Thank you,
Oleksandr

___
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

2018-09-11 Thread Hans Verkuil
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

2018-09-11 Thread Oleksandr Andrushchenko

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"



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
should map this to V4L2_COLORSPACE_SMPTE170M)
V4L2_COLORSPACE_JPEG (historical V4L2 artifact, see here how to map:
 
https://hverkuil.home.xs4all.nl/spec/uapi/v4l/colorspaces-details.html#col-jpeg)

V4L2_COLORSPACE_SMPTE240M (rarely used, map to V4L2_COLORSPACE_SMPTE170M if 
seen in backend)

V4L2_XFER_FUNC_SMPTE240M (rarely used, map to V4L2_XFER_FUNC_709)

V4L2_YCBCR_ENC_SMPTE240M (rarely used, map to V4L2_YCBCR_ENC_709)

While V4L2 allows 0 (DEFAULT) values for xfer_func, ycbcr_enc and quantization, 
and
provides macros to map default values to the actual values (for legacy reasons),
the Xen backend should always fill this in explicitly, using those same mapping
macros (see e.g. V4L2_MAP_XFER_FUNC_DEFAULT).

The V4L2 spec has extensive information on colorspaces (sections 2.14-2.17).


The vivid driver can actually reproduce all combinations, so that's a good 
driver
to test this with.

You mean I can use it on backend side instead of real HW camera and
test all the configurations possible/those of interest?

Right.

Regards,

Hans

Thank 

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

2018-09-10 Thread Oleksandr Andrushchenko

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;

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
should map this to V4L2_COLORSPACE_SMPTE170M)
V4L2_COLORSPACE_JPEG (historical V4L2 artifact, see here how to map:
 
https://hverkuil.home.xs4all.nl/spec/uapi/v4l/colorspaces-details.html#col-jpeg)

V4L2_COLORSPACE_SMPTE240M (rarely used, map to V4L2_COLORSPACE_SMPTE170M if 
seen in backend)

V4L2_XFER_FUNC_SMPTE240M (rarely used, map to V4L2_XFER_FUNC_709)

V4L2_YCBCR_ENC_SMPTE240M (rarely used, map to V4L2_YCBCR_ENC_709)

While V4L2 allows 0 (DEFAULT) values for xfer_func, ycbcr_enc and quantization, 
and
provides macros to map default values to the actual values (for legacy reasons),
the Xen backend should always fill this in explicitly, using those same mapping
macros (see e.g. V4L2_MAP_XFER_FUNC_DEFAULT).

The V4L2 spec has extensive information on colorspaces (sections 2.14-2.17).


Thank you for such a detailed explanation!
I'll define the constants as agreed above.


The vivid driver can actually reproduce all combinations, so that's a good 
driver
to test this with.

You mean I can use it on backend side instead of real HW camera and
test all the configurations possible/those of interest?

Right.

Regards,

Hans

It seems that the number of changes discussed are begging
for the v2 of the protocol to be published ;)

Thank you,
Oleksandr

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org

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

2018-09-10 Thread Hans Verkuil
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
should map this to V4L2_COLORSPACE_SMPTE170M)
V4L2_COLORSPACE_JPEG (historical V4L2 artifact, see here how to map:
 
https://hverkuil.home.xs4all.nl/spec/uapi/v4l/colorspaces-details.html#col-jpeg)

V4L2_COLORSPACE_SMPTE240M (rarely used, map to V4L2_COLORSPACE_SMPTE170M if 
seen in backend)

V4L2_XFER_FUNC_SMPTE240M (rarely used, map to V4L2_XFER_FUNC_709)

V4L2_YCBCR_ENC_SMPTE240M (rarely used, map to V4L2_YCBCR_ENC_709)

While V4L2 allows 0 (DEFAULT) values for xfer_func, ycbcr_enc and quantization, 
and
provides macros to map default values to the actual values (for legacy reasons),
the Xen backend should always fill this in explicitly, using those same mapping
macros (see e.g. V4L2_MAP_XFER_FUNC_DEFAULT).

The V4L2 spec has extensive information on colorspaces (sections 2.14-2.17).

> 
>> The vivid driver can actually reproduce all combinations, so that's a good 
>> driver
>> to test this with.
> You mean I can use it on backend side instead of real HW camera and
> test all the configurations possible/those of interest?


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

2018-09-10 Thread Oleksandr Andrushchenko

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?

Do I need to define all those or there can be some subset of the
above for my simpler use-case?


The vivid driver can actually reproduce all combinations, so that's a good 
driver
to test this with.

You mean I can use it on backend side instead of real HW camera and
test all the configurations possible/those of interest?

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.

Agreed



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.

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 

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

2018-09-10 Thread Hans Verkuil
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.
 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.
> I'll probably stick to SET_CONFIG here
>>
>>> + *
>>> + * 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 

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

2018-09-10 Thread Oleksandr Andrushchenko

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?



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.


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.

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.

I'll probably stick to SET_CONFIG here



+ *
+ * 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   3octet
+ * +++++
+ * |   id|_GET_BUF_DETAILS|   reserved | 4
+ * +++++
+ * |  reserved | 8
+ * +++++
+ * |/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/|
+ * +++++
+ * |  reserved | 64
+ * 

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

2018-09-10 Thread Oleksandr Andrushchenko

On 09/10/2018 11:14 AM, Hans Verkuil wrote:

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 VM owns such a control and even which control and which
VM is decided while configuring the whole system.
So, I would like to keep it as is.

Well, I am not convinced you can avoid this: some controls are set by drivers
when something happens and so applications will subscribe to the control and
wait for changes in their values. While this is for more advanced use-cases
(certainly more than what you propose today), I would suggest that it is wise
to at least think on how this can be added in the future.

Controls and control events are a key part of V4L2.


ok, then I'll add
#define XENCAMERA_EVT_CTRL_CHANGE  0x02


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 as firmware implemented adjustment. And the backend still
controls the rest of the controls of the real HW camera you mention.

Just an 

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

2018-09-10 Thread Oleksandr Andrushchenko

On 09/10/2018 10:53 AM, Hans Verkuil wrote:

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"
+
+/*
+ **
+ *  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 

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

2018-09-10 Thread Hans Verkuil
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 VM owns such a control and even which control and which
> VM is decided while configuring the whole system.
> So, I would like to keep it as is.

Well, I am not convinced you can avoid this: some controls are set by drivers
when something happens and so applications will subscribe to the control and
wait for changes in their values. While this is for more advanced use-cases
(certainly more than what you propose today), I would suggest that it is wise
to at least think on how this can be added in the future.

Controls and control events are a key part of V4L2.

>>
>>> 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
>>> 

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

2018-09-10 Thread Hans Verkuil
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"
>>> +
>>> +/*
>>> + 
>>> **
>>> + *  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 

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

2018-09-10 Thread Oleksandr Andrushchenko

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.

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"
+
+/*
+ **
+ *  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
+ 

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

2018-09-10 Thread Oleksandr Andrushchenko

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 VM owns such a control and even which control and which
VM is decided while configuring the whole system.
So, I would like to keep it as is.



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 as firmware implemented adjustment. And the backend still
controls the rest of the controls of the real HW camera you mention.

Just an example of automotive use-case (we can imagine many more):
1. Driver Domain - owns real camera HW and runs the camera backend.
 Uses camera output for mission critical tasks, e.g. parking assistance.
2. In-Vehicle Infotainment domain - uses PV camera for infotainment
purposes,
 e.g. taking pictures while in motion.
3. Navigation domain - uses PV camera for high definition maps

Hope, this helps understanding the possible uses of the proposed
protocol, its
intention and restrictions.

Right, so in this scenario you probably do not want hotpluggable
sources in the Driver Domain. So 

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

2018-09-09 Thread Hans Verkuil
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 as firmware implemented adjustment. And the backend still
> controls the rest of the controls of the real HW camera you mention.
> 
> Just an example of automotive use-case (we can imagine many more):
> 1. Driver Domain - owns real camera HW and runs the camera backend.
> Uses camera output for mission critical tasks, e.g. parking assistance.
> 2. In-Vehicle Infotainment domain - uses PV camera for infotainment 
> purposes,
> e.g. taking pictures while in motion.
> 3. Navigation domain - uses PV camera for high definition maps
> 
> Hope, this helps understanding the possible uses of the proposed 
> protocol, its
> intention and restrictions.

Right, so in this scenario you probably do not want hotpluggable
sources in the Driver Domain. So support for fixed camera's only.

If this is indeed 

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

2018-09-09 Thread Hans Verkuil
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

2018-09-04 Thread Oleksandr Andrushchenko

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?
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 as firmware implemented adjustment. And the backend still
controls the rest of the controls of the real HW camera you mention.

Just an example of automotive use-case (we can imagine many more):
1. Driver Domain - owns real camera HW and runs the camera backend.
   Uses camera output for mission critical tasks, e.g. parking assistance.
2. In-Vehicle Infotainment domain - uses PV camera for infotainment 
purposes,

   e.g. taking pictures while in motion.
3. Navigation domain - uses PV camera for high definition maps

Hope, this helps understanding the possible uses of the proposed 
protocol, its

intention and restrictions.


Regards,

Hans

Thank you,
Oleksandr

___
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

2018-09-03 Thread Hans Verkuil
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

2018-09-03 Thread Oleksandr Andrushchenko

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



---
  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
+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;

Add explicit padding?


+};
+struct xencamera_req {
+    uint16_t id;
+    uint8_t operation;
+    uint8_t reserved[5];
+    union {
+    struct xencamera_config config;
+    struct xencamera_buf_create_req buf_create;
+    struct xencamera_buf_destroy_req buf_destroy;
+    struct xencamera_set_ctrl_req set_ctrl;

No tabs, please.


+    uint8_t reserved[56];
+    } req;
+};
+
+struct xencamera_resp {
+    uint16_t id;
+    uint8_t operation;
+    uint8_t reserved;
+    int32_t status;
+    union {
+    struct xencamera_config config;
+    struct xencamera_buf_details_resp buf_details;
+    struct xencamera_get_ctrl_details_resp ctrl_details;

Tab again.


+    uint8_t reserved1[56];
+    } resp;
+};


Juergen

Thank you,
Oleksandr



___
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

2018-08-20 Thread Oleksandr Andrushchenko

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)


---
  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
+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;

Add explicit padding?


+};
+struct xencamera_req {
+uint16_t id;
+uint8_t operation;
+uint8_t reserved[5];
+union {
+struct xencamera_config config;
+struct xencamera_buf_create_req buf_create;
+   struct xencamera_buf_destroy_req buf_destroy;
+   struct xencamera_set_ctrl_req set_ctrl;

No tabs, please.


+uint8_t reserved[56];
+} req;
+};
+
+struct xencamera_resp {
+uint16_t id;
+uint8_t operation;
+uint8_t reserved;
+int32_t status;
+union {
+struct xencamera_config config;
+struct xencamera_buf_details_resp buf_details;
+   struct xencamera_get_ctrl_details_resp ctrl_details;

Tab again.


+uint8_t reserved1[56];
+} resp;
+};


Juergen

Thank you,
Oleksandr

___
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

2018-08-14 Thread Juergen Gross
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...

> ---
>  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

> +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;

Add explicit padding?

> +};

> +struct xencamera_req {
> +uint16_t id;
> +uint8_t operation;
> +uint8_t reserved[5];
> +union {
> +struct xencamera_config config;
> +struct xencamera_buf_create_req buf_create;
> + struct xencamera_buf_destroy_req buf_destroy;
> + struct xencamera_set_ctrl_req set_ctrl;

No tabs, please.

> +uint8_t reserved[56];
> +} req;
> +};
> +
> +struct xencamera_resp {
> +uint16_t id;
> +uint8_t operation;
> +uint8_t reserved;
> +int32_t status;
> +union {
> +struct xencamera_config config;
> +struct xencamera_buf_details_resp buf_details;
> + struct xencamera_get_ctrl_details_resp ctrl_details;

Tab again.

> +uint8_t reserved1[56];
> +} resp;
> +};


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

2018-07-31 Thread Oleksandr Andrushchenko
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 
+ */
+
+#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/frontend = 
"/local/domain/1/device/vcamera/0"
+ * /local/domain/0/backend/vcamera/1/0/state = "4"
+ * /local/domain/0/backend/vcamera/1/0/versions = "1,2"
+ *
+ *- Frontend --
+ *
+ * /local/domain/1/device/vcamera/0/backend-id = "0"
+ *