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

[Hans Verkuil <hverkuil@xxxxxxxxx>]

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

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