linux-kernel - Re: [PATCH v4 3/5] media: i2c: Add TDA1997x HDMI receiver driver

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]

Date:   Thu, 14 Dec 2017 15:26:57 +0100
From:   Hans Verkuil <hverkuil@...all.nl>
To:     Tim Harvey <tharvey@...eworks.com>
Cc:     linux-media <linux-media@...r.kernel.org>,
        alsa-devel@...a-project.org,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Shawn Guo <shawnguo@...nel.org>,
        Steve Longerbeam <slongerbeam@...il.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Hans Verkuil <hansverk@...co.com>,
        Mauro Carvalho Chehab <mchehab@...pensource.com>
Subject: Re: [PATCH v4 3/5] media: i2c: Add TDA1997x HDMI receiver driver

On 14/12/17 00:35, Tim Harvey wrote:
>>> Close. What is missing is a check of the AVI InfoFrame: if it has an explicit
>>> colorimetry then use that. E.g. check for HDMI_COLORIMETRY_ITU_601 or ITU_709
>>> and set the colorspace accordingly. Otherwise fall back to what you have here.
>>>
>>
>> This function currently matches adv7604/adv7842 where they don't look
>> at colorimetry (but I do see a TODO in adv748x_hdmi_fill_format to
>> look at this) so I don't have an example and may not understand.
>>
>> Do you mean:
>>
>>        format->colorspace = V4L2_COLORSPACE_SRGB;
>>        if (bt->flags & V4L2_DV_FL_IS_CE_VIDEO) {
>>                 if ((state->colorimetry == HDMI_COLORIMETRY_ITU_601) ||
>>                     (state->colorimetry == HDMI_COLORIMETRY_ITU_709))
>>                         format->colorspace = state->colorspace;
>>                 else
>>                         format->colorspace = is_sd(bt->height) ?
>>                                 V4L2_COLORSPACE_SMPTE170M :
>> V4L2_COLORSPACE_REC709;
>>         }
>>
>> Also during more testing I've found that I'm not capturing interlaced
>> properly and know I at least need:
>>
>> -        format->field = V4L2_FIELD_NONE;
>> +        format->field = (bt->interlaced) ?
>> +                V4L2_FIELD_ALTERNATE : V4L2_FIELD_NONE;
>>
>> I'm still not quite capturing interlaced yet but I think its an issue
>> of setting up the media pipeline improperly.
>>
> 
> Hans,
> 
> Did you see this question above? I'm not quite understanding what you
> want me to do for filling in colorspace and don't see any examples in
> the existing drivers that appear to look at colorimetry for this.

Yeah, I missed that question. I started answering that yesterday, but then
I realized that it would be better if I would make a helper function for
v4l2-dv-timings. The rules are complex so coding that in a single place
that everyone can use is the smart thing to do.

I hope to finish it tomorrow (too many interruptions today).

Regards,

	Hans