From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-qk0-f196.google.com ([209.85.220.196]:47056 "EHLO mail-qk0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030816AbeEZBOi (ORCPT ); Fri, 25 May 2018 21:14:38 -0400 Received: by mail-qk0-f196.google.com with SMTP id k86-v6so5419954qkh.13 for ; Fri, 25 May 2018 18:14:38 -0700 (PDT) Message-ID: Subject: Re: [PATCH 3/6] media: videodev2.h: Add macro V4L2_FIELD_IS_SEQUENTIAL From: Nicolas Dufresne To: Steve Longerbeam , Philipp Zabel , Krzysztof =?UTF-8?Q?Ha=C5=82asa?= , Mauro Carvalho Chehab , Greg Kroah-Hartman , Hans Verkuil Cc: linux-media@vger.kernel.org, Steve Longerbeam Date: Fri, 25 May 2018 21:14:35 -0400 In-Reply-To: References: <1527292416-26187-1-git-send-email-steve_longerbeam@mentor.com> <1527292416-26187-4-git-send-email-steve_longerbeam@mentor.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-media-owner@vger.kernel.org List-ID: Le vendredi 25 mai 2018 à 17:19 -0700, Steve Longerbeam a écrit : > > On 05/25/2018 05:10 PM, Nicolas Dufresne wrote: > > (in text this time, sorry) > > > > Le vendredi 25 mai 2018 à 16:53 -0700, Steve Longerbeam a écrit : > > > Add a macro that returns true if the given field type is > > > 'sequential', > > > that is, the data is transmitted, or exists in memory, as all top > > > field > > > lines followed by all bottom field lines, or vice-versa. > > > > > > Signed-off-by: Steve Longerbeam > > > --- > > > include/uapi/linux/videodev2.h | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/include/uapi/linux/videodev2.h > > > b/include/uapi/linux/videodev2.h > > > index 600877b..408ee96 100644 > > > --- a/include/uapi/linux/videodev2.h > > > +++ b/include/uapi/linux/videodev2.h > > > @@ -126,6 +126,10 @@ enum v4l2_field { > > > (field) == V4L2_FIELD_INTERLACED_BT ||\ > > > (field) == V4L2_FIELD_SEQ_TB ||\ > > > (field) == V4L2_FIELD_SEQ_BT) > > > +#define V4L2_FIELD_IS_SEQUENTIAL(field) \ > > > + ((field) == V4L2_FIELD_SEQ_TB ||\ > > > + (field) == V4L2_FIELD_SEQ_BT ||\ > > > + (field) == V4L2_FIELD_ALTERNATE) > > > > No, alternate has no place here, in alternate mode each buffers have > > only one field. > > Then I misunderstand what is meant by "alternate". The name implies > to me that a source sends top or bottom field alternately, or top/bottom > fields exist in memory buffers alternately, but with no information about > which field came first. In other words, "alternate" is either seq-tb or > seq-bt, > without any info about field order. > > If it is just one field in a memory buffer, why isn't it called > V4L2_FIELD_TOP_OR_BOTTOM, e.g. we don't know which? I don't see why this could be better then ALTERNATE, were buffers are only top or bottom fields alternatively. And even if there was another possible name, this is public API. V4L2_FIELD_ALTERNATE is a mode, that will only be used with v4l2_pix_format or v4l2_pix_format_mplane. I should never bet set on the v4l2_buffer.field, instead the driver indicates the parity of the field by setting V42_FIELD_TOP/BOTTOM on the v4l2_buffer returned by DQBUF. This is a very different mode of operation compared to sequential, hence why I believe it is wrong to make it part of the new helper. So far, it's the only field value that has this asymmetric usage and meaning. > > Steve >