Hi Hans,

On Sat, Dec 06, 2014 at 01:05:16PM +0100, Hans Verkuil wrote:
> On 12/06/2014 12:48 PM, Sakari Ailus wrote:
> > Hi Hans,
> > 
> > On Fri, Dec 05, 2014 at 04:10:05PM +0100, Hans Verkuil wrote:
> >> On 12/03/2014 12:14 PM, Sakari Ailus wrote:
> >>> From: Sakari Ailus <sakari.ai...@linux.intel.com>
> 
> <snip>
> 
> >> I think we need to add new helper functions that give back the real plane 
> >> size
> >> (i.e. bytesused - data_offset) and the actual plane start position (plane 
> >> start
> >> + data_offset). It will be a bit tricky though to check existing drivers.
> > 
> > I think this mostly applies to OUTPUT buffers.
> > 
> > I find the definition for multi-plane buffers a little bit odd --- why not
> > allow setting this for CAPTURE buffers as well, on hardware that supports
> > it? This makes sense, in order to use the buffers on other interfaces
> > without memory copies this may be even mandatory.
> 
> It's meant for drivers that have a header before the actual image (e.g. sensor
> metadata passed on before the image). Userspace has no control over that, so
> that's why it is set by the driver at capture time.

This depends on hardware actually. Some devices can choose the offset the
data is written to in a buffer, meaning the beginning of the buffer would
not be written to by the hardware.

The "header" is very probably metadata that should be passed to the user
space, but this is out of scope of this discussion.

> > 
> > I wonder if we should change the spec regarding this, even if no driver
> > support was added yet.
> 
> I don't think so. There is a good and clear reason for this.
> 
> > 
> >> AFAICT vivid is one driver that uses vb2_plane_size() to check if enough 
> >> space
> >> is available for the image, but that doesn't take the data_offset into 
> >> account.
> >>
> >> I suspect that similar problems occur for output drivers. And what isn't 
> >> properly
> >> defined at the moment is what should happen if an output driver doesn't 
> >> support
> >> a particular data_offset value.
> >>
> >> I think the only thing you can do in that case is to return an error when 
> >> QBUF
> >> is called.
> > 
> > I'd think so. Same for PREPARE_BUF.
> > 
> > I suppose very few drivers support this at the moment, and the ones that
> > don't would return -EINVAL on QBUF. This could reveal broken user space
> > applications. An alternative would be to silently assign a valid value to
> > the field, but I'm not sure if that's any better.
> 
> I wouldn't do that. In my opinion it is a clear error.

I agree. Anyway some people are quite pedantic about it, however broken this
user space application would be.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi     XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to