linux-kernel - Re: [PATCH v2 2/2] media: v4l: xilinx: Add Xilinx UHD-SDI Rx Subsystem 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <d696e0ed-3e59-f6a6-1f8d-b1725083de84@xs4all.nl>
Date:   Tue, 2 Jun 2020 12:44:20 +0200
From:   Hans Verkuil <hverkuil@...all.nl>
To:     Vishal Sagar <vsagar@...inx.com>, Hyun Kwon <hyunk@...inx.com>,
        "laurent.pinchart@...asonboard.com" 
        <laurent.pinchart@...asonboard.com>,
        "mchehab@...nel.org" <mchehab@...nel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        Michal Simek <michals@...inx.com>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "hans.verkuil@...co.com" <hans.verkuil@...co.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Dinesh Kumar <dineshk@...inx.com>,
        Sandip Kothari <sandipk@...inx.com>,
        Joe Perches <joe@...ches.com>
Subject: Re: [PATCH v2 2/2] media: v4l: xilinx: Add Xilinx UHD-SDI Rx
 Subsystem driver

On 01/06/2020 16:59, Vishal Sagar wrote:
> Hi Hans,
> 
> Thanks for reviewing!
> 
>>> +	case V4L2_CID_XILINX_SDIRX_TS_IS_INTERLACED:
>>> +		if (!xsdirxss->vidlocked) {
>>> +			dev_err(dev, "Can't get values when video not
>> locked!\n");
>>> +			return -EINVAL;
>>> +		}
>>> +		ctrl->val = xsdirxss->ts_is_interlaced;
>>
>> This control makes no sense: the v4l2_dv_timings struct will already tell you
>> if it is an interlaced format or not. Same for v4l2_mbus_framefmt.
>>
> 
> The SDI has a concept of supporting progressive, interlaced (both as we know normally) and a progressive segmented frames(psf).
> The progressive segmented frames have their video content in progressive format but the transport stream is interlaced.
> This is distinguished using the bit 6 and 7 of Byte 2 in the 4 byte ST352 payload.
> Refer to sec 5.3 in SMPTE ST 352:2010.
> 
> This control can be used by the application to distinguish normal interlaced and progressive segmented frames.

Ah, interesting. So this relies on the receiver to reconstruct the progressive
frame by combining the top and bottom field, right?

I think this deserves a new v4l2_field value:

V4L2_FIELD_ALTERNATE_PROG

Basically this is identical to V4L2_FIELD_ALTERNATE, except that the two fields
combine to a single progressive frame.

Regards,

	Hans

PS: I'll look at your other comments separately