[MPlayer-dev-eng] [PATCH] Multiple problems with tfields filter

[John Lindgren]

Reimar Döffinger wrote:
> Please send a separate patch per issue.

I can split it into (1) the deint() rewrite (2) the bytes vs. pixels
issue and (3) the pts calculation.  But all the issues with deint() make
it impractical to patch the current code block.

> I sure can believe it is faster, but that's certainly not hard if you replace
> a kind of threshold-and-filter algorithm by a simple linear blend.

The current code has this:

			if (((src[x-ss] < src[x]) && (src[x+ss] < src[x])) ||
				((src[x-ss] > src[x]) && (src[x+ss] > src[x]))) {
				//dest[x] = (src[x+ss] + src[x-ss])>>1;
				dest[x] = ((src[x+ss]<<1) + (src[x-ss]<<1)
					+ src[x+ss+1] + src[x-ss+1]
					+ src[x+ss-1] + src[x-ss-1])>>3;
			}
			else dest[x] = src[x];

What that code was intended to do may be a better algorithm than mine,
but it's not correct, because src[x+ss+1] and src[x+ss-1], which I
presume are intended to read from the pixels to the left and right, in
fact read from the *bytes* to the left and right, thereby mixing color
channels, as I mentioned, causing visible corruption.

Anyway, I thought tfields=1 was supposed to be a fast algorithm as
opposed to tfields=2 or tfields=4 which were supposed to be better and
slower?  I don't like draining my laptop battery any faster than
necessary.

> >       * Fix continue_buffered_image() to recalculate pts value when "i"
> >         counter changes.
> 
> Since that's an inaccurate hack anyway it shouldn't be duplicated, e.g.
> a function like
> double adjust_pts(double pts, int delay)
> or so might be ok.

The hack's already there.  All I did was apply it consistently, instead
of only in --correct-pts mode.

> > By the way, MPlayer's manual page needs updating.  Contrary to the
> > manual page, tfields *does* work in MPlayer, not only MEncoder, and does
> > not need the -fps command line switch to work correctly.
> 
> Not really, it just happens to work if your vo does vsync and the vsync
> frequency is about twice the original frame rate.
> Might be good enough for most, but calling it "work" is a bit of a stretch.

As a matter of fact, I tested it with and without vsync and it works
either way.  That was after the above pts hack; perhaps that made it
work?

Uoti Urpala wrote:
> It has worked better than that if -correct-pts is active since my commit
> that enabled video filters to add frames back in 2006 (svn r18922),
> though the filter doesn't implement exact pts calculation for the added
> frames (just adds that 0.02 seconds to last frame pts). Since he
> modified the value added to pts I think the behavior is probably exactly
> right for his particular videos. In git behavior even without
> -correct-pts is better now in the sense that you can at least frame step
> over the added frames individually, though in that case they don't have
> any individual pts generation (in svn it'll jump over both frames at
> once).

1.001 / 60 at least corresponds to NTSC, whereas 0.02 seems completely
arbitrary.  With my patch, pts is calculated correctly (at least for
NTSC content) even without -correct-pts.  (Side note: Using -correct-pts
and tfields, both before and after my patch, produces stunning screen
corruption, so I don't know what you mean by it working "better".)

Carl Eugen Hoyos wrote:
> Did you try yadif? (I only ask because it is usually far superior.)

Yes, I tried it.  I prefer tfields=1 because (1) I can't notice any
difference between the two, (2) yadif is not compatible with OpenGL
output, so I have to use Xv, which does not have vsync on my machine,
and (3) yadif raises CPU usage of mplayer by 70% over tfields=1.

John Lindgren


On Sun, 2010-01-31 at 16:34 -0500, John Lindgren wrote:
> Hi,
> 
> I recently started using the tfields=1 filter for deinterlacing when
> watching DVD's, and noticed some problems, and while trying to fix them,
> I stumbled on some more.  A couple of these are merely for the sake of
> correctness; a couple are noticeable only when the filter is applied to
> a non-planar video format, i.e. when using OpenGL output instead of Xv.
> 
>       * Fix deint() interpolating even lines instead of odd and vice
>         versa.
>       * Fix deint() mixing color channels when applied to non-planar
>         formats.
>       * Fix deint() interpolating bottom line from random data past
>         image boundary.
>       * Optimize deint() by reducing number of operations in innermost
>         loop.
>       * Fix continue_buffered_image() to pass width to deint() in bytes,
>         not pixels.
>       * Fix continue_buffered_image() to recalculate pts value when "i"
>         counter changes.
> 
> By the way, MPlayer's manual page needs updating.  Contrary to the
> manual page, tfields *does* work in MPlayer, not only MEncoder, and does
> not need the -fps command line switch to work correctly.
> 
> John Lindgren