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

[Jonne Ahner]

Added your local align things. The code2 thing seems to have been picked up
already.





On Tue, Sep 20, 2011 at 12:36 AM, Jonne Ahner <jonne.ahner at gmail.com> wrote:

> 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
>>
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.diff
Type: text/x-patch
Size: 5601 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110920/76cde539/attachment.bin>