[vlc-devel] [PATCH 5/8] decoder: add VLCDEC_ECRITICAL status

Thomas Guillem thomas at gllm.fr
Wed Feb 8 19:53:52 CET 2017


This replaces the decoder_t->b_error boolean.

For now, only decoders that were setting b_error return VLCDEC_ECRITICAL, but a
lot more decoders should use this value since critical errors are often ignored.
---
 include/vlc_codec.h                 |  8 ++++++--
 modules/codec/avcodec/audio.c       |  9 +++++----
 modules/codec/avcodec/video.c       | 29 ++++++++++++++++++-----------
 modules/codec/gstreamer/gstdecode.c | 20 +++++++++-----------
 modules/codec/mpg123.c              | 13 +++++++++++--
 modules/codec/omxil/mediacodec.c    |  9 ++++++---
 modules/codec/videotoolbox.m        |  6 +++---
 modules/codec/vpx.c                 |  5 +++--
 modules/packetizer/a52.c            |  1 -
 modules/packetizer/dts.c            |  1 -
 modules/packetizer/mpeg4audio.c     |  1 -
 modules/packetizer/mpegaudio.c      |  1 -
 src/input/decoder.c                 | 25 ++++++++++++++++---------
 13 files changed, 77 insertions(+), 51 deletions(-)

diff --git a/include/vlc_codec.h b/include/vlc_codec.h
index 2f8facc18f..24b18054de 100644
--- a/include/vlc_codec.h
+++ b/include/vlc_codec.h
@@ -68,6 +68,7 @@ struct decoder_t
     bool                b_frame_drop_allowed;
 
 #   define VLCDEC_SUCCESS   VLC_SUCCESS
+#   define VLCDEC_ECRITICAL VLC_EGENERIC
     /* This function is called to decode one packetized block.
      *
      * The module implementation will own the input block (p_block) and should
@@ -78,6 +79,11 @@ struct decoder_t
      * If p_block is NULL, the decoder asks the module to drain itself. The
      * module should return all available output frames/block via the queue
      * functions.
+     *
+     * Return values can be:
+     *  VLCDEC_SUCCESS: pf_decode will be called again
+     *  VLCDEC_ECRITICAL: in case of critical error, pf_decode won't be called
+     *  again.
      */
     int                 ( * pf_decode )   ( decoder_t *, block_t *p_block );
 
@@ -171,8 +177,6 @@ struct decoder_t
 
     /* Private structure for the owner of the decoder */
     decoder_owner_sys_t *p_owner;
-
-    bool                b_error;
 };
 
 /**
diff --git a/modules/codec/avcodec/audio.c b/modules/codec/avcodec/audio.c
index 0700963745..92fbc8e01b 100644
--- a/modules/codec/avcodec/audio.c
+++ b/modules/codec/avcodec/audio.c
@@ -301,7 +301,7 @@ static void Flush( decoder_t *p_dec )
 /*****************************************************************************
  * DecodeBlock: Called to decode one frame
  *****************************************************************************/
