On Fri, Jul 26, 2013 at 07:18:44PM +0200, Vittorio Giovara wrote:
> --- a/libavcodec/h264.c
> +++ b/libavcodec/h264.c
> @@ -2760,6 +2772,7 @@ static int field_end(H264Context *h, int in_setup)
>  {
>      AVCodecContext *const avctx = h->avctx;
>      int err = 0;
> +    int ret;
>      h->mb_y = 0;
>  
>      if (!in_setup && !h->droppable)
> @@ -2782,7 +2795,8 @@ static int field_end(H264Context *h, int in_setup)
>      }
>  
>      if (avctx->hwaccel) {
> -        if (avctx->hwaccel->end_frame(avctx) < 0)
> +        ret = avctx->hwaccel->end_frame(avctx);
> +        if (ret < 0)
>              av_log(avctx, AV_LOG_ERROR,
>                     "hardware accelerator failed to decode picture\n");
>      }

You're not returning ret, so this seems pointless.

> @@ -3015,7 +3029,7 @@ static int h264_slice_header_init(H264Context *h, int 
> reinit)
>      int nb_slices = (HAVE_THREADS &&
>                       h->avctx->active_thread_type & FF_THREAD_SLICE) ?
>                      h->avctx->thread_count : 1;
> -    int i;
> +    int i, ret;
>  
>      h->avctx->sample_aspect_ratio = h->sps.sar;
>      av_assert0(h->avctx->sample_aspect_ratio.den);
> @@ -3038,7 +3052,8 @@ static int h264_slice_header_init(H264Context *h, int 
> reinit)
>      h->prev_interlaced_frame = 1;
>  
>      init_scan_tables(h);
> -    if (ff_h264_alloc_tables(h) < 0) {
> +    ret = ff_h264_alloc_tables(h);
> +    if (ret < 0) {
>          av_log(h->avctx, AV_LOG_ERROR,
>                 "Could not allocate memory for h264\n");
>          return AVERROR(ENOMEM);

same

> @@ -3097,11 +3113,13 @@ static int h264_slice_header_init(H264Context *h, int 
> reinit)
>  
> -        for (i = 0; i < h->slice_context_count; i++)
> -            if (context_init(h->thread_context[i]) < 0) {
> +        for (i = 0; i < h->slice_context_count; i++) {
> +            ret = context_init(h->thread_context[i]);
> +            if (ret < 0) {
>                  av_log(h->avctx, AV_LOG_ERROR, "context_init() failed.\n");
> -                return -1;
> +                return ret;
>              }
> +        }
>      }

stray extra formatting change

> @@ -3128,6 +3146,7 @@ static int decode_slice_header(H264Context *h, 
> H264Context *h0)
>      int needs_reinit = 0;
> +    int field_pic_flag, bottom_field_flag;
>  
> @@ -3305,8 +3324,10 @@ static int decode_slice_header(H264Context *h, 
> H264Context *h0)
>      if (h->sps.frame_mbs_only_flag) {
>          h->picture_structure = PICT_FRAME;
>      } else {
> -        if (get_bits1(&h->gb)) { // field_pic_flag
> -            h->picture_structure = PICT_TOP_FIELD + get_bits1(&h->gb); // 
> bottom_field_flag
> +        field_pic_flag = get_bits1(&h->gb);
> +        if (field_pic_flag) {
> +            bottom_field_flag = get_bits1(&h->gb);
> +            h->picture_structure = PICT_TOP_FIELD + bottom_field_flag;
>          } else {
>              h->picture_structure = PICT_FRAME;
>              h->mb_aff_frame      = h->sps.mb_aff;

This does not look like a return value change to me.

> @@ -3588,12 +3612,13 @@ static int decode_slice_header(H264Context *h, 
> H264Context *h0)
>      // or h->mmco, which will cause ref list mix-ups and decoding errors
>      // further down the line. This may break decoding if the first slice is
>      // corrupt, thus we only do this if frame-mt is enabled.
> -    if (h->nal_ref_idc &&
> -        ff_h264_decode_ref_pic_marking(h0, &h->gb,
> +    if (h->nal_ref_idc) {
> +        ret = ff_h264_decode_ref_pic_marking(h0, &h->gb,
>                              !(h->avctx->active_thread_type & 
> FF_THREAD_FRAME) ||
> -                            h0->current_slice == 0) < 0 &&
> -        (h->avctx->err_recognition & AV_EF_EXPLODE))
> -        return AVERROR_INVALIDDATA;
> +                            h0->current_slice == 0);
> +        if (ret < 0 && (h->avctx->err_recognition & AV_EF_EXPLODE))
> +            return AVERROR_INVALIDDATA;
> +    }

Indentation is now off.

> @@ -4134,6 +4161,7 @@ static int decode_slice(struct AVCodecContext *avctx, 
> void *arg)
>  {
>      H264Context *h = *(void **)arg;
>      int lf_x_start = h->mb_x;
> +    int ret, eos;
>  
>      h->mb_skip_run = -1;
>  
> @@ -4155,8 +4183,7 @@ static int decode_slice(struct AVCodecContext *avctx, 
> void *arg)
>  
>          for (;;) {
>              // START_TIMER
> -            int ret = ff_h264_decode_mb_cabac(h);
> -            int eos;
> +            ret = ff_h264_decode_mb_cabac(h);
>              // STOP_TIMER("decode_mb_cabac")

Keep the "eos" declaration where it is; no need to increase its scope.

> @@ -4269,17 +4297,20 @@ static int decode_slice(struct AVCodecContext *avctx, 
> void *arg)
>  
> -            if (get_bits_left(&h->gb) <= 0 && h->mb_skip_run <= 0) {
> -                tprintf(h->avctx, "slice end %d %d\n",
> -                        get_bits_count(&h->gb), h->gb.size_in_bits);
> -                if (get_bits_left(&h->gb) == 0) {
> +            ret = get_bits_left(&h->gb);
> +            if (ret <= 0 && h->mb_skip_run <= 0) {
> +                ret = get_bits_count(&h->gb);
> +                tprintf(h->avctx, "slice end %d %d\n", ret, 
> h->gb.size_in_bits);
> +
> +                ret = get_bits_left(&h->gb);
> +                if (ret == 0) {
>                      er_add_slice(h, h->resync_mb_x, h->resync_mb_y,
>                                      h->mb_x - 1, h->mb_y,
>                                      ER_MB_END);
>                      if (h->mb_x > lf_x_start)
>                          loop_filter(h, lf_x_start, h->mb_x);
>  
> -                    return 0;
> +                    return ret;

I'm not sure this is an improvement.

> @@ -4361,7 +4392,7 @@ static int decode_nal_units(H264Context *h, const 
> uint8_t *buf, int buf_size,
>              int bit_length;
>              const uint8_t *ptr;
>              int i, nalsize = 0;
> -            int err;
> +            int err, ret;
>  
>              if (buf_index >= next_avc) {
>                  if (buf_index >= buf_size - h->nal_length_size)
> @@ -4442,7 +4473,8 @@ static int decode_nal_units(H264Context *h, const 
> uint8_t *buf, int buf_size,
>                  case NAL_IDR_SLICE:
>                  case NAL_SLICE:
>                      init_get_bits(&hx->gb, ptr, bit_length);
> -                    if (!get_ue_golomb(&hx->gb))
> +                    ret = get_ue_golomb(&hx->gb);
> +                    if (ret == 0)
>                          nals_needed = nal_index;
>                  }
>                  continue;
> @@ -4560,8 +4592,8 @@ again:
>                  break;
>              case NAL_SPS:
>                  init_get_bits(&h->gb, ptr, bit_length);
> -                if (ff_h264_decode_seq_parameter_set(h) < 0 &&
> -                    h->is_avc && (nalsize != consumed) && nalsize) {
> +                ret = ff_h264_decode_seq_parameter_set(h);
> +                if (ret < 0 && h->is_avc && (nalsize != consumed) && 
> nalsize) {
>                      av_log(h->avctx, AV_LOG_DEBUG,
>                             "SPS decoding failure, trying again with the 
> complete NAL\n");
>                      init_get_bits(&h->gb, buf + buf_index + 1 - consumed,
> @@ -4569,7 +4601,8 @@ again:
>                      ff_h264_decode_seq_parameter_set(h);
>                  }
>  
> -                if (h264_set_parameter_from_sps(h) < 0) {
> +                ret = h264_set_parameter_from_sps(h);
> +                if (ret < 0) {
>                      buf_index = -1;
>                      goto end;
>                  }

ret is not returned, so I fail to see the point..

Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to