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
