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

Jonne Ahner jonne.ahner at gmail.com
Tue Sep 20 00:36:59 CEST 2011


On Tue, Sep 20, 2011 at 12:20 AM, Reimar Döffinger <Reimar.Doeffinger at gmx.de
> wrote:

> 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.
>
>
> Yes that would be a good test. I think it would work but good for eventual
future changes to the decoder.
Is any of the sample repos available for upload?

And yes I though it looked weird to.


> @@ -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).
>
> Agreed, better.



> > @@ -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...
> ___________
>

I dont know. This is an issue with just certain 444 files. It might be
related to log-c encoded sample values?
It works and I am just happy that it does really.
This might be an extension in the fileformat version 0 -> 1

It it not constant across frames.




> ____________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


More information about the ffmpeg-devel mailing list