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

[Michael Niedermayer]

Hi

On Thu, Oct 26, 2006 at 10:33:39AM +0900, Alexey Sidelnikov wrote:
> Hello!
> 
> As far as I understand, if you want to "modify" somehow the picture 
> after it was decoded but before it is fed to the encoder, you should use 
> a temporary buffer, is that correct? 

this depends on a few things, a AVFrame for which release_buffer() has been
called or which has reference=0 can be changed by the user


> The reason is (it's my guess only) 
> is that  AVFrame picture from output_packet() function points to the 
> buffer somewhere inside decoder, and you are not allowed to change it. 
> Is that also correct?
> 
> If so, then I'd say that current implementation of 
> pre_process_video_frame() function is wrong. If both deinterlace and 
> vhook are used, and deinterlacing failed (basically that means picture 
> is not interlaced, and nothing bad is here I'd say), then vhook will use 

its not possible to detect if a picture is interlaced (its possible to
guess yes but thats about it, theres no 100% working method) so theres no way
to return an error for not interlaced images, any error of deinterlacing
should be equivalent to assert(0)/exit()/abort() in ffmpeg.c


> original picture and will corrupt the video file. Also it is hard to 
> figure out how to add extra preprocessing. I changed 
> pre_process_video_frame() function to address both issues.
> 
> 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


> 
> Also in this patch I added checks (fixed, thanks Michael) for direct 
> video stream copying from here:

this and the other unrelated things should be in seperate patches


> 
> news://news.gmane.com:119/20061025102127.GF5556 at MichaelsNB
> 
> 
> 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


> However, that can 
> be done relatively simple only: we have no more than one video stream 

not true, there can be many video streams


> (well, not a big deal to extend to several streams actually) AND video 
> stream picture size is constant during the time. Does that sound 
> reasonable? Shall I make the patch?

video frame size should be able to change between filters, libmpcodecs
support that, vhook not i think


> 
> Ohh, and one more thing - I added AVOutputStream parameter to the 
> pre_process_video_frame(). I think it can be useful somehow (for 
> example, getting pts or whatever), and it comes for free... so why not?

libmpcodecs pass pts around nicely ...


also all your patches are malformed, i cant apply them even if i want ...
you should do something to fix this, maybe disabling line wrap in your
mail user agent would be enough if not and attaching them fails then
please switch to some other mail user agent, iam not sure what OS you use
but iam sure theres someone here who can recommand a working 
mail user agent for it ...

[...]
-- 
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