[MPlayer-users] XviD sync-bug and its fix

[armin.gerritsen at philips.com]

Hello,

Some weeks ago I sent a mail to the mailing list about a problem when
porting mplayer to an embedded platform - an ARM 100MHz in our case. The
problem was that when playing XviD content the audio/video got terribly out
of sync, because mplayer refused to drop frames. The problem didn't occur
with MPEG1 or 2, so this was 'clearly' a bug of mplayer.

Unfortunately because of obvious reasons the userbase of mplayer on embedded
platforms is rather limited and desktop-systems have usually enough
calculation-power to display XviD nicely. So the bug was not noticed (or
reported) before.

I didn't have time to look at it earlier, so I just placed it aside. But now
I found a few hours and found and 'fixed' the problem. The problem as said,
is caused by mplayer not dropping any frames. Not even when
using -hardframedrop.

It turned out the problem is a combination of two (three) problems:

(- a problem in vd_ffmpeg.c)
- a problem in h263dec.c
- the specific content we used, which didn't contain any B-frames.

The latter however doesn't seem to be that rare I noticed. When using
VirtualDub with the December 2002 release XviD codex, the default settings
for instance produce content like this.

Well, you might guess where I'm going. The h263dec.c doesn't drop any frames
other than B-frames, so for content like this it never drops any frames and
gets out-of-sync.

If we look into the code, we see the s->hurry_up value which controls
framedroping. This value will be 0, 1 (-framedrop) or 2 (-hardframedrop). So
the code at line 615 is never called with -hardframedrop

/* skip everything if we are in a hurry>=5 */
if(s->hurry_up>=5) return get_consumed_bytes(s, buf_size);

This leaves indeed only like 613, which doesn't drop with content as
explained.
I 'fixed' this by replacing the 615-line by:

if ( (1 < s->hurry_up) && (s->pict_type != I_TYPE) )
{
 // If we drop a P-frame, we must drop all other B/P's too,
 // in order to prevent serious image-corruption ... ;-(
 //if ( s->pict_type == P_TYPE ) // We know it is P.
  must_flush_non_I_frames = 1 ;
 return get_consumed_bytes( s, buf_size ) ;
}

I say 'fixed' because dropping P-frames has the problem that this causes
image-corruption, because P-frames depend on the previous. So all following
P-frames will be referring to a dropped frame. If you don't want to accept
this (as I didn't), one should add also:

// Flush all non B/P-frames.
if ( must_flush_non_I_frames )
{
 if ( s->pict_type == I_TYPE )
  must_flush_non_I_frames = 0 ;
 else
  return get_consumed_bytes(s, buf_size) ;
}

where must_flush_non_I_frames is a static unsigned integer in my case.
This will produce nice quality pictures again, although it will drop of
course lots more than required ...
But all is better than losing sync I would say, and remember this code only
gets active with -hardframedrop.



A better, but more intrusive fix would be to decode the frame, but don't
return it so it will not be dithered and displayed, but this may not buy you
much cycles (it didn't on our embedded CPU).



The second fix, isn't required, but I found it by accident. It is in
vd_ffmpeg.c in the decode function. This function is called with a flag
parameter, which is the frame_drop param in mplayer.c. So it will be 0, 1 or
2 (no drop, output drop, decoder-drop). Problem is that this flag has no
effect. Other codecs implement something like returning NULL, when they need
to drop. vd_ffmpeg.c does nothing. Well, nothing. At line 519 it passes it
on to the actual codec via the mentioned hurry_up value.

This covers pretty most all situations where the actual decoding drops a
frame, but in the extreme case that an I-frame needs to be dropped (which
the decoder even with my fix doesn't do), it will fail. I admit this is
extreme (so that's why I did put brackets when listing the problem.), but if
you want to cover this case too, add at line 604 of vd_ffmepg.c, something
like:

if ( 3 & flags )
{
 return NULL ;
}



as some other mplayer-codecs also do.



Regards,



Armin