[04/17] h264: Make it possible to compile without error_resilience

Message ID 1362578206-2806-4-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö March 6, 2013, 1:56 p.m.
From: "Ronald S. Bultje" <rsbultje@gmail.com>

Error resilience is enabled and used if automatically enabled
by other components built in.
---
 configure         |    4 ++--
 libavcodec/h264.c |   26 ++++++++++++++++++--------
 2 files changed, 20 insertions(+), 10 deletions(-)

Comments

Luca Barbato March 6, 2013, 2:38 p.m. | #1
On 06/03/13 14:56, Martin Storsjö wrote:
> From: "Ronald S. Bultje" <rsbultje@gmail.com>
> 
> Error resilience is enabled and used if automatically enabled
> by other components built in.
> ---
>  configure         |    4 ++--
>  libavcodec/h264.c |   26 ++++++++++++++++++--------
>  2 files changed, 20 insertions(+), 10 deletions(-)
> 

I would make er enabled by default, beside that the patch doesn't look
wrong (I'll follow up with a reformatting patch if one isn't available
once the evil plan lands)

lu
Anton Khirnov March 6, 2013, 3:46 p.m. | #2
On Wed,  6 Mar 2013 15:56:33 +0200, Martin Storsjö <martin@martin.st> wrote:
> From: "Ronald S. Bultje" <rsbultje@gmail.com>
> 
> Error resilience is enabled and used if automatically enabled
> by other components built in.
> ---
>  configure         |    4 ++--
>  libavcodec/h264.c |   26 ++++++++++++++++++--------
>  2 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/configure b/configure
> index 0aed51d..2b9d679 100755
> --- a/configure
> +++ b/configure
> @@ -1542,7 +1542,7 @@ h263_decoder_select="error_resilience h263_parser mpegvideo"
>  h263_encoder_select="aandcttables error_resilience mpegvideoenc"
>  h263i_decoder_select="h263_decoder"
>  h263p_encoder_select="h263_encoder"
> -h264_decoder_select="error_resilience golomb h264chroma h264dsp h264pred h264qpel videodsp"
> +h264_decoder_select="golomb h264chroma h264dsp h264pred h264qpel videodsp"
>  huffyuv_encoder_select="huffman"
>  iac_decoder_select="fft mdct sinewin"
>  imc_decoder_select="fft mdct sinewin"
> @@ -1689,7 +1689,7 @@ wmv3_vdpau_decoder_select="vc1_vdpau_decoder"
>  wmv3_vdpau_hwaccel_select="vc1_vdpau_hwaccel"
>  
>  # parsers
> -h264_parser_select="error_resilience golomb h264chroma h264dsp h264pred h264qpel videodsp"
> +h264_parser_select="golomb h264chroma h264dsp h264pred h264qpel videodsp"
>  mpeg4video_parser_select="error_resilience mpegvideo"
>  mpegvideo_parser_select="error_resilience mpegvideo"
>  vc1_parser_select="error_resilience mpegvideo"
> diff --git a/libavcodec/h264.c b/libavcodec/h264.c
> index 0b43248..c11207a 100644
> --- a/libavcodec/h264.c
> +++ b/libavcodec/h264.c
> @@ -1237,6 +1237,7 @@ static int context_init(H264Context *h)
>      h->ref_cache[1][scan8[7]  + 1] =
>      h->ref_cache[1][scan8[13] + 1] = PART_NOT_AVAILABLE;
>  
> +    if (CONFIG_ERROR_RESILIENCE) {
>      /* init ER */
>      er->avctx          = h->avctx;
>      er->dsp            = &h->dsp;
> @@ -1276,6 +1277,7 @@ static int context_init(H264Context *h)
>      er->dc_val[2] = er->dc_val[1] + c_size;
>      for (i = 0; i < yc_size; i++)
>          h->dc_val_base[i] = 1024;
> +    }
>  

Missing reindent (unless i failed to notice a subsequent patch for this)

