"Ronald S. Bultje" <[email protected]> writes: > Hi, > > 2011/6/6 Måns Rullgård <[email protected]>: >> "Ronald S. Bultje" <[email protected]> writes: >>> 2011/6/6 Måns Rullgård <[email protected]>: >>>> "Ronald S. Bultje" <[email protected]> writes: >>>>> 2011/6/6 Måns Rullgård <[email protected]>: >>>>>> "Ronald S. Bultje" <[email protected]> writes: >>>>> [..] >>>>>>>>>>> - if (avctx->codec->priv_class) >>>>>>>>>>> + if (avctx->codec && avctx->codec->priv_class) >>>>>>>>>>> av_opt_free(avctx->priv_data); >>>>> [..] >>>>>>> Logic-wise, because what else would it be? >>>>>> >>>>>> Not null of course? Are you saying the avctx variable has a totally >>>>>> different meaning here when threads are used? >>>>> >>>>> Yes. Look at the code. What we're doing here is free the codec private >>>>> options. >>>>> >>>>> For threading, the AVCodecContext that interacts with the application >>>>> _has no codec-specific context in private data_. >>>>> >>>>> This data is in the worker threads, not in the application-facing >>>>> thread. The application facing AVCodecContext has some frame threading >>>>> private data there that is used in pthread.c, but calling >>>>> AVCodec-specific functions or option-freeing functions there would >>>>> crash. >>>>> >>>>>>> The application-level AVCtx has nothing in it, it's a placeholder that >>>>>>> is there to synchronize the individual per-thread AVCtxs that run in >>>>>>> each worker thread. It has no private context other than the one that >>>>>>> syncs between threads. It has no private_data with codec information >>>>>>> in it. Therefore, AVCodec == NULL. >>>>>> >>>>>> So where is whatever is there in the non-threaded case? >>>>> >>>>> In the non-threaded case, everything is in AVCodecContext, the one >>>>> facing the application. >>>>> >>>>> In the threaded case, it is not. That's why AVCodec is set to NULL >>>>> before freeing stuff. Otherwise you'd free stuff that isn't there -> >>>>> crash. >>>> >>>> So how do codec-private options work here? I don't see them being freed >>>> anywhere. If valgrind didn't report leaks, that's because none of the >>>> tests set any private options. >>> >>> I don't think codec-private options work with threading enabled ATM. >> >> That sounds like a serious flaw. > > They don't work yet in git/master anyway for decoders, so it's not > like this broke anything. > > Anton posted WIP patches to make them work. To make this work for MT, > the syncing between cached options and the decoder context should > still happen in avcodec_open, but should proxy across child thread > contexts. Freeing should be done like the code above, but in pthread.c > in frame_thread_close(). Anton can probably provide patches that do > that when he makes it work for the non-threaded case.
Very well, patch OK. -- Måns Rullgård [email protected] _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
