[FFmpeg-devel] [patch] - fixes a few prores 4444 samples

Reimar Döffinger Reimar.Doeffinger at gmx.de
Mon Sep 19 22:13:12 CEST 2011


On Mon, Sep 19, 2011 at 09:01:26PM +0200, Jonne Ahner wrote:
> @@ -168,7 +168,11 @@ static int decode_frame_header(ProresContext *ctx, const uint8_t *buf,
>          ctx->frame.top_field_first = ctx->frame_type == 1;
>      }
>  
> -    avctx->pix_fmt = PIX_FMT_YUV422P10;
> +    if (avctx->codec_tag == MKTAG('a','p','4','h')) {
> +        avctx->pix_fmt = PIX_FMT_YUV444P10;
> +    } else {
> +        avctx->pix_fmt = PIX_FMT_YUV422P10;
> +    }

codec tag is a really bad way to distinguish those.
If there's no other way to distinguish them, e.g. at runtime,
a different codec ID should be used (IMO).

> +static void decode_slice_chroma444(AVCodecContext *avctx, SliceContext *slice,
> +                              uint8_t *dst, int dst_stride,
> +                              const uint8_t *buf, unsigned buf_size,
> +                              const int *qmat)
> +{
> +    ProresContext *ctx = avctx->priv_data;
> +    DECLARE_ALIGNED(16, DCTELEM, blocks)[8*4*64], *block;

I see that is already in the current code, but...
I think DECLARE_ALIGNED is just wrong, it has to be LOCAL_ALIGNED_16.
Also with 4kB size IMO it would be better if it was not on the stack at all.

> @@ -491,9 +523,15 @@ static int decode_slice_thread(AVCodecContext *avctx, void *arg, int jobnr, int
>          chroma_stride = pic->linesize[1] << 1;
>      }
>  
> -    dest_y = pic->data[0] + (slice->mb_y << 4) * luma_stride + (slice->mb_x << 5);
> -    dest_u = pic->data[1] + (slice->mb_y << 4) * chroma_stride + (slice->mb_x << 4);
> -    dest_v = pic->data[2] + (slice->mb_y << 4) * chroma_stride + (slice->mb_x << 4);
> +    if (avctx->pix_fmt == PIX_FMT_YUV444P10) {
> +        dest_y = pic->data[0] + (slice->mb_y << 4) * luma_stride + (slice->mb_x << 5);
> +        dest_u = pic->data[1] + (slice->mb_y << 4) * chroma_stride + (slice->mb_x << 5);
> +        dest_v = pic->data[2] + (slice->mb_y << 4) * chroma_stride + (slice->mb_x << 5);
> +    } else {
> +        dest_y = pic->data[0] + (slice->mb_y << 4) * luma_stride + (slice->mb_x << 5);
> +        dest_u = pic->data[1] + (slice->mb_y << 4) * chroma_stride + (slice->mb_x << 4);
> +        dest_v = pic->data[2] + (slice->mb_y << 4) * chroma_stride + (slice->mb_x << 4);
> +    }

You should probably avoid the duplication, using << (5 - chroma_h_shift) or
such.

> @@ -539,12 +585,15 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *data_size,
>      int buf_size = avpkt->size;
>      int frame_hdr_size, pic_size;
>  
> -    if (buf_size < 28 || buf_size != AV_RB32(buf) ||
> -        AV_RL32(buf +  4) != AV_RL32("icpf")) {
> +    if (buf_size < 28 || AV_RL32(buf +  4) != AV_RL32("icpf")) {
>          av_log(avctx, AV_LOG_ERROR, "invalid frame header\n");
>          return -1;
>      }
>  
> +    if (buf_size != AV_RB32(buf)) {
> +        av_log(avctx, AV_LOG_INFO, "buf_size != frame size (difference: %d bytes) \n", abs(AV_RB32(buf) - buf_size));
> +    }

Why? Also I don't think the abs() is really helping.


More information about the ffmpeg-devel mailing list