>      return 0;
>  
> @@ -1684,7 +1686,9 @@ int ff_h264_frame_start(H264Context *h)
>      h->cur_pic     = *h->cur_pic_ptr;
>      h->cur_pic.f.extended_data = h->cur_pic.f.data;
>  
> -    ff_er_frame_start(&h->er);
> +    if (CONFIG_ERROR_RESILIENCE) {
> +        ff_er_frame_start(&h->er);
> +    }
>  
>      assert(h->linesize && h->uvlinesize);
>  
> @@ -2722,7 +2726,7 @@ static int field_end(H264Context *h, int in_setup)
>       * past end by one (callers fault) and resync_mb_y != 0
>       * causes problems for the first MB line, too.
>       */
> -    if (!FIELD_PICTURE) {
> +    if (CONFIG_ERROR_RESILIENCE && !FIELD_PICTURE) {
>          h->er.cur_pic  = h->cur_pic_ptr;
>          h->er.last_pic = h->ref_count[0] ? &h->ref_list[0][0] : NULL;
>          h->er.next_pic = h->ref_count[1] ? &h->ref_list[1][0] : NULL;
> @@ -4021,10 +4025,12 @@ static void decode_finish_row(H264Context *h)
>  static void er_add_slice(H264Context *h, int startx, int starty,
>                           int endx, int endy, int status)
>  {
> -    ERContext *er = &h->er;
> +    if (CONFIG_ERROR_RESILIENCE) {
> +        ERContext *er = &h->er;
>  
> -    er->ref_count = h->ref_count[0];
> -    ff_er_add_slice(er, startx, starty, endx, endy, status);
> +        er->ref_count = h->ref_count[0];
> +        ff_er_add_slice(er, startx, starty, endx, endy, status);
> +    }

I'd prefer an #if construct here, it saves us one indentation level and is more
correct i think

>  }
>  
>  static int decode_slice(struct AVCodecContext *avctx, void *arg)
> @@ -4208,7 +4214,9 @@ static int execute_decode_slices(H264Context *h, int context_count)
>      } else {
>          for (i = 1; i < context_count; i++) {
>              hx                    = h->thread_context[i];
> -            hx->er.error_count  = 0;
> +            if (CONFIG_ERROR_RESILIENCE) {
> +                hx->er.error_count = 0;
> +            }

Unnecessary change, we only care that the nonexisting ER functions aren't called

>          }
>  
>          avctx->execute(avctx, decode_slice, h->thread_context,
> @@ -4220,8 +4228,10 @@ static int execute_decode_slices(H264Context *h, int context_count)
>          h->mb_y              = hx->mb_y;
>          h->droppable         = hx->droppable;
>          h->picture_structure = hx->picture_structure;
> -        for (i = 1; i < context_count; i++)
> -            h->er.error_count += h->thread_context[i]->er.error_count;
> +        if (CONFIG_ERROR_RESILIENCE) {
> +            for (i = 1; i < context_count; i++)
> +                h->er.error_count += h->thread_context[i]->er.error_count;
> +        }
>      }

same
Diego Biurrun March 6, 2013, 4:51 p.m. | #3
On Wed, Mar 06, 2013 at 03:56:33PM +0200, Martin Storsjö wrote:
> From: "Ronald S. Bultje" <rsbultje@gmail.com>
> 
> Error resilience is enabled and used if automatically enabled
> by other components built in.
> ---
>  configure         |    4 ++--
>  libavcodec/h264.c |   26 ++++++++++++++++++--------
>  2 files changed, 20 insertions(+), 10 deletions(-)

What's the point as long as ER is not command line selectable?
It will be enabled seemingly at random depending on the other
codecs that are part of the build.

Diego
Martin Storsjö March 6, 2013, 4:56 p.m. | #4
On Wed, 6 Mar 2013, Diego Biurrun wrote:

> On Wed, Mar 06, 2013 at 03:56:33PM +0200, Martin Storsjö wrote:
>> From: "Ronald S. Bultje" <rsbultje@gmail.com>
>>
>> Error resilience is enabled and used if automatically enabled
>> by other components built in.
>> ---
>>  configure         |    4 ++--
>>  libavcodec/h264.c |   26 ++++++++++++++++++--------
>>  2 files changed, 20 insertions(+), 10 deletions(-)
>
> What's the point as long as ER is not command line selectable?
> It will be enabled seemingly at random depending on the other
> codecs that are part of the build.

The point is shrinking the h264-decoder-only builds. (I assume the final 
target is to exclude ER from chrome builds, to have less code to audit.)

// Martin

Patch

diff --git a/configure b/configure
index 0aed51d..2b9d679 100755
--- a/configure
+++ b/configure
@@ -1542,7 +1542,7 @@  h263_decoder_select="error_resilience h263_parser mpegvideo"
 h263_encoder_select="aandcttables error_resilience mpegvideoenc"
 h263i_decoder_select="h263_decoder"
 h263p_encoder_select="h263_encoder"
-h264_decoder_select="error_resilience golomb h264chroma h264dsp h264pred h264qpel videodsp"
+h264_decoder_select="golomb h264chroma h264dsp h264pred h264qpel videodsp"
 huffyuv_encoder_select="huffman"
 iac_decoder_select="fft mdct sinewin"
 imc_decoder_select="fft mdct sinewin"
@@ -1689,7 +1689,7 @@  wmv3_vdpau_decoder_select="vc1_vdpau_decoder"
 wmv3_vdpau_hwaccel_select="vc1_vdpau_hwaccel"
 
 # parsers
-h264_parser_select="error_resilience golomb h264chroma h264dsp h264pred h264qpel videodsp"
+h264_parser_select="golomb h264chroma h264dsp h264pred h264qpel videodsp"
 mpeg4video_parser_select="error_resilience mpegvideo"
 mpegvideo_parser_select="error_resilience mpegvideo"
 vc1_parser_select="error_resilience mpegvideo"
diff --git a/libavcodec/h264.c b/libavcodec/h264.c
index 0b43248..c11207a 100644
--- a/libavcodec/h264.c
+++ b/libavcodec/h264.c
@@ -1237,6 +1237,7 @@  static int context_init(H264Context *h)
     h->ref_cache[1][scan8[7]  + 1] =
     h->ref_cache[1][scan8[13] + 1] = PART_NOT_AVAILABLE;
 
+    if (CONFIG_ERROR_RESILIENCE) {
     /* init ER */
     er->avctx          = h->avctx;
     er->dsp            = &h->dsp;
@@ -1276,6 +1277,7 @@  static int context_init(H264Context *h)
     er->dc_val[2] = er->dc_val[1] + c_size;
     for (i = 0; i < yc_size; i++)
         h->dc_val_base[i] = 1024;
+    }
 
     return 0;
 
@@ -1684,7 +1686,9 @@  int ff_h264_frame_start(H264Context *h)
     h->cur_pic     = *h->cur_pic_ptr;
     h->cur_pic.f.extended_data = h->cur_pic.f.data;
 
-    ff_er_frame_start(&h->er);
+    if (CONFIG_ERROR_RESILIENCE) {
+        ff_er_frame_start(&h->er);
+    }
 
     assert(h->linesize && h->uvlinesize);
 
@@ -2722,7 +2726,7 @@  static int field_end(H264Context *h, int in_setup)
      * past end by one (callers fault) and resync_mb_y != 0
      * causes problems for the first MB line, too.
      */
-    if (!FIELD_PICTURE) {
+    if (CONFIG_ERROR_RESILIENCE && !FIELD_PICTURE) {
         h->er.cur_pic  = h->cur_pic_ptr;
         h->er.last_pic = h->ref_count[0] ? &h->ref_list[0][0] : NULL;
         h->er.next_pic = h->ref_count[1] ? &h->ref_list[1][0] : NULL;
@@ -4021,10 +4025,12 @@  static void decode_finish_row(H264Context *h)
 static void er_add_slice(H264Context *h, int startx, int starty,
                          int endx, int endy, int status)
 {
-    ERContext *er = &h->er;
+    if (CONFIG_ERROR_RESILIENCE) {
+        ERContext *er = &h->er;
 
-    er->ref_count = h->ref_count[0];
-    ff_er_add_slice(er, startx, starty, endx, endy, status);
+        er->ref_count = h->ref_count[0];
+        ff_er_add_slice(er, startx, starty, endx, endy, status);
+    }
 }
 
 static int decode_slice(struct AVCodecContext *avctx, void *arg)
@@ -4208,7 +4214,9 @@  static int execute_decode_slices(H264Context *h, int context_count)
     } else {
         for (i = 1; i < context_count; i++) {
             hx                    = h->thread_context[i];
-            hx->er.error_count  = 0;
+            if (CONFIG_ERROR_RESILIENCE) {
+                hx->er.error_count = 0;
+            }
         }
 
         avctx->execute(avctx, decode_slice, h->thread_context,
@@ -4220,8 +4228,10 @@  static int execute_decode_slices(H264Context *h, int context_count)
         h->mb_y              = hx->mb_y;
         h->droppable         = hx->droppable;
         h->picture_structure = hx->picture_structure;
-        for (i = 1; i < context_count; i++)
-            h->er.error_count += h->thread_context[i]->er.error_count;
+        if (CONFIG_ERROR_RESILIENCE) {
+            for (i = 1; i < context_count; i++)
+                h->er.error_count += h->thread_context[i]->er.error_count;
+        }
     }
 
     return 0;