[MPlayer-dev-eng] possible bugs in vf_decimate filter

[Rich Felker]

I'm the author so I'll comment.

On Sat, Oct 14, 2006 at 07:09:32AM -0700, Trent Piepho wrote:
> The decimate filter calculates 8x8 SADs over the image.  The loop that
> calls the SAD function increments x and y by 4 each time, rather than 8.
> This means all the pixels, except for the outer four, are included in four
> SAD calculations instead of one.

This is intentional. Ideally it would increment by 1 each time, but
that would be much slower and not much more accurate. The idea is to
look for maximal change over _any_ 8x8 block, not just
aligned-to-8-pixels 8x8 blocks.

> It's not clear if this is intentional or not, the code being 100%
> comment-free.

Sorry 'bout that.

> The filter also skips the leftmost 8 columns.  Nothing is skipped on the
> top, bottom, or right side, and this behavior is not documented.

It does? This sounds like a bug, unless there was some reason that
measuring them was difficult.

> The calculation for the number of blocks that need to exceed the 'lo' value
> is wrong too.  It's using (w/16)*(h/16)*frac, but, if the code is adjusted
> to not do overlapping SADs, it should be (w/8)*(h/8)*frac.  If the code is
> supposed to do SADs that overlap by four pixels, then it would be ((w-4)/4)
> * ((h-4)/4) * frac.

The normalization is probably to prevent integer overflow. Don't
change it to /8 or /4 unless you're sure it won't overflow. Also it's
really irrelevant exactly what the normalization constant is as long
as:

1) the results are roughly invariant under rescalings of the image
   (i.e. dimensionally correct)
2) the total doesn't overflow.
3) significant loss due to quantization does not occur

> Patch attached that removes the skipping of the left 8 columns, stops doing
> overlapping SADs, and calculates the number of 'lo' blocks correctly.

Overlap is intentional and should not be changed (it could be made
optional, analogous to -vf spp levels, tho... this might be nice for
performance purposes). Other stuff can probably be changed, but check
for overflow!

Rich