[vlc-devel] [RFC PATCH 1/6] decoder: change error handling

Thomas Guillem thomas at gllm.fr
Fri Jul 1 18:14:50 CEST 2016


We need an atomic since the state can be changed from an other thread (via
aout_update_format())
---
 include/vlc_codec.h                 | 25 ++++++++++++++++++++++++-
 modules/codec/avcodec/video.c       |  4 ++--
 modules/codec/gstreamer/gstdecode.c | 16 +++++++++-------
 modules/codec/videotoolbox.m        |  2 +-
 src/input/decoder.c                 | 21 +++++++++++----------
 5 files changed, 47 insertions(+), 21 deletions(-)

diff --git a/include/vlc_codec.h b/include/vlc_codec.h
index 2d01d26..132d935 100644
--- a/include/vlc_codec.h
+++ b/include/vlc_codec.h
@@ -24,6 +24,7 @@
 #ifndef VLC_CODEC_H
 #define VLC_CODEC_H 1
 
+#include <vlc_atomic.h>
 #include <vlc_block.h>
 #include <vlc_es.h>
 #include <vlc_picture.h>
@@ -43,6 +44,16 @@
 
 typedef struct decoder_owner_sys_t decoder_owner_sys_t;
 
+#define DECODER_STATE_INIT          0
+#define DECODER_STATE_HW            (1<<0)
+#define DECODER_STATE_ERROR_GENERIC (1<<1)
+#define DECODER_STATE_ERROR_INPUT   (1<<2)
+#define DECODER_STATE_ERROR_OUTPUT  (1<<3)
+
+#define DECODER_STATE_ERROR (DECODER_STATE_ERROR_GENERIC | \
+                             DECODER_STATE_ERROR_INPUT | \
+                             DECODER_STATE_ERROR_OUTPUT)
+
 /*
  * BIG FAT WARNING : the code relies in the first 4 members of filter_t
  * and decoder_t to be the same, so if you have anything to add, do it
@@ -158,7 +169,9 @@ struct decoder_t
     /* Private structure for the owner of the decoder */
     decoder_owner_sys_t *p_owner;
 
-    bool                b_error;
+    /* Bitwise state set by decoder module or by the decoder thread. See
+     * DECODER_STATE_* define */
+    atomic_int           state;
 };
 
 /**
@@ -353,6 +366,16 @@ static inline int decoder_UpdateAudioFormat( decoder_t *dec )
         return -1;
 }
 
+static inline void decoder_AddState( decoder_t *dec, int state )
+{
+    atomic_fetch_or( &dec->state, state );
+}
+
+static inline void decoder_SetErrorGeneric( decoder_t *dec )
+{
+    decoder_AddState( dec, DECODER_STATE_ERROR_GENERIC );
+}
+
 /**
  * This function will return a new audio buffer usable by a decoder as an
  * output buffer. It must be released with block_Release() or returned it to
diff --git a/modules/codec/avcodec/video.c b/modules/codec/avcodec/video.c
index a97fec2..0e854dd 100644
--- a/modules/codec/avcodec/video.c
+++ b/modules/codec/avcodec/video.c
@@ -302,7 +302,7 @@ static void lavc_CopyPicture(decoder_t *dec, picture_t *pic, AVFrame *frame)
 
         msg_Err(dec, "Unsupported decoded output format %d (%s)",
                 sys->p_context->pix_fmt, (name != NULL) ? name : "unknown");
-        dec->b_error = true;
+        decoder_SetErrorGeneric( dec );
         return;
     }
 
@@ -724,7 +724,7 @@ static picture_t *DecodeVideo( decoder_t *p_dec, block_t **pp_block )
         AVFrame *frame = av_frame_alloc();
         if (unlikely(frame == NULL))
         {
-            p_dec->b_error = true;
+            decoder_SetErrorGeneric( p_dec );
             break;
         }
 
diff --git a/modules/codec/gstreamer/gstdecode.c b/modules/codec/gstreamer/gstdecode.c
index ecaf2a2..c0eaee3 100644
--- a/modules/codec/gstreamer/gstdecode.c
+++ b/modules/codec/gstreamer/gstdecode.c
@@ -686,7 +686,7 @@ static picture_t *DecodeBlock( decoder_t *p_dec, block_t **pp_block )
         if( unlikely( p_buf == NULL ) )
         {
             msg_Err( p_dec, "failed to create input gstbuffer" );
-            p_dec->b_error = true;
+            decoder_SetErrorGeneric( p_dec );
             block_Release( p_block );
             goto done;
         }
@@ -728,7 +728,7 @@ static picture_t *DecodeBlock( decoder_t *p_dec, block_t **pp_block )
         {
             /* block will be released internally,
              * when gst_buffer_unref() is called */
-            p_dec->b_error = true;
+            decoder_SetErrorGeneric( p_dec );
             msg_Err( p_dec, "failed to push buffer" );
             goto done;
         }
@@ -756,9 +756,9 @@ check_messages:
             msg_Dbg( p_dec, "Pipeline is prerolled" );
             break;
         default:
-            p_dec->b_error = default_msg_handler( p_dec, p_msg );
-            if( p_dec->b_error )
+            if( default_msg_handler( p_dec, p_msg ) )
             {
+                decoder_SetErrorGeneric( p_dec );
                 gst_message_unref( p_msg );
                 goto done;
             }
@@ -793,7 +793,7 @@ check_messages:
             {
                 msg_Err( p_dec, "failed to map gst video frame" );
                 gst_buffer_unref( p_buf );
-                p_dec->b_error = true;
+                decoder_SetErrorGeneric( p_dec );
                 goto done;
             }
 
@@ -845,9 +845,11 @@ static void CloseDecoder( vlc_object_t *p_this )
                 msg_Dbg( p_dec, "got eos" );
                 break;
             default:
