[FFmpeg-devel] Threading issue with avcodec_decode_video2 ?

[Hendrik Leppkes]

On Sat, Oct 27, 2012 at 9:40 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Sat, Oct 27, 2012 at 12:59:37AM -0400, Don Moir wrote:
>> >I load up multiple instances of the same file. These are used for
>> >playback, thumbnails, and other things. Each is in a separate
>> >thread with their own context, variables, etc.
>> >
>> >We noticed a crash when decoding WMV3 files. It's was a rare event so hard to track down.
>> >
>> >Recently I put some code in to help find the problem. It seems
>> >that avcodec_decode_video2 is not thread safe at least for WMV3
>> >decoding.
>> >
>> >I have disable-pthreads set. I use av_lockmgr_register and see it
>> >allows a lock for avformat and avcodec. The lock for avcodec never
>> >gets called.
>> >
>> >If I put a lock around avcodec_decode_video2, the problem goes
>> >away. After several crashes that led nowhere, I isolated it to
>> >avcodec_decode_video2.
>> >
>> >Shouldn't related decoders be setting the lock via the lock manager if needed ?
>> >
>> >I don't want to have to lock the entire avcodec_decode_video2 call
>> >and lock should be special cased for each decoder and so I am left
>> >wondering.
>> >
>>
>> Looks like the call to ff_msmpeg4_decode_init is causing the problem.
>>
>> About midway down in vc1_decode_frame you will see:
>>
>> if (ff_msmpeg4_decode_init(avctx) < 0 || ff_vc1_decode_init_alloc_tables(v) < 0)
>>
>> I added avpriv_lock_avcodec and avpriv_unlock_avcodec and wrapped
>> ff_msmpeg4_decode_init(avctx) with a lock and problem went away. So
>> there may be more to it but can't get it to crash anymore.
>> ff_vc1_decode_init_alloc_tables(v) appears to be ok.
>>
>> The scary part is what about the other decoders. I have not noticed
>> a problem outside of this though. There could also be a way to fix
>> it without using a lock but didn't look further.
>
> thanks for pointing me to these functions, they indeed had some
> thread saftey bugs.
> should be fixed, does it work without the lock now ?
>

Couldn't ff_msmpeg4_decode_init be called from the decoder init
function, which is already inside a lock, so the static portions get
initialized there?
On a quick look, it doesn't seem to depend on any important values
into avctx which would only be available later.

- Hendrik