"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:
>>> [..]
>>>>>>>>> -    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.

-- 
Måns Rullgård
[email protected]
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to