linux-kernel - Re: [RFC PATCH v3 05/11] [media] vimc: common: Add vimc_link

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <73152b7a-2d10-7a4d-ce09-e28ddb742f22@xs4all.nl>
Date:   Tue, 13 Jun 2017 08:37:36 +0200
From:   Hans Verkuil <hverkuil@...all.nl>
To:     Helen Koike <helen.koike@...labora.com>,
        linux-media@...r.kernel.org,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        linux-kernel@...r.kernel.org
Cc:     jgebben@...eaurora.org, mchehab@....samsung.com,
        Sakari Ailus <sakari.ailus@....fi>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>
Subject: Re: [RFC PATCH v3 05/11] [media] vimc: common: Add vimc_link_validate

On 06/12/2017 07:20 PM, Helen Koike wrote:
> Hi Hans,
> 
> Thanks for your review, just a question below
> 
> On 2017-06-12 06:50 AM, Hans Verkuil wrote:
>> On 06/03/2017 04:58 AM, Helen Koike wrote:
>>> +    /* The width, height, code and colorspace must match. */
>>> +    if (source_fmt.format.width != sink_fmt.format.width
>>> +        || source_fmt.format.height != sink_fmt.format.height
>>> +        || source_fmt.format.code != sink_fmt.format.code
>>> +        || source_fmt.format.colorspace != sink_fmt.format.colorspace)
>>
>> Source and/or Sink may be COLORSPACE_DEFAULT. If that's the case, then
>> you should skip comparing ycbcr_enc, quantization or xfer_func. If
>> colorspace
>> is DEFAULT, then that implies that the other fields are DEFAULT as well.
>> Nothing
>> else makes sense in that case.
> 
> I thought that the colorspace couldn't be COLORSPACE_DEFAULT, in the
> documentation it is written "The default colorspace. This can be used by
> applications to let the driver fill in the colorspace.", so the
> colorspace is always set to something different from default no ?
> I thought that the COLORSPACE_DEFAULT was only used by the userspace in
> VIDIOC_{SUBDEV}_S_FMT so say "driver, use wherever colorspace you want",
> but if usespace calls VIDIOC_{SUBDEV}_G_FMT, it would return which exact
> colorspace the driver is using, no?

I don't think this rule works for the MC. For regular v4l2 devices the
bridge driver knows what colorspace it receives so it can replace the
colorspace DEFAULT with the real one.

But a e.g. scaler subdev does not actually touch on the colorspace. And
it doesn't know what colorspace it will receive or transmit.

I don't feel it makes sense to require userspace to set and propagate the
colorspace information throughout the pipeline. Allowing it to be set to
DEFAULT (i.e. 'don't care') makes sense to me.

I might change my mind later on this. The simple fact is that the spec isn't
clear what to do with MC devices. That's also where this vimc driver comes
in, so we can try this out without requiring specialized hardware.

Regards,

	Hans