From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans Verkuil Date: Fri, 02 Aug 2013 09:43:52 +0000 Subject: Re: [PATCH v5 2/9] Documentation: media: Clarify the VIDIOC_CREATE_BUFS format requirements Message-Id: <51FB7F58.2070101@xs4all.nl> List-Id: References: <1375405408-17134-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com> <1375405408-17134-3-git-send-email-laurent.pinchart+renesas@ideasonboard.com> In-Reply-To: <1375405408-17134-3-git-send-email-laurent.pinchart+renesas@ideasonboard.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Laurent Pinchart Cc: linux-media@vger.kernel.org, linux-sh@vger.kernel.org, Sakari Ailus , Katsuya MATSUBARA , Sylwester Nawrocki Hi Laurent, On 08/02/2013 03:03 AM, Laurent Pinchart wrote: > The VIDIOC_CREATE_BUFS ioctl takes a format argument that must contain a > valid format supported by the driver. Clarify the documentation. > > Signed-off-by: Laurent Pinchart > --- > Documentation/DocBook/media/v4l/vidioc-create-bufs.xml | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml > index cd99436..407937a 100644 > --- a/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml > +++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml > @@ -69,10 +69,11 @@ the v4l2_create_buffers structure. They set the > structure, to the respective stream or buffer type. > count must be set to the number of required buffers. > memory specifies the required I/O method. The > -format field shall typically be filled in using > -either the VIDIOC_TRY_FMT or > -VIDIOC_G_FMT ioctl(). Additionally, applications can adjust > -sizeimage fields to fit their specific needs. The > +format field must be a valid format supported by the > +driver. Applications shall typically fill it using either the > +VIDIOC_TRY_FMT or VIDIOC_G_FMT > +ioctl(). Any format that would be modified by the > +VIDIOC_TRY_FMT ioctl() will be rejected with an error. The I'm a bit unhappy with this formulation. How about: "The format must be a valid format, otherwise an error will be returned." The main reason for this is that changes made to the 'field' and 'colorspace' format fields by TRY_FMT do not affect CREATE_BUFS. Does a wrong colorspace mean that CREATE_BUFS should return an error? I'm not sure we want to do that, frankly. You also removed the bit about the sizeimage field. That should be kept, although admittedly it needs some improvement. The reason for that is that applications cannot set sizeimage when calling S_FMT: that field is set by the driver. Only through CREATE_BUFS can apps select a different (larger) sizeimage. I think it would be useful if there was a link to CREATE_BUFS was added to the 'sizeimage' description of struct v4l2_pix_format in DocBook. It is not exactly obvious that CREATE_BUFS can be used for that purpose. It's really a workaround for a limitation in the spec, of course. Regards, Hans > reserved array must be zeroed. > > When the ioctl is called with a pointer to this structure the driver > @@ -144,9 +145,9 @@ mapped I/O. > > EINVAL > > - The buffer type (type field) or the > -requested I/O method (memory) is not > -supported. > + The buffer type (type field), > +requested I/O method (memory) or format > +(format field) is not valid. > > > > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from smtp-vbr8.xs4all.nl ([194.109.24.28]:2141 "EHLO smtp-vbr8.xs4all.nl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753271Ab3HBJoL (ORCPT ); Fri, 2 Aug 2013 05:44:11 -0400 Message-ID: <51FB7F58.2070101@xs4all.nl> Date: Fri, 02 Aug 2013 11:43:52 +0200 From: Hans Verkuil MIME-Version: 1.0 To: Laurent Pinchart CC: linux-media@vger.kernel.org, linux-sh@vger.kernel.org, Sakari Ailus , Katsuya MATSUBARA , Sylwester Nawrocki Subject: Re: [PATCH v5 2/9] Documentation: media: Clarify the VIDIOC_CREATE_BUFS format requirements References: <1375405408-17134-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com> <1375405408-17134-3-git-send-email-laurent.pinchart+renesas@ideasonboard.com> In-Reply-To: <1375405408-17134-3-git-send-email-laurent.pinchart+renesas@ideasonboard.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org List-ID: Hi Laurent, On 08/02/2013 03:03 AM, Laurent Pinchart wrote: > The VIDIOC_CREATE_BUFS ioctl takes a format argument that must contain a > valid format supported by the driver. Clarify the documentation. > > Signed-off-by: Laurent Pinchart > --- > Documentation/DocBook/media/v4l/vidioc-create-bufs.xml | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml > index cd99436..407937a 100644 > --- a/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml > +++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml > @@ -69,10 +69,11 @@ the v4l2_create_buffers structure. They set the > structure, to the respective stream or buffer type. > count must be set to the number of required buffers. > memory specifies the required I/O method. The > -format field shall typically be filled in using > -either the VIDIOC_TRY_FMT or > -VIDIOC_G_FMT ioctl(). Additionally, applications can adjust > -sizeimage fields to fit their specific needs. The > +format field must be a valid format supported by the > +driver. Applications shall typically fill it using either the > +VIDIOC_TRY_FMT or VIDIOC_G_FMT > +ioctl(). Any format that would be modified by the > +VIDIOC_TRY_FMT ioctl() will be rejected with an error. The I'm a bit unhappy with this formulation. How about: "The format must be a valid format, otherwise an error will be returned." The main reason for this is that changes made to the 'field' and 'colorspace' format fields by TRY_FMT do not affect CREATE_BUFS. Does a wrong colorspace mean that CREATE_BUFS should return an error? I'm not sure we want to do that, frankly. You also removed the bit about the sizeimage field. That should be kept, although admittedly it needs some improvement. The reason for that is that applications cannot set sizeimage when calling S_FMT: that field is set by the driver. Only through CREATE_BUFS can apps select a different (larger) sizeimage. I think it would be useful if there was a link to CREATE_BUFS was added to the 'sizeimage' description of struct v4l2_pix_format in DocBook. It is not exactly obvious that CREATE_BUFS can be used for that purpose. It's really a workaround for a limitation in the spec, of course. Regards, Hans > reserved array must be zeroed. > > When the ioctl is called with a pointer to this structure the driver > @@ -144,9 +145,9 @@ mapped I/O. > > EINVAL > > - The buffer type (type field) or the > -requested I/O method (memory) is not > -supported. > + The buffer type (type field), > +requested I/O method (memory) or format > +(format field) is not valid. > > > >