[Ffmpeg-devel] DV decoding crash [ plus PATCHES]

Steven M. Schultz sms
Sat Dec 31 19:17:19 CET 2005


On Thu, 29 Dec 2005, Steven M. Schultz wrote:

> Hi -
> 
> 	It looks like the massive changes over the last couple days
> 	broke DV decoding.

	With talk of an impending release I'd think that fixing a 
	segfault would be of interest ;)

> 	In libavcodec/dv.c  the 'dv_anchor' array was made part of
> 	'DVVideoContext' - a "per context" (thread?) structure.  However,
> 	'dv_anchor' is only initialized in the "global" context.

	The problem manifests itself on the 2nd call to avcodec_open()
	within a program.

> 	In 'dvvideo_init()' the logic is:
> 
> 	static int done=0;
> 	if (!done) {
>          done = 1;
> 	   s->dv_anchor = av_malloc(12*27*sizeof(void*));
> 	   ...
> 	}
> 
> 	Thus dv_anchor is only initialized once on the first call to
> 	dvvideo_init() which would come from calling avcodec_open.

> 	After that, further calls to avcodec_open() will NOT initialize
> 	dv_anchor leaving it NULL and causing a crash.

	I have attached 2 patches from which to choose.  The first restores
	the "global to DV codec" behaviour of dv_anchor.  The second
	allocates an identical array for each DVVideoContext. 

	Both work - pick the one you like the best :)

	Cheers,
	Steven Schulz
-------------- next part --------------
--- dv.c.dist	2005-12-27 22:53:01.000000000 -0800
+++ dv.c	2005-12-29 15:10:42.000000000 -0800
@@ -49,11 +49,11 @@
     void (*get_pixels)(DCTELEM *block, const uint8_t *pixels, int line_size);
     void (*fdct[2])(DCTELEM *block);
     void (*idct_put[2])(uint8_t *dest, int line_size, DCTELEM *block);
-
-    /* MultiThreading */
-    uint8_t** dv_anchor;
 } DVVideoContext;
 
+/* MultiThreading - applies to entire DV codec, not just the avcontext */
+uint8_t** dv_anchor;
+
 #define TEX_VLC_BITS 9
 
 #ifdef DV_CODEC_TINY_TARGET
@@ -118,12 +118,12 @@
             return -ENOMEM;
 
         /* dv_anchor lets each thread know its Id */
-        s->dv_anchor = av_malloc(12*27*sizeof(void*));
-        if (!s->dv_anchor) {
+        dv_anchor = av_malloc(12*27*sizeof(void*));
+        if (!dv_anchor) {
             return -ENOMEM;
         }
         for (i=0; i<12*27; i++)
-            s->dv_anchor[i] = (void*)(size_t)i;
+            dv_anchor[i] = (void*)(size_t)i;
 
         /* it's faster to include sign bit in a generic VLC parsing scheme */
         for (i=0, j=0; i<NB_DV_VLC; i++, j++) {
@@ -150,10 +150,9 @@
                  new_dv_vlc_len, 1, 1, new_dv_vlc_bits, 2, 2, 0);
 
         dv_rl_vlc = av_mallocz_static(dv_vlc.table_size * sizeof(RL_VLC_ELEM));
-        if (!dv_rl_vlc) {
-            av_free(s->dv_anchor);
+        if (!dv_rl_vlc)
             return -ENOMEM;
-        }
+
         for(i = 0; i < dv_vlc.table_size; i++){
             int code= dv_vlc.table[i][0];
             int len = dv_vlc.table[i][1];
@@ -939,7 +938,7 @@
     s->picture.top_field_first = 0;
 
     s->buf = buf;
-    avctx->execute(avctx, dv_decode_mt, (void**)&s->dv_anchor[0], NULL,
+    avctx->execute(avctx, dv_decode_mt, (void**)&dv_anchor[0], NULL,
                    s->sys->difseg_size * 27);
 
     emms_c();
@@ -968,7 +967,7 @@
     s->picture.pict_type = FF_I_TYPE;
 
     s->buf = buf;
-    c->execute(c, dv_encode_mt, (void**)&s->dv_anchor[0], NULL,
+    c->execute(c, dv_encode_mt, (void**)&dv_anchor[0], NULL,
                s->sys->difseg_size * 27);
 
     emms_c();
@@ -977,9 +976,6 @@
 
 static int dvvideo_close(AVCodecContext *c)
 {
-    DVVideoContext *s = c->priv_data;
-
-    av_free(s->dv_anchor);
 
     return 0;
 }
-------------- next part --------------
--- dv.c.dist	2005-12-31 10:00:16.000000000 -0800
+++ dv.c	2005-12-31 10:09:45.000000000 -0800
@@ -117,14 +117,6 @@
         if (!dv_vlc_map)
             return -ENOMEM;
 
-        /* dv_anchor lets each thread know its Id */
-        s->dv_anchor = av_malloc(12*27*sizeof(void*));
-        if (!s->dv_anchor) {
-            return -ENOMEM;
-        }
-        for (i=0; i<12*27; i++)
-            s->dv_anchor[i] = (void*)(size_t)i;
-
         /* it's faster to include sign bit in a generic VLC parsing scheme */
         for (i=0, j=0; i<NB_DV_VLC; i++, j++) {
             new_dv_vlc_bits[j] = dv_vlc_bits[i];
@@ -215,6 +207,14 @@
         }
     }
 
+    /* dv_anchor lets each thread know its Id */
+    s->dv_anchor = av_malloc(12*27*sizeof(void*));
+    if (!s->dv_anchor) {
+        return -ENOMEM;
+    }
+    for (i=0; i<12*27; i++)
+        s->dv_anchor[i] = (void*)(size_t)i;
+
     /* Generic DSP setup */
     dsputil_init(&dsp, avctx);
     s->get_pixels = dsp.get_pixels;



More information about the ffmpeg-devel mailing list