[FFmpeg-devel] Threading issue with avcodec_decode_video2 ?

[Don Moir]

>> I looked over those two functions, and sadly its required to call them
>> there, because its possible that width/height of the video are not
>> known during the init function, or may even change in case of vc1image
>> decoding.
>> Now, the static structures should only be initialized once, and guard
>> themself against re-init, so i see two solutions: Either lock this
>> block, or call them once in the init function to create the static
>> data, and keep calling them in there.
>>
>> I'm partial to the locking.
>>
>> - Hendrik
>
> Thank you Hendrik. Given what you said, putting a lock around the init functions in vc1_decode_frame does for sure have the least 
> amount of potential impact. That was my first pass on it but found it necessary to add avpriv_lock_avcodec and 
> avpriv_unlock_avcodec since these don't currently exist. Could be other ways as well. Should be a rare event.

My only current way to fix this is to have a unique build. After looking around some more and hints from Hendrik, it appears the fix 
is best put in vc1_decode_frame.

The problem code is:

    if (!s->context_initialized) {
        if (ff_msmpeg4_decode_init(avctx) < 0 ||  ff_vc1_decode_init_alloc_tables(v) < 0)
            goto err;
        s->low_delay = !avctx->has_b_frames || v->res_sprite;
        if (v->profile == PROFILE_ADVANCED) {
            s->h_edge_pos = avctx->coded_width;
            s->v_edge_pos = avctx->coded_height;
        }
    }

The offending statement is:

        if (ff_msmpeg4_decode_init(avctx) < 0 ||  ff_vc1_decode_init_alloc_tables(v) < 0)

After adding avpriv_lock_codec and avpriv_unlock_codec since they don't exist.

A couple ways to organize the fix but this way produces no addional testing or assignments:

    if (!s->context_initialized) {
        avpriv_lock_avcodec ();
        if (ff_msmpeg4_decode_init(avctx) < 0 ||  ff_vc1_decode_init_alloc_tables(v) < 0) {
            avpriv_unlock_avcodec ();
            goto err;
        }
        avpriv_unlock_avcodec ();
        s->low_delay = !avctx->has_b_frames || v->res_sprite;
        if (v->profile == PROFILE_ADVANCED) {
            s->h_edge_pos = avctx->coded_width;
            s->v_edge_pos = avctx->coded_height;
        }
    }

I don't like this way with the 2 unlocks and I would probably restructure to an initialize_context function but trying to keep it 
straight foward here.

If there is any way else to fix it with less impact, I don't know.

This is a severe problem when multi-threading leading to crashes and as Carl stated: "As you know, we consider crashes important". 
Who doesn't ?

I don't know if you have any way of testing multi-threading issues.

I don't know if I should create a ticket on it just so Carl can close it because I did not provide "complete, uncut console output 
and backtrace" and he will not be able to produce the problem with a single threaded (or single video) application.