linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH] Docbook/media: improve data_offset/bytesused documentation
@ 2014-07-21 13:16 Hans Verkuil
  2014-07-21 13:30 ` Nicolas Dufresne
  0 siblings, 1 reply; 3+ messages in thread
From: Hans Verkuil @ 2014-07-21 13:16 UTC (permalink / raw)
  To: Linux Media Mailing List,
	nicolas.dufresne@collabora.com >> Nicolas Dufresne,
	Marek Szyprowski, Pawel Osciak

This patch explicitly documents the relationship between bytesused and data_offset.

But looking in the kernel there isn't a single driver that actually sets data_offset.
Do such beasts exist? It's also annoying that there is no such equivalent for the
single planar API, so it is asymmetrical. What was the reason that we never did that
for single planar? Because existing applications wouldn't know what to do with it?

Regards,

	Hans

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
index 8c4ee74..e5e8325 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -870,7 +870,8 @@ should set this to 0.</entry>
 	      If the application sets this to 0 for an output stream, then
 	      <structfield>bytesused</structfield> will be set to the size of the
 	      plane (see the <structfield>length</structfield> field of this struct)
-	      by the driver.</entry>
+	      by the driver. Note that the actual image data starts at
+	      <structfield>data_offset</structfield> which may not be 0.</entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
@@ -919,6 +920,10 @@ should set this to 0.</entry>
 	    <entry>Offset in bytes to video data in the plane.
 	      Drivers must set this field when <structfield>type</structfield>
 	      refers to an input stream, applications when it refers to an output stream.
+	      Note that data_offset is included in <structfield>bytesused</structfield>.
+	      So the size of the image in the plane is
+	      <structfield>bytesused</structfield>-<structfield>data_offset</structfield> at
+	      offset <structfield>data_offset</structfield> from the start of the plane.
 	    </entry>
 	  </row>
 	  <row>

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH] Docbook/media: improve data_offset/bytesused documentation
  2014-07-21 13:16 [RFC PATCH] Docbook/media: improve data_offset/bytesused documentation Hans Verkuil
@ 2014-07-21 13:30 ` Nicolas Dufresne
  2014-07-25  7:59   ` Hans Verkuil
  0 siblings, 1 reply; 3+ messages in thread
From: Nicolas Dufresne @ 2014-07-21 13:30 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: Linux Media Mailing List, Marek Szyprowski, Pawel Osciak

Le lundi 21 juillet 2014 à 15:16 +0200, Hans Verkuil a écrit :
> +             Note that data_offset is included in <structfield>bytesused</structfield>.
> +             So the size of the image in the plane is
> +             <structfield>bytesused</structfield>-<structfield>data_offset</structfield> at
> +             offset <structfield>data_offset</structfield> from the start of the plane.

This seem like messing applications a lot. Let's say you have a well
known format, NV12, but your driver add some customer header at the
beginning. Pretty much all the application in the world would work just
fine ignoring that header, but in fact most of them will not work,
because bytesused is including the header. Considering this wasn't
documented before, I would strongly suggest to keep the bytesused as
being the size for the format know by everyone.



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH] Docbook/media: improve data_offset/bytesused documentation
  2014-07-21 13:30 ` Nicolas Dufresne
@ 2014-07-25  7:59   ` Hans Verkuil
  0 siblings, 0 replies; 3+ messages in thread
From: Hans Verkuil @ 2014-07-25  7:59 UTC (permalink / raw)
  To: Nicolas Dufresne; +Cc: Linux Media Mailing List, Marek Szyprowski, Pawel Osciak

On 07/21/2014 03:30 PM, Nicolas Dufresne wrote:
> Le lundi 21 juillet 2014 à 15:16 +0200, Hans Verkuil a écrit :
>> +             Note that data_offset is included in <structfield>bytesused</structfield>.
>> +             So the size of the image in the plane is
>> +             <structfield>bytesused</structfield>-<structfield>data_offset</structfield> at
>> +             offset <structfield>data_offset</structfield> from the start of the plane.
> 
> This seem like messing applications a lot. Let's say you have a well
> known format, NV12, but your driver add some customer header at the
> beginning. Pretty much all the application in the world would work just
> fine ignoring that header, but in fact most of them will not work,
> because bytesused is including the header. Considering this wasn't
> documented before, I would strongly suggest to keep the bytesused as
> being the size for the format know by everyone.

1) data_offset applies *only* to drivers that use the multiplanar API (i.e. have
V4L2_CAP_VIDEO_CAPTURE/OUTPUT/M2M_MPLANE set). The older single planar API is not
touched by this. So only applications that can handle the mp API should take
data_offset into account.

2) I don't see how it matters whether or not bytesused includes the data_offset.
With a non-zero data_offset and an application that doesn't understand data_offset
it will be wrong either way. In the case of V4L2 'bytesused' has always been the
amount of data that is stored in the buffer. It makes no assumptions on what that
data contains. And that's not going to change.

I've added this patch to a pull request to at least get the documentation fixed,
because the documentation was really not clear on this topic, so that's a real
bug.

Regards,

	Hans

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2014-07-25  8:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-21 13:16 [RFC PATCH] Docbook/media: improve data_offset/bytesused documentation Hans Verkuil
2014-07-21 13:30 ` Nicolas Dufresne
2014-07-25  7:59   ` Hans Verkuil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).