Hi Laurent, On 09/21/2011 01:12 AM, Laurent Pinchart wrote: > Hi Sylwester, > > Thanks for the patch. > > On Monday 19 September 2011 19:07:55 Sylwester Nawrocki wrote: >> FIELD is an Even/Odd field selection signal, as specified in ITU-R BT.601 >> standard. Add corresponding flag for configuring the FIELD signal polarity. >> Also add a comment about usage of V4L2_MBUS_[HV]SYNC* flags for the >> hardware that uses [HV]REF signals. > > I like this approach better. > ... >> +/* Field selection signal for interlaced scan mode */ >> +#define V4L2_MBUS_FIELD_ACTIVE_HIGH (1 << 10) >> +#define V4L2_MBUS_FIELD_ACTIVE_LOW (1 << 11) > > What does this mean ? The FIELD signal is used to select between odd and even > fields. Does "active high" mean that the field is odd or even when the signal > has a high level ? The comment should make it explicit, or we could even > rename those two constants to FIELD_ODD_HIGH/FIELD_ODD_LOW (or > FIELD_EVEN_HIGH/FIELD_EVEN_LOW). Yes, certainly I didn't think enough about this. I silently assumed that for V4L2_MBUS_FIELD_ACTIVE_HIGH FIELD = 0 selects Field1 (odd) and FIELD = 1 selects Field2 (even). I think it would be good to construct the macro so it is possibly self-explanatory, rather than requiring often to dig in the documentation. So I would go for V4L2_MBUS_FIELD_ODD_LOW/V4L2_MBUS_FIELD_ODD_HIGH. Unless someone proposes something different/better I'll send an amended version tomorrow. Thanks, -- Sylwester Nawrocki Samsung Poland R&D Center -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