-                p_dec->b_error = default_msg_handler( p_dec, p_msg );
-                if( p_dec->b_error )
+                if( default_msg_handler( p_dec, p_msg ) )
+                {
+                    decoder_SetErrorGeneric( p_dec );
                     msg_Err( p_dec, "pipeline may not close gracefully" );
+                }
                 break;
             }
 
diff --git a/modules/codec/videotoolbox.m b/modules/codec/videotoolbox.m
index 73423a9..9d0f4c2 100644
--- a/modules/codec/videotoolbox.m
+++ b/modules/codec/videotoolbox.m
@@ -1003,7 +1003,7 @@ static picture_t *DecodeBlock(decoder_t *p_dec, block_t **pp_block)
                         msg_Err(p_dec, "decoder failure: invalid SPS/PPS");
                     else if (status == -6661) {
                         msg_Err(p_dec, "decoder failure: invalid argument");
-                        p_dec->b_error = true;
+                        decoder_SetErrorGeneric( p_dec );
                     } else if (status == -8969 || status == -12909) {
                         msg_Err(p_dec, "decoder failure: bad data (%i)", status);
                         StopVideoToolbox(p_dec);
diff --git a/src/input/decoder.c b/src/input/decoder.c
index 06e569d..5b0fcc0 100644
--- a/src/input/decoder.c
+++ b/src/input/decoder.c
@@ -187,7 +187,6 @@ static void UnloadDecoder( decoder_t *p_dec )
 
     es_format_Clean( &p_dec->fmt_in );
     es_format_Clean( &p_dec->fmt_out );
-    p_dec->b_error = false;
 }
 
 static void DecoderUpdateFormatLocked( decoder_t *p_dec )
@@ -302,7 +301,7 @@ static int aout_update_format( decoder_t *p_dec )
         if( p_aout == NULL )
         {
             msg_Err( p_dec, "failed to create audio output" );
-            p_dec->b_error = true;
+            decoder_AddState( p_dec, DECODER_STATE_ERROR_OUTPUT );
             return -1;
         }
 
@@ -461,7 +460,7 @@ static subpicture_t *spu_new_buffer( decoder_t *p_dec,
 
     while( i_attempts-- )
     {
-        if( p_dec->b_error )
+        if( atomic_load( &p_dec->state ) & DECODER_STATE_ERROR )
             break;
 
         p_vout = input_resource_HoldVout( p_owner->p_resource );
@@ -756,7 +755,7 @@ static void DecoderProcessSout( decoder_t *p_dec, block_t *p_block )
             {
                 msg_Err( p_dec, "cannot create packetizer output (%4.4s)",
                          (char *)&p_owner->fmt.i_codec );
-                p_dec->b_error = true;
+                decoder_SetErrorGeneric( p_dec );
 
                 block_ChainRelease(p_sout_block);
                 break;
@@ -773,7 +772,7 @@ static void DecoderProcessSout( decoder_t *p_dec, block_t *p_block )
             {
                 msg_Err( p_dec, "cannot continue streaming due to errors" );
 
-                p_dec->b_error = true;
+                decoder_SetErrorGeneric( p_dec );
 
                 /* Cleanup */
                 block_ChainRelease( p_next );
@@ -1011,7 +1010,7 @@ static void DecoderProcessVideo( decoder_t *p_dec, block_t *p_block )
                 UnloadDecoder( p_dec );
                 if( LoadDecoder( p_dec, false, &p_packetizer->fmt_out ) )
                 {
-                    p_dec->b_error = true;
+                    decoder_SetErrorGeneric( p_dec );
                     block_ChainRelease( p_packetized_block );
                     return;
                 }
@@ -1189,7 +1188,7 @@ static void DecoderProcessAudio( decoder_t *p_dec, block_t *p_block )
                 UnloadDecoder( p_dec );
                 if( LoadDecoder( p_dec, false, &p_packetizer->fmt_out ) )
                 {
-                    p_dec->b_error = true;
+                    decoder_SetErrorGeneric( p_dec );
                     block_ChainRelease( p_packetized_block );
                     return;
                 }
@@ -1315,7 +1314,7 @@ static void DecoderProcess( decoder_t *p_dec, block_t *p_block )
 {
     decoder_owner_sys_t *p_owner = p_dec->p_owner;
 
-    if( p_dec->b_error )
+    if( atomic_load( &p_dec->state ) & DECODER_STATE_ERROR )
         goto error;
 
     if( p_block )
@@ -1344,7 +1343,7 @@ static void DecoderProcess( decoder_t *p_dec, block_t *p_block )
 
         default:
             msg_Err( p_dec, "unknown ES format" );
-            p_dec->b_error = true;
+            decoder_SetErrorGeneric( p_dec );
     }
 
 error:
@@ -1357,7 +1356,7 @@ static void DecoderProcessFlush( decoder_t *p_dec )
     decoder_owner_sys_t *p_owner = p_dec->p_owner;
     decoder_t *p_packetizer = p_owner->p_packetizer;
 
-    if( p_dec->b_error )
+    if( atomic_load( &p_dec->state ) & DECODER_STATE_ERROR )
         return;
 
     if( p_packetizer != NULL && p_packetizer->pf_flush != NULL )
@@ -1598,6 +1597,8 @@ static decoder_t * CreateDecoder( vlc_object_t *p_parent,
     p_dec->pf_queue_audio = DecoderQueueAudio;
     p_dec->pf_queue_sub = DecoderQueueSpu;
 
+    atomic_init( &p_dec->state, DECODER_STATE_INIT );
+
     /* Load a packetizer module if the input is not already packetized */
     if( p_sout == NULL && !fmt->b_packetized )
     {
-- 
2.8.1



More information about the vlc-devel mailing list