[FFmpeg-trac] #1876(avcodec:closed): vc1dec.c multi-threading decode crash issue

[FFmpeg]

#1876: vc1dec.c multi-threading decode crash issue
-------------------------------------+-------------------------------------
             Reporter:  DonMoir      |                    Owner:
                 Type:  defect       |                   Status:  closed
             Priority:  important    |                Component:  avcodec
              Version:  unspecified  |               Resolution:
             Keywords:  vc1 crash    |  needs_more_info
             Blocking:               |               Blocked By:
Analyzed by developer:  0            |  Reproduced by developer:  0
-------------------------------------+-------------------------------------

Comment (by DonMoir):

 It can be hard to reproduce and it may take me about 10 tries. It came to
 me as a bug from a beta tester where it would happen on occasion. So then
 I wrote some code to trigger it more often. Running, opening multiple
 instances of the above file at the same time is the best way I can
 reproduce it. It does not happen anywhere else except in code that calls
 vc1_decode_frame. The crash can be anywhere because of memory overwrites
 etc.

 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 in vc1_decode_frame 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.

-- 
Ticket URL: <https://ffmpeg.org/trac/ffmpeg/ticket/1876#comment:15>
FFmpeg <http://ffmpeg.org>
FFmpeg issue tracker