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. Ronald _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
