Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1664495

Re: [RFC PATCH v3 05/11] [media] vimc: common: Add vimc_link_validate

Show all headers | View raw

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

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

Re: [RFC PATCH v3 05/11] [media] vimc: common: Add vimc_link_validate Hans Verkuil <hverkuil@xs4all.nl> - 2017-06-12 12:00 +0200
  Re: [RFC PATCH v3 05/11] [media] vimc: common: Add vimc_link_validate Helen Koike <helen.koike@collabora.com> - 2017-06-12 19:30 +0200
    Re: [RFC PATCH v3 05/11] [media] vimc: common: Add vimc_link_validate Hans Verkuil <hverkuil@xs4all.nl> - 2017-06-13 08:40 +0200

csiph-web