-static block_t *DecodeBlock( decoder_t *p_dec, block_t **pp_block )
+static block_t *DecodeBlock( decoder_t *p_dec, block_t **pp_block, bool *error )
 {
     decoder_sys_t *p_sys = p_dec->p_sys;
     AVCodecContext *ctx = p_sys->p_context;
@@ -452,7 +452,7 @@ static block_t *DecodeBlock( decoder_t *p_dec, block_t **pp_block )
     return ( p_sys->p_decoded ) ? DequeueOneDecodedFrame( p_sys ) : NULL;
 
 end:
-    p_dec->b_error = true;
+    *error = true;
     if( pp_block )
     {
         assert( *pp_block == p_block );
@@ -468,10 +468,11 @@ drop:
 
 static int DecodeAudio( decoder_t *p_dec, block_t *p_block )
 {
+    bool error = false;
     block_t **pp_block = p_block ? &p_block : NULL, *p_out;
-    while( ( p_out = DecodeBlock( p_dec, pp_block ) ) != NULL )
+    while( ( p_out = DecodeBlock( p_dec, pp_block, &error ) ) != NULL )
         decoder_QueueAudio( p_dec, p_out );
-    return VLCDEC_SUCCESS;
+    return error ? VLCDEC_ECRITICAL : VLCDEC_SUCCESS;
 }
 
 static block_t * ConvertAVFrame( decoder_t *p_dec, AVFrame *frame )
diff --git a/modules/codec/avcodec/video.c b/modules/codec/avcodec/video.c
index b2ab47878a..a00e2b4634 100644
--- a/modules/codec/avcodec/video.c
+++ b/modules/codec/avcodec/video.c
@@ -305,7 +305,7 @@ static int lavc_UpdateVideoFormat(decoder_t *dec, AVCodecContext *ctx,
  * Copies a picture from the libavcodec-allocate buffer to a picture_t.
  * This is used when not in direct rendering mode.
  */
-static void lavc_CopyPicture(decoder_t *dec, picture_t *pic, AVFrame *frame)
+static int lavc_CopyPicture(decoder_t *dec, picture_t *pic, AVFrame *frame)
 {
     decoder_sys_t *sys = dec->p_sys;
 
@@ -315,8 +315,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;
-        return;
+        return VLC_EGENERIC;
     }
 
     for (int plane = 0; plane < pic->i_planes; plane++)
@@ -334,6 +333,7 @@ static void lavc_CopyPicture(decoder_t *dec, picture_t *pic, AVFrame *frame)
             dst += dst_stride;
         }
     }
+    return VLC_SUCCESS;
 }
 
 static int OpenVideoCodec( decoder_t *p_dec )
@@ -708,7 +708,7 @@ static void update_late_frame_count( decoder_t *p_dec, block_t *p_block, mtime_t
 /*****************************************************************************
  * DecodeBlock: Called to decode one or more frames
  *****************************************************************************/
-static picture_t *DecodeBlock( decoder_t *p_dec, block_t **pp_block )
+static picture_t *DecodeBlock( decoder_t *p_dec, block_t **pp_block, bool *error )
 {
     decoder_sys_t *p_sys = p_dec->p_sys;
     AVCodecContext *p_context = p_sys->p_context;
@@ -846,7 +846,7 @@ static picture_t *DecodeBlock( decoder_t *p_dec, block_t **pp_block )
             if (ret == AVERROR(ENOMEM) || ret == AVERROR(EINVAL))
             {
                 msg_Err(p_dec, "avcodec_send_packet critical error");
-                p_dec->b_error = true;
+                *error = true;
             }
             av_packet_unref( &pkt );
             break;
@@ -857,7 +857,7 @@ static picture_t *DecodeBlock( decoder_t *p_dec, block_t **pp_block )
         AVFrame *frame = av_frame_alloc();
         if (unlikely(frame == NULL))
         {
-            p_dec->b_error = true;
+            *error = true;
             break;
         }
 
@@ -867,7 +867,7 @@ static picture_t *DecodeBlock( decoder_t *p_dec, block_t **pp_block )
             if (ret == AVERROR(ENOMEM) || ret == AVERROR(EINVAL))
             {
                 msg_Err(p_dec, "avcodec_receive_frame critical error");
-                p_dec->b_error = true;
+                *error = true;
             }
             av_frame_free(&frame);
             break;
@@ -936,7 +936,7 @@ static picture_t *DecodeBlock( decoder_t *p_dec, block_t **pp_block )
                       = malloc( sizeof(video_palette_t) );
             if( !p_palette )
             {
-                p_dec->b_error = true;
+                *error = true;
                 av_frame_free(&frame);
                 break;
             }
@@ -970,7 +970,13 @@ static picture_t *DecodeBlock( decoder_t *p_dec, block_t **pp_block )
             }
 
             /* Fill picture_t from AVFrame */
-            lavc_CopyPicture(p_dec, p_pic, frame);
+            if( lavc_CopyPicture( p_dec, p_pic, frame ) != VLC_SUCCESS )
+            {
+                *error = true;
+                av_frame_free(&frame);
+                picture_Release( p_pic );
+                break;
+            }
         }
         else
         {
@@ -1022,9 +1028,10 @@ static int DecodeVideo( decoder_t *p_dec, block_t *p_block )
 {
     block_t **pp_block = p_block ? &p_block : NULL;
     picture_t *p_pic;
-    while( ( p_pic = DecodeBlock( p_dec, pp_block ) ) != NULL )
+    bool error = false;
+    while( ( p_pic = DecodeBlock( p_dec, pp_block, &error ) ) != NULL )
         decoder_QueueVideo( p_dec, p_pic );
-    return VLCDEC_SUCCESS;
+    return error ? VLCDEC_ECRITICAL : VLCDEC_SUCCESS;
 }
 
 /*****************************************************************************
diff --git a/modules/codec/gstreamer/gstdecode.c b/modules/codec/gstreamer/gstdecode.c
index 89cc39bb73..e020cffe47 100644
--- a/modules/codec/gstreamer/gstdecode.c
+++ b/modules/codec/gstreamer/gstdecode.c
@@ -680,9 +680,8 @@ static int DecodeBlock( decoder_t *p_dec, block_t *p_block )
         if( unlikely( p_buf == NULL ) )
         {
             msg_Err( p_dec, "failed to create input gstbuffer" );
-            p_dec->b_error = true;
             block_Release( p_block );
-            goto done;
+            return VLCDEC_ECRITICAL;
         }
 
         if( p_block->i_dts > VLC_TS_INVALID )
@@ -722,9 +721,8 @@ static int DecodeBlock( decoder_t *p_dec, block_t *p_block )
         {
             /* block will be released internally,
              * when gst_buffer_unref() is called */
-            p_dec->b_error = true;
             msg_Err( p_dec, "failed to push buffer" );
-            goto done;
+            return VLCDEC_ECRITICAL;
         }
     }
     else
