[MPlayer-users] A frame too many, a frame too few

[Michael Zinn]

Hihihi :)

MPlayer source: 0.90rc1

Two "half" fencepost / off-by-one errors I ran across in the code; I
say "half" because in both cases I can sort of see that the the code
which results in the error makes a degree of sense sometimes...

(There is a "who cares" at the very bottom, in case you start
 wondering why a frame off matters :P)

1) mplayer.c

   outputs one frame too many... most of the time?.  
   If video_read_frame at or about line 1560 returns a negative in_size
   (which I gather will occur if and only if there are no more video
   frames to read), then no decoding (decode_video) is done (which
   makes sense, but see (2)) (due to the 'if' at or about line 1561);
   so far so good.. BUT blit_frame is still set to the value it had at
   the end of the previous frame - 1, assuming said frame wasn't
   dropped etc. mplayer continues on its merry way, and about line
   1753 [*] video_out->flip_page is called which outputs... the
   previous frame again.  put_image, which prepares the video frame
   for output using the appropriate vo stuff, is called from inside
   decode_video.
   Note that it would not get to the video_out->flip_page if
   blit_frame were set to 0 (due to the if( blit_frame ... ) about
   line 1738 [*]).

[*] These line numbers aren't exactly right, since i shoved in some
    debugging code of my own... try maybe half a dozen lines up.  The
    description should be clear, but failing that, I'll revert my code
    to whats in the source tarball and get exact 0.90rc1 line numbers.

   The "obvious" solution would be to set blit_frame to 0 when
   video_read_frame fails.  Or perhaps better, set it to zero before
   then, and allowing the decode function to set it to 1 if appropriate.

   BUT.... see (2) :P

   Note that mencoder does NOT appear to suffer from this problem - at
   least nor from what I've observed.  I haven't looked at mencoder's
   code at all, though.


2) libmpcodecs/vd_libmpeg2.c ??

   Short and sweet version: The first call (from mplayer.c) to
   decode_video, on an mpeg2, returns 0 for blit_frame.  This is
   more-or-less directly due to the 'last' call to parse_chunk in
   vd_libmpeg2.c returning NULL.  This in turn is due to the
   interesting fiddling with what frame to return when parse_chunk has
   "finished" a frame (the first 'if' block up at the top of
   parse_chunk).  The long and the short of it is that parse_chunk
   returns the *PREVIOUS* frame decoded, not the current one.  When
   decoding the first frame, there is no previous frame to return, so
   we get NULL.
   I'm uncertain whether this is "correct" behaviour or not - possibly
   it has something to do with the way that B frames are handled? 
   (which need 'future' information..?).
   However, the net result of it is that the first time around the "do
   a video frame" bit in mplayer.c, no frame is displayed.  If
   mplayer.c ran around this "do a video frame" "loop" exactly once
   for every frame in the video stream, we'd end up one frame short
   (the last frame would never be displayed..).
   This is where (1) comes in... Was the behaviour in (1) intentional,
   and designed to cover this case?  As it happends, it is not doing
   so at the moment (the second last frame in the mpeg2 is displayed
   twice, the last frame not at all).  But perhaps that is why the
   behaviour in (1) is present.. ??  In which case something would
   have to be done to get the very last frame into the right place for
   flip_page to display it, and something else ought to be done so
   that when a codec is being used which _doesn't_ nee this extra time
   around the loop, then nothing is output.


Hmm.... that's pretty long and rambling, and possibly confusing... let
me know if I can clarify something.



As to the "who cares" bit...  after all, it *is* only a single frame,
and most likely a black one at that...  well, on the one hand we
simply have the question of "doing the right thing".  But more
substantially, it does sometimes get noticable, eg, twice on my B5 DVDs:

  - The copyright and other exciting (:P) warnings are stored in a DVD
    'title' of their own... the title consists of a grand total of 10
    frames, each of which is displayed for some amount of time; from
    the output of xine during this part, it seems that the amount of
    time is somehow related to dvdnav functionality, which ofc doesn't
    work spectacularly well in mplayer at the moment (and is indeed
    'off' by default).  If this title is played in mplayer, its about
    a half second of 10 frames whizzing past :P But if i use, eg,
    mplayer -vo jpeg, I can actually look at those frames in my own
    time, and lo and behold, the last one is not present - or more
    accurately, is a duplicate of the second last one.

  - Of course we can all live without the copyright notices, but that
    kind of feature may be used for other things; eg again on (my -
    R2) B5 DVDs, there is a title with a single (!!) frame showing
    certain credits... attempting to view this in mplayer, even
    mplayer -vo jpeg, results in nothing at all.  (The 'off by one'
    error mentioned in (1) above doesn't occur because blit_frame is
    initialised to 0, and never *becomes* 1... if it did... then
    presumably all hell would break lose when flip_page tries to do
    the displaying, if not sooner.


Aaanyway... I doubt this is hard to fix (oh well, at least (1) seems
easy to fix... I understand less about (2)) :-)

-- 
    Michael `Wumpus' Zinn <*>     | "`An eye for an eye' leaves everyone
michaelz02 at alumni.cse.unsw.edu.au |  blind."   -- Martin Luther King