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

[Reimar Döffinger]

I don't have much useful to comment, just a few minor things:

On Tue, Sep 20, 2011 at 12:03:53AM +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 ((buf[12] & 0xC0) == 0xC0) {
> +        avctx->pix_fmt = PIX_FMT_YUV444P10;
> +    } else {
> +        avctx->pix_fmt = PIX_FMT_YUV422P10;
> +    }

Would be good to make a single file with both formats in one and check
that the decoder does not crash or something when switching.
>From a cosmetic standpoint, ?: could be used, but it might look
a bit craped.


> @@ -491,9 +524,11 @@ static int decode_slice_thread(AVCodecContext *avctx, void *arg, int jobnr, int
>          chroma_stride = pic->linesize[1] << 1;
>      }
>  
> +    chroma_h_shift = (avctx->pix_fmt == PIX_FMT_YUV444P10) ? 0 : 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);
> +    dest_u = pic->data[1] + (slice->mb_y << 4) * chroma_stride + (slice->mb_x << (5 - chroma_h_shift));
> +    dest_v = pic->data[2] + (slice->mb_y << 4) * chroma_stride + (slice->mb_x << (5 - chroma_h_shift));

Ah, I thought chroma_h_shift would already be available somewhere.
If not just going with
mb_x_shift = avctx->pix_fmt == PIX_FMT_YUV444P10 ? 5 : 4;
might be more readable.
(suggesting x, because unfortunately h could be either horizontal
or height, which makes it confusing to use).

> @@ -539,8 +582,7 @@ 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")) {

Well, I am still quite curious as to what AV_RB32(buf) then indicates
and in particular why this check should work for 422 but not 444...