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