[MPlayer-dev-eng] [PATCH] vf_bmovl2, complete and documented

[Michael Niedermayer]

Hi

On Wednesday 14 January 2004 03:38, Jason Tackaberry wrote:
> Greetings all,
>
> It has been several months since I first posted my initial bmovl2 patch
> to the list for comments.  I've received much helpful feedback and
> advice, and having made significant chances since the first post, I
> submit this final patch for consideration of inclusion into MPlayer.
>
> EXECUTIVE SUMMARY: WHAT IS VF_BMOVL2?
>
> vf_bmovl2 is a video filter which reads bitmaps from a fifo and overlays
> them onto the video.  It is, by and large, a ground-up rewrite of the
> original bmovl video filter, written by Per Wigren.  It provides
> significant enhancements over the original bmovl:
>
>       * Is a superset of original bmovl in terms of functionality
>         (however the protocol is different).
>       * Allows an arbitrary number of images to be stored.  Data
>         allocated is only as large as it needs to be, so when used as a
>         bmovl replacement, memory consumption is lower.
>       * MMX-optimized.  For non-MMX systems, bmovl2 still performs
>         better than bmovl.
>       * Can move images to different positions efficiently.  (Original
>         bmovl required retransmitting the image bitmap over the fifo.)
>       * Multiple commands can be atomized and performed between frames,
>         allowing smooth transition effects, such as OSD output sliding
>         and fading in from off-screen.
>       * Works properly even when Mplayer is paused, allowing for a
>         working menu system controlled by an external application.
>       * Fixes a number of correctness issues.  (Blending computations
>         are more accurate.)
>
> WHY NOT JUST FIX BMOVL?
>
> The original bmovl filter was designed around the concept of holding a
> single image equal in resolution to the video frame.  Most operations
> would have needed to be rewritten almost completely.  The result would
> have been nearly the same as a rewrite anyway.
>
> SCOPE OF IMPACT
>
> This patch creates libmpcodecs/vf_bmovl2.c and updates vf.c and Makefile
> to properly build and load the filter.  The man page is updated with a
> brief description of what the filter does, and a reference to
> DOCS/tech/vf_bmovl2.txt, where the full documentation is provided,
> including a brief example.
>
> In order to accommodate the requirement that bmovl2 respond while
> MPlayer is paused, a small number of changes were made to mplayer.c.  (8
> lines of code were affected: 6 added, 2 lines modified.)  Specifically,
> during the pause loop, mplayer now invokes vf_bmovl2_pause_update().
> The largest impact to this code is that the timing within the pause loop
> has been adjusted -- the usleep delay has been decreased from 20000 usec
> to 5000 usec, and the time for mp_input_get_cmd() was decreased from 20
> to 3.  This allows for bmovl2 transitions to remain smooth while
> paused.  On the systems I tested (Athlon 1400, P3 800, P4 2.6), I have
> not noticed any additional CPU usage incurred, or any other adverse
> effects.
>
> SO WHAT?
>
> Several projects, such as Freevo and RVE, have expressed interest in
> bmovl2's additional features.  Although bmovl2 has 35% more LOC than the
> original bmovl, it provides much more functionality, significantly
> faster operation on MMX-capable processors, and cleaner, more robust
> code.  There is a concern about increasing bloat in MPlayer, the result
> of feature creep, however vf_bmovl2 is a logical successor to vf_bmovl,
> and, at a suitable time, vf_bmovl can be marked as deprecated and
> eventually removed.
>
> Let me know if any additional changes are needed.
ok, iam only commenting the mmx code!
asm statements have output, input, and clobber list
anything which changes must be on the output or clobber list, or u must be 
sure the compiler is too dumb to mess with it (mmx registers for example can 
be ignored)
otherwise gcc doesnt know about it, yes i belive u that the code works fine 
ATM, but it may break any time in the future, differnt compiler / different 
compiler version, and it may be very difficult to find such bugs
the second issue i noticed is that the memory operands are unsigned char but 
are accessed as 64bit, so IMHO gcc would be allowed to copy these somewhere 
else (for example on the stack to save some registers accessing it), gcc 
doesnt do this as its way too dumb, its also not entirely clear if this would 
be alowed at all, as there is AFAIK no authorative doc which clearly defines 
this ...

[...]
-- 
Michael
level[i]= get_vlc(); i+=get_vlc();		(violates patent EP0266049)
median(mv[y-1][x], mv[y][x-1], mv[y+1][x+1]);	(violates patent #5,905,535)
buf[i]= qp - buf[i-1];				(violates patent #?)
for more examples, see http://mplayerhq.hu/~michael/patent.html
stop it, see http://petition.eurolinux.org & http://petition.ffii.org/eubsa/en