[FFmpeg-devel] [PATCH] ffv1dec: Ensure that pixel format constraints are respected

[Vittorio Giovara]

On Wed, Jul 18, 2018 at 10:37 PM, Michael Niedermayer <
michael at niedermayer.cc> wrote:

> On Wed, Jul 18, 2018 at 11:03:47AM -0400, Derek Buitenhuis wrote:
> > On Wed, Jul 18, 2018 at 10:01 AM, Vittorio Giovara
> > <vittorio.giovara at gmail.com> wrote:
> > >> this does not follow from what you write below. But more so this is
> not
> > >> how pixel formats were implemented in FFmpeg.
> > >> Where does this idea come from ?
> > >
> > > I found the following description of this pixel format pretty accurate:
> > > https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-yuv410.html
> > >
> > > I am not sure how non-mod4 sizes work, but probably codecs within
> ffmpeg
> > > take into account more alignment than necessary and make things work
> >
> > To expand on this (and other replies): The behavior in FFmpeg is very
> unexpected
> > for an API user who may wish to actually use the returned yuv410p data
> with an
> > application or other library that is *not* from FFmpeg, such as a
> renderer, or
> > external encoder lib, resizer, etc. Everything else on the planet
> > assumes that if
> > you have a buffer of a specific chroma subsampling type, you actually
> > have enough
> > data for it to be valid, with width/height that make it so. It's very
> > surprising when
> > you get back a set of buffers/width/height that don't make sense for a
> > given pixel
> > format, and is little to no documentation about why/how.
>
> I think i see what you mean.
> But iam not sure i understand how the proposed changes would be the ideal
> solution.
>

The change would guarantee that the received buffer respects the pixel
format constraints, and informs the users that there are some lines and
columns that ought to be cropped away in order to display the desired (or
valid) output.


> For example:
> lets assume we have a 3x3 image of 420 or 410 (be that from a jpeg or
> whatever)
>
> If we want to use this with software that that does support odd resolutions
> then it should just work. Theres no 4th column or row in the logic image
> that
> could be used.
>
> OTOH if we want to use this with softwate that does not support odd
> resolutions
> then its not going to work with a 3x3 (as odd) or 2x2 (looses a column and
> row)
> or a 4x4 (which has a column and a row that is uninitialized or black)
>

The problem with this reasoning is that things "happen" to work only
because ffmpeg decoders always over-allocate the output buffers and make
sure that there is addressable data. So as long as you stay "within" the
ffmpeg APIs, everything is perfect, however when you use such data in third
party libraries you risk running in these issues.


> what i mean is that the API by which one exports the width and height
> doesnt
> really affect this. To get from a 3x3 to something that actually works with
> external code which only supports even resolutions, something somewhere has
> to make it even and either scale, crop or fill in.
>

Well, IMO the API is low level enough that it would make sense to always
return the coded sizes and let the user crop to the desired resolution.
Yes several filters/encoders and user apps would need some tweaks, but
since this is the n-th case in which cropping produces broken results maybe
it's worth it.


> More specifically, saying that this 3x3 is a 4x4 image with crop is not
> really
> true as theres not neccesarily any data in the last column and row. And a
> filter or encoder using that anyway could produce bad output
>

There is always data in the last column and row, or it would have been
impossible to encode the buffer.
Whether it contains visible data or garbage is another story.

> I don't think "logic guarantees the buffer is mod4/aligned" is a
> > reasonable thing
> > to tell an API user in lieu of documentation for unexpected behavior.
>
> just posted a small patch to document this
>

I added some padding/cropping code to my program, so that's enough for now,
but i think a more thorough solution should be found.
-- 
Vittorio