[FFmpeg-devel] libavcodec/pthread.c:138: error: no previous prototype for 'avcodec_thread_execute2'

[Benoit Fouet]

Hi M?ns,

On Tue, 09 Mar 2010 12:47:50 +0000 M?ns Rullg?rd wrote:
> Benoit Fouet <benoit.fouet at free.fr> writes:
> 
> > On Mon, 08 Mar 2010 16:16:57 +0000 M?ns Rullg?rd wrote:
> >> Benoit Fouet <benoit.fouet at free.fr> writes:
> >>
> >> > On Mon, 08 Mar 2010 14:52:44 +0000 M?ns Rullg?rd wrote:
> >> >> Benoit Fouet <benoit.fouet at free.fr> writes:
> >> >>
> >> >> > On Mon, 08 Mar 2010 14:17:01 +0000 M?ns Rullg?rd wrote:
> >> >> >> Benoit Fouet <benoit.fouet at free.fr> writes:
> >> >> >>
> >> >> >> > On Mon, 08 Mar 2010 13:58:49 +0100 Andreas ?man wrote:
> >> >> >> >> Georgi Chorbadzhiyski wrote:
> >> >> >> >> > Around 03/08/10 12:43, Benoit Fouet scribbled:
> >> >> >> >> >> On Mon, 08 Mar 2010 12:28:37 +0200 Georgi Chorbadzhiyski wrote:
> >> >> >> >> >>> gf at gf:~/ffmpeg$ make
> >> >> >> >> >>> CC      libavcodec/pthread.o
> >> >> >> >> >>> libavcodec/pthread.c:138: error: no previous prototype for 'avcodec_thread_execute2'
> >> >> >> >> >>> make: *** [libavcodec/pthread.o] Error 1
> >> >> >> >> >>>
> >> >> >> >> >>> This fixes it but I don't think this is the correct fix. It should not give error
> >> >> >> >> >>> here.
> >> >> >> >> >> missing prototypes now results in an error.
> >> >> >> >> >
> >> >> >> >> > I've missed this. This is a good change.
> >> >> >> >>
> >> >> >> >> Fixed, and for w32thread.c as well.
> >> >> >> >
> >> >> >> > Your fix is not in line with what exists e.g. for
> >> >> >> > avcodec_thread_execute I don't know which solution we
> >> >> >> > should aim at, but now we're somewhere in between, and
> >> >> >> > IMHO, that needs to be fixed: either make
> >> >> >> > avcodec_thread_execute2 part of the API, or remove the
> >> >> >> > other ones from the API.
> >> >> >>
> >> >> >> Neither of them make sense in the public API.  There is no sensible
> >> >> >> way for an application to use them.
> >> >> >>
> >> >> >
> >> >> > this is fine by me.
> >> >> > If my understanding is correct, anyone can add his own thread
> >> >> > management, right ? Then I guess we should provide function pointer
> >> >> > prototypes, instead of providing these.
> >> >>
> >> >> We already have AVCodecContext.execute as a function pointer.
> >> >>
> >> >
> >> > let's drop what's unused in avcodec.h then.
> >> > Patch below to do that (I also made thread_func in OS/2 static, though
> >> > I can do it in a separate patch if needed, and if correct (maybe there
> >> > is some black magic things there preventing that to work correctly)),
> >> > does that look ok ?
> >>
> >> Skip the ifdeffery, just drop the prototypes.  These being in the
> >> public header file is an error, and nobody should be using them.
> >>
> >
> > here is the patch without the ifdeffery, I'll apply whichever is picked up...
> 
> Something is appending ridiculous amounts of whitespace to the end of
> each line in your patch.  (My nice mail reader has a function to strip
> it.)
> 

yep, I noticed that, but did not bother to fix it :)

> > Index: libavcodec/w32thread.c
> > ===================================================================
> > --- libavcodec/w32thread.c      (revision 22379)
> > +++ libavcodec/w32thread.c      (working copy)
> > @@ -93,7 +93,7 @@ void avcodec_thread_free(AVCodecContext
> >      av_freep(&s->thread_opaque);
> >  }
> >
> > -int avcodec_thread_execute(AVCodecContext *s, int (*func)(AVCodecContext *c2, void *arg2),void *arg, int *ret, int count, int size){
> > +static int avcodec_thread_execute(AVCodecContext *s, int (*func)(AVCodecContext *c2, void *arg2),void *arg, int *ret, int count, int size){
> >      ThreadContext *c= s->thread_opaque;
> >      int i;
> >      int jobnr = 0;
> > Index: libavcodec/options.c
> > ===================================================================
> > --- libavcodec/options.c        (revision 22379)
> > +++ libavcodec/options.c        (working copy)
> > @@ -26,6 +26,7 @@
> >
> >  #include "avcodec.h"
> >  #include "opt.h"
> > +#include "internal.h"
> >  #include <float.h>              /* FLT_MIN, FLT_MAX */
> >
> >  static const char* context_to_name(void* ptr) {
> 
> Why?
> 

because of the needed definition of avcodec_default_execute and
avcodec_default_execute2 which are now there.

> > Index: libavcodec/internal.h
> > ===================================================================
> > --- libavcodec/internal.h       (revision 22379)
> > +++ libavcodec/internal.h       (working copy)
> > @@ -48,4 +48,7 @@ AVHWAccel *ff_find_hwaccel(enum CodecID
> >   */
> >  int ff_match_2uint16(const uint16_t (*tab)[2], int size, int a, int b);
> >
> > +int avcodec_default_execute(AVCodecContext *c, int (*func)(AVCodecContext *c2, void *arg2),void *arg, int *ret, int count, int size);
> > +int avcodec_default_execute2(AVCodecContext *c, int (*func)(AVCodecContext *c2, void *arg2, int, int),void *arg, int *ret, int count);
> > +
> 
> This looks wrong.  You're making the functions definitions static, so
> there should be no prototypes in headers.  This shouldn't even compile.
> 

No, I am not. I think you are mixing avcodec_default_execute* and
avcodec_thread_execute* definitions, aren't you ?

Ben