[FFmpeg-devel] [PATCH] snow: remove strange av_assert2

Ronald S. Bultje rsbultje at gmail.com
Wed Jul 8 13:24:13 CEST 2015


Hi,

On Tue, Jul 7, 2015 at 7:22 PM, Michael Niedermayer <michaelni at gmx.at>
wrote:

> On Tue, Jul 07, 2015 at 11:32:59PM +0200, Andreas Cadhalpun wrote:
> > On 06.07.2015 02:40, Michael Niedermayer wrote:
> > > On Sun, Jul 05, 2015 at 09:11:44PM +0200, Andreas Cadhalpun wrote:
> > >> Can you explain how elements can be too larger to fit?
> > >
> > > emulated_edge_mc() writes a block of width x height,
> >
> > More precisely that's block_w x block_h.
> >
>
> > > if stride < width this will not work as intended
> >
> > Then an assert for stride > block_w might make sense in emulated_edge_mc,
> > but not in add_yblock.
>
> ok, added


I don't think this makes sense:
- first of all, it should be >=;
- second of all, are we going to add asserts to each and every function
that takes a stride argument?
- third of all, if people want to call if with stride = 0 or negative
stride or whatever, why not? The function still does what it does, just
overwrites its own data. Not our problem.
- last of all, I want to remind you guys that we haven't done a very
thorough code analysis to see if this can actually be triggered. As an
example, look at commit 458446acfa1441d283dacf9e6e545beb083b8bb0. Before
that, vp8 files smaller than 16x16 or vp9 files smaller than 64x64 could
cause aborts. The vp9 ones I noticed due to some fate files being so small.
I doubt you'll ever find such vp8 files, but they're valid.

Let's not add silly checks to replace other silly ones.

Ronald


More information about the ffmpeg-devel mailing list