[FFmpeg-devel] [PATCH] moves ff_set_mpeg4_time to mpegvideo_enc.c

[Aurelien Jacobs]

On Mon, 9 Jul 2007 18:07:42 +0200
Michael Niedermayer <michaelni at gmx.at> wrote:

> Hi
> 
> On Mon, Jul 09, 2007 at 03:16:10PM +0200, Aurelien Jacobs wrote:
> > On Fri, 6 Jul 2007 10:44:55 +0200
> > Michael Niedermayer <michaelni at gmx.at> wrote:
> > 
> > > Hi
> > > 
> > > On Fri, Jul 06, 2007 at 01:51:20AM +0200, Aurelien Jacobs wrote:
> > > > Hi,
> > > > 
> > > > The attached patch moves ff_set_mpeg4_time to mpegvideo_enc.c because
> > > > it is only used by this file. This allows to make the function static.
> > > > Moreover this function is used for mpeg1/2 as well as mpeg4 so it
> > > > definitely don't belong in h263.c and it really deserve a renaming
> > > > (as suggested by the FIXME comment).
> > > > 
> > > > Is it OK ?
> > > 
> > > well ...
> > > 
> > > the function should be split, some of the things are needed for all
> > > b frame encodings some are mpeg4 specific, later belong in h263.c
> > > not mpegvideo*.c unless you split mpeg4 out of h263.c ...
> > 
> > I'm not sure of what you mean exactly. The only mpeg4 specific thing
> > in this function is the call to ff_mpeg4_init_direct_mv().
> > I now moved it out of the function (see attached patch). Tell me
> > if that's what you mean.
> 
> the only thing in this function which is not mpeg4 specific is
> pb_time and pp_time and they are also almost not used outside
> mpeg4
> 
> calling the function set_mpeg_time() is TOTALLY wrong, its mpeg4
> specific, just out of convenience some non mpeg4 code also used
> these variables
> that is motion estimation and error resilience uses pb_time and pp_time
> to find the distance between frames ...

OK. But calculation of pp_time and pb_time depends on all other values
this function manipulate.
So I have no idea how this function could be split.
I would be glad if you could advise me how do you expect it to be split.

Note that ff_set_mpeg4_time() is the only hard dependency of mpegvideo_enc.c
over h263.c and I would really like to break this dependency. Also this
function is only used in mpegvideo_enc.c and nowhere else.

Aurel