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