On Tue, Apr 21, 2009 at 10:35:08PM +0200, Stefano Sabatini wrote: > On date Monday 2009-04-20 07:01:56 +0200, Michael Niedermayer encoded: > > On Sun, Mar 09, 2008 at 01:00:36PM +0100, Stefano Sabatini wrote: > > > On date Saturday 2008-03-08 20:31:25 +0100, Michael Dorr encoded: > > > > On Saturday 08 March 2008 07:25:49 pm Stefano Sabatini wrote: > > > > > On date Saturday 2008-03-08 18:55:26 +0100, Michael Niedermayer encoded: > > > > > > On Sat, Mar 08, 2008 at 06:33:27PM +0100, Stefano Sabatini wrote: > > > > > > > Hi, > > > > > > > as in $subject. > > > > > > > > > > > > > > If threads support is not enabled and threads count is set to a > > > > > > > value greater than MAX_THREADS (currently is 8), then encoder is > > > > > > > initialized with a value of thread_count equal to 1, then > > > > > > > enc->threads_count is set to the value set by the user, finally > > > > > > > libavcodec crashes when it tryies to free the first thread beyond > > > > > > > MAX_THREADS (this happens when quitting ffplay). > > > > > > > > > > > > the command line parsing code should not accept >MAX_THREADS > > > > > > > > > > [...] > > > > > > > > > > I agree, but MAX_THREADS is defined in libavcodec/mpegvideo.h, which > > > > > is not public, furthermore the patch implements the correct behaviour > > > > > for opening an encoder so I'd like to see it applied. > > > > > > > > Shouldn't manually setting enc->thread_count be omitted completely, since it > > > > gets set correctly in avcodec_thread_init anyway (and if thread_init isn't > > > > called, it defaults to 1, which is also correct)? Theoretically, > > > > avcodec_thread_init could fail, then enc->thread_count and the 'real' number > > > > of threads will differ, and ffmpeg will crash ... > > > > > > Hi, > > [...] > > > Index: libavcodec/utils.c > > > =================================================================== > > > --- libavcodec/utils.c (revision 12393) > > > +++ libavcodec/utils.c (working copy) > > > @@ -1350,6 +1350,7 @@ > > > > > > #if !defined(HAVE_THREADS) > > > int avcodec_thread_init(AVCodecContext *s, int thread_count){ > > > + s->thread_count = thread_count; > > > return -1; > > > } > > > #endif > > > > patch ok > >. I approved this patch because it allows code to be simplified not because it adds some error checking, which it clearly does not. I dont think this change introduces any new bugs, but i might miss: <>