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

[Oleksandr Andrushchenko <andr2000@gmail.com>]

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