@@ -749,11 +747,10 @@ static int DecodeBlock( decoder_t *p_dec, block_t *p_block )
             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 ) )
             {
                 gst_message_unref( p_msg );
-                goto done;
+                return VLCDEC_ECRITICAL;
             }
             break;
         }
@@ -788,8 +785,7 @@ static int DecodeBlock( decoder_t *p_dec, block_t *p_block )
             {
                 msg_Err( p_dec, "failed to map gst video frame" );
                 gst_buffer_unref( p_buf );
-                p_dec->b_error = true;
-                goto done;
+                return VLCDEC_ECRITICAL;
             }
 
             gst_CopyPicture( p_pic, &frame );
@@ -841,9 +837,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 ) )
+                {
                     msg_Err( p_dec, "pipeline may not close gracefully" );
+                    return VLCDEC_ECRITICAL;
+                }
                 break;
             }
 
diff --git a/modules/codec/mpg123.c b/modules/codec/mpg123.c
index 90880f21bd..07b5e31937 100644
--- a/modules/codec/mpg123.c
+++ b/modules/codec/mpg123.c
@@ -55,6 +55,7 @@ struct decoder_sys_t
     mpg123_handle * p_handle;
     date_t          end_date;
     block_t       * p_out;
+    bool            b_opened;
 };
 
 /*****************************************************************************
@@ -120,6 +121,7 @@ static int MPG123Open( decoder_t *p_dec )
         return VLC_EGENERIC;
     }
 
+    p_sys->b_opened = true;
     return VLC_SUCCESS;
 }
 
@@ -133,8 +135,8 @@ static void Flush( decoder_t *p_dec )
     date_Set( &p_sys->end_date, 0 );
 
     mpg123_close( p_sys->p_handle );
-    if( MPG123Open( p_dec ) )
-        p_dec->b_error = true;
+    p_sys->b_opened = false;
+    MPG123Open( p_dec );
 }
 
 static int UpdateAudioFormat( decoder_t *p_dec )
@@ -196,6 +198,13 @@ static int DecodeBlock( decoder_t *p_dec, block_t *p_block )
     decoder_sys_t *p_sys = p_dec->p_sys;
     block_t *p_out = NULL;
 
+    if( !p_sys->b_opened )
+    {
+        if( p_block )
+            block_Release( p_block );
+        return VLCDEC_ECRITICAL;
+    }
+
     /* Feed input block */
     if( p_block != NULL )
     {
diff --git a/modules/codec/omxil/mediacodec.c b/modules/codec/omxil/mediacodec.c
index c42f9a1acb..36bad57f5a 100644
--- a/modules/codec/omxil/mediacodec.c
+++ b/modules/codec/omxil/mediacodec.c
@@ -1581,11 +1581,14 @@ end:
             if (var_Create(p_dec, "mediacodec-failed", VLC_VAR_VOID)
              == VLC_SUCCESS)
                 decoder_RequestReload(p_dec);
+            vlc_mutex_unlock(&p_sys->lock);
+            return VLCDEC_SUCCESS;
         }
         else
-            p_dec->b_error = true;
-        vlc_mutex_unlock(&p_sys->lock);
-        return VLCDEC_SUCCESS;
+        {
+            vlc_mutex_unlock(&p_sys->lock);
+            return VLCDEC_ECRITICAL;
+        }
     }
     else
     {
diff --git a/modules/codec/videotoolbox.m b/modules/codec/videotoolbox.m
index 8c90f318c5..a63b91748a 100644
--- a/modules/codec/videotoolbox.m
+++ b/modules/codec/videotoolbox.m
@@ -1136,13 +1136,13 @@ skip:
     return VLCDEC_SUCCESS;
 
 reload:
+    block_Release(p_block);
     /* Add an empty variable so that videotoolbox won't be loaded again for
      * this ES */
-     if (var_Create(p_dec, "videotoolbox-failed", VLC_VAR_VOID) == VLC_SUCCESS)
+    if (var_Create(p_dec, "videotoolbox-failed", VLC_VAR_VOID) == VLC_SUCCESS)
         decoder_RequestReload(p_dec);
     else
-        p_dec->b_error = true;
-    block_Release(p_block);
+        return VLCDEC_ECRITICAL;
     return VLCDEC_SUCCESS;
 }
 
diff --git a/modules/codec/vpx.c b/modules/codec/vpx.c
index a0db6c86c0..d742d01e6a 100644
--- a/modules/codec/vpx.c
+++ b/modules/codec/vpx.c
@@ -189,8 +189,9 @@ static int Decode(decoder_t *dec, block_t *block)
         free(pkt_pts);
         VPX_ERR(dec, ctx, "Failed to decode frame");
         if (err == VPX_CODEC_UNSUP_BITSTREAM)
-            dec->b_error = true;
-        return VLCDEC_SUCCESS;
+            return VLCDEC_ECRITICAL;
+        else
+            return VLCDEC_SUCCESS;
     }
 
     const void *iter = NULL;
diff --git a/modules/packetizer/a52.c b/modules/packetizer/a52.c
index d61a0c054b..1d66a538ef 100644
--- a/modules/packetizer/a52.c
+++ b/modules/packetizer/a52.c
@@ -265,7 +265,6 @@ static block_t *PacketizeBlock( decoder_t *p_dec, block_t **pp_block )
         case STATE_SEND_DATA:
             if( !(p_out_buffer = GetOutBuffer( p_dec )) )
             {
-                //p_dec->b_error = true;
                 return NULL;
             }
 
diff --git a/modules/packetizer/dts.c b/modules/packetizer/dts.c
index b7b43b3711..4c2351e8f8 100644
--- a/modules/packetizer/dts.c
+++ b/modules/packetizer/dts.c
@@ -283,7 +283,6 @@ static block_t *PacketizeBlock( decoder_t *p_dec, block_t **pp_block )
         case STATE_SEND_DATA:
             if( !(p_out_buffer = GetOutBuffer( p_dec )) )
             {
-                //p_dec->b_error = true;
                 return NULL;
             }
 
diff --git a/modules/packetizer/mpeg4audio.c b/modules/packetizer/mpeg4audio.c
index 73e4bdf343..6fe622aeb6 100644
--- a/modules/packetizer/mpeg4audio.c
+++ b/modules/packetizer/mpeg4audio.c
@@ -1123,7 +1123,6 @@ static block_t *PacketizeStreamBlock(decoder_t *p_dec, block_t **pp_block)
 
         p_out_buffer = block_Alloc(p_sys->i_frame_size);
         if (!p_out_buffer) {
-            //p_dec->b_error = true;
             return NULL;
         }
         p_buf = p_out_buffer->p_buffer;
diff --git a/modules/packetizer/mpegaudio.c b/modules/packetizer/mpegaudio.c
index 8716f17f41..391cb99fd2 100644
--- a/modules/packetizer/mpegaudio.c
+++ b/modules/packetizer/mpegaudio.c
@@ -535,7 +535,6 @@ static block_t *DecodeBlock( decoder_t *p_dec, block_t **pp_block )
         case STATE_SEND_DATA:
             if( !(p_buf = GetOutBuffer( p_dec, &p_out_buffer )) )
             {
-                //p_dec->b_error = true;
                 return NULL;
             }
 
diff --git a/src/input/decoder.c b/src/input/decoder.c
index 4ce0ff2b7a..596243cfaf 100644
--- a/src/input/decoder.c
+++ b/src/input/decoder.c
@@ -117,6 +117,8 @@ struct decoder_owner_sys_t
     unsigned frames_countdown;
     bool paused;
 
+    bool error;
+
     /* Waiting */
     bool b_waiting;
     bool b_first;
@@ -198,7 +200,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 int ReloadDecoder( decoder_t *p_dec, bool b_packetizer,
@@ -209,12 +210,13 @@ static int ReloadDecoder( decoder_t *p_dec, bool b_packetizer,
     es_format_Init( &fmt_in, UNKNOWN_ES, 0 );
     if( es_format_Copy( &fmt_in, p_fmt ) != VLC_SUCCESS )
     {
-        p_dec->b_error = true;
+        p_dec->p_owner->error = true;
         return VLC_EGENERIC;
     }
 
     /* Restart the decoder module */
     UnloadDecoder( p_dec );
+    p_dec->p_owner->error = false;
 
     if( reload == RELOAD_DECODER_AOUT )
     {
@@ -234,7 +236,7 @@ static int ReloadDecoder( decoder_t *p_dec, bool b_packetizer,
 
     if( LoadDecoder( p_dec, b_packetizer, &fmt_in ) )
     {
-        p_dec->b_error = true;
+        p_dec->p_owner->error = true;
         es_format_Clean( &fmt_in );
         return VLC_EGENERIC;
     }
@@ -513,7 +515,7 @@ static subpicture_t *spu_new_buffer( decoder_t *p_dec,
 
     while( i_attempts-- )
     {
-        if( p_dec->b_error )
+        if( p_owner->error )
             break;
 
         p_vout = input_resource_HoldVout( p_owner->p_resource );
@@ -817,7 +819,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;
+                p_owner->error = true;
 
                 if(p_block)
                     block_Release(p_block);
@@ -837,7 +839,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;
+                p_owner->error = true;
 
                 /* Cleanup */
 
@@ -1253,6 +1255,9 @@ static void DecoderDecode( decoder_t *p_dec, block_t *p_block )
         case VLCDEC_SUCCESS:
             p_owner->pf_update_stat( p_owner, 1, 0 );
             break;
+        case VLCDEC_ECRITICAL:
+            p_owner->error = true;
+            break;
         default:
             vlc_assert_unreachable();
     }
@@ -1268,7 +1273,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( p_owner->error )
         goto error;
 
     /* Here, the atomic doesn't prevent to miss a reload request.
@@ -1336,7 +1341,7 @@ static void DecoderProcess( decoder_t *p_dec, block_t *p_block )
                 p_packetized_block->p_next = NULL;
 
                 DecoderDecode( p_dec, p_packetized_block );
-                if( p_dec->b_error )
+                if( p_owner->error )
                 {
                     block_ChainRelease( p_next );
                     return;
@@ -1363,7 +1368,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( p_owner->error )
         return;
 
     if( p_packetizer != NULL && p_packetizer->pf_flush != NULL )
@@ -1569,6 +1574,8 @@ static decoder_t * CreateDecoder( vlc_object_t *p_parent,
     p_owner->b_first = true;
     p_owner->b_has_data = false;
 
+    p_owner->error = false;
+
     p_owner->flushing = false;
     p_owner->b_draining = false;
     atomic_init( &p_owner->drained, false );
-- 
2.11.0



More information about the vlc-devel mailing list