Re: [PATCH v5 4/5] [media] allegro: add Allegro DVT video IP core driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2019-05-10 at 10:28 +0200, Hans Verkuil wrote:
[...]
> > +static int allegro_g_selection(struct file *file, void *priv,
> > +			       struct v4l2_selection *s)
> > +{
> > +	struct v4l2_fh *fh = file->private_data;
> > +	struct allegro_channel *channel = fh_to_channel(fh);
> > +
> > +	if (!V4L2_TYPE_IS_OUTPUT(s->type))
> > +		return -EINVAL;
> > +
> > +	switch (s->target) {
> > +	case V4L2_SEL_TGT_CROP:
> > +	case V4L2_SEL_TGT_CROP_DEFAULT:
> > +	case V4L2_SEL_TGT_CROP_BOUNDS:
> > +		s->r.left = 0;
> > +		s->r.top = 0;
> > +		s->r.width = channel->width;
> > +		s->r.height = channel->height;
> 
> I don't think this is quite right. The CROP target should return the visible
> width/height (e.g. 1920x1080) whereas the other two targets should return the
> coded width/height (e.g. 1920x1088 when rounded to the macroblock alignment).

The stateful encoder spec says about CROP_DEFAULT and CROP_BOUNDS:

    V4L2_SEL_TGT_CROP_BOUNDS

         equal to the full source frame, matching the active OUTPUT
         format

    V4L2_SEL_TGT_CROP_DEFAULT

        equal to V4L2_SEL_TGT_CROP_BOUNDS

There is no mention that the "full source frame" must be equal to the
coded size. Enforcing this would unnecesarily limit the ability to
import DMA-Buffers from other sources.

Imagine for example 1080p capture and encoding - if the source provides
unpadded 1920x1080 buffers (with the required stride), importing them
into an encoder that enforces height to be a multiple ot 16 wouldn't
work, even though the hardware may be capable of only reading 1080 lines
from the source frame.

> Note: if the hardware doesn't require that the raw frame is macroblock aligned,
> then I need to think a bit more about how the selection handling should be
> done.

regards
Philipp



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux