[Ffmpeg-devel] Re: [PATCH proposal] ffmpeg.c - pre_process_video_frame()

[Michael Niedermayer]

Hi

On Fri, Oct 27, 2006 at 01:46:23AM +0000, Alexey Sidelnikov wrote:
> Hi
> 
> > > Another conclusion I got is that pre_process_video_frame() was designed 
> > > to modify frames which will be written to the movie, right? If so, then 
> > > I think it should be called only before actual encoding the frame, i.e. 
> > > right before do_video_out() call. I changed output_packet() function 
> > > accordingly.
> > 
> > this would make all filters which use the previous frame missbehave
> > if the previous frames was not encoded
> 
> okay, ic... will change back.
> 
> > > I also have another improvement which can affect the performance I think 
> > > - why not to make the buffer for preprocessing static and malloc/free it 
> > > only once? Now it is malloced/freed every video frame. 
> > 
> > i strongly suggest that you look at mplayer/libmpcodecs and port that
> > to ffmpeg instead of spending alot of time slowly improving the current
> > half broken vhook system
> 
> Well, if you don't mind, I'd like to ask for a few minutes of your time.
> Actually I'm very impressed with the framehook, and my next plan was to port it
> to Windows (plus some other not that important changes I'd to do). So what you
> are saying now is that vhook it "bad" and "half-broken"?. does that mean it will
> be removed from ffmpeg one day? 

yes, as soon as something else better is in svn and working ...


> Can you please very briefly describe the reasons
> why it is half-broken, and what, in your opinion, should be changed to make it
> "good"? 

in no particular order ...
* the width and height cant be changed in filters (scale, crop, extracting 
  fields, merging fields, 90? rotate wont be possible, all that is possible
  and implemented in the mplayer filter system)
* filters cannot change the timestamp of the current frame, even worse
  currently they get the current "time of the day" instead of the timestamp
  from the frame in the source file 
* they cannot change the pix_fmt, so filters generally convert to their
  favorite pix_fmt do the filtering and then convert back, not only is that
  very slow, leads to more rounding errors its also totally unneeded mplayers
  filter system allows filters to change the pix_fmt and any needed
  conversaton is done by a seperate filter if the supported output formats
  cant be used be the next filter ...
* filters cannot drop frames (framerate changing filters, merge 2 frames
  like fields together, detelecine, drop similar frames, ... arent possible,
  all that is possible and working filters have been implemented for it in 
  mplayer)
* filters cannot insert new frames (again works in mplayer, partially at
  least)
* filters cannot change the data array origin and width/height, so while
  a crop filter in mplayer just changes the origin and decreses width/height
  (=no memcpy) a vhook filter would have to memcpy (assuming it could change
  the width and height to begin with)
* no direct rendering support, in mplayer filters can ask the next filter
  for an output buffer (which could be in video memory or could be a buffer
  provided by a API like SDL or xv) vhook would need a memcpy()
* no slices, codecs often decode video in horizontal slices (16pixel 
  macroblock rows are common), after one is rendered its likely in the L2
  cache, mplayer can simple pass such a slice through the filter chain
  as long as the filters support it, vhook needs a complete frame which
  likely wont fit in the L2 cache and so filtering is slower


> And sinse I have never ever looked at mplayer, could you please just
> give me the name of the module which you are suggesting to port?

libmpcodecs
vf_*.c are the filters
vd* are video decoders (not interresting)
ad* are audio decoders (not interresting)
ve*/ae* are the corresponding encoders again not interresting for us

what should be done is that 
1. libmpcodecs (the video filter related stuff)
   should be made independant from mplayer, so that other applications could
   link to it and use it
2. ffmpeg.c and ffplay.c should be changed to use libmpcodecs if its
   available (the idea here is not to have a different libmpcodecs in
   ffmpeg then in mplayer but rather do neccesary changes in mplayer svn
3. optional (if mplayer developers want, IIRC diego was strongly in favor
   of it) move the video filter stuff from libmpcodecs into ffmpeg svn and
   let mplayer use it like libswscale/libavcodec/... that way it would
   also become easier to use for other applications which use ffmpeg/lib*

and see mplayer/DOCS/tech/libmpcodecs.txt (but warning this is old and
might not match the current filter system 100%)

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is