[Ffmpeg-devel] [PATCH] x264 avc encoding, movenc avcC, ctts

[Luca Abeni]

Hi Baptiste,

I do not know anything about .mov files and about the MOV muxer, but I
think the "codec" part of your patches (x264_avc_nal_encode.patch) might
be incorrect:

On Sun, 2006-02-19 at 18:23 +0100, Baptiste COUDURIER wrote:
[...]
> x264_avc_nal_encode.patch
> 
> which encodes nals correctly for AVC and write extradata for avcC atom.
BTW, what is AVC? I was under the impression that AVC (Advanced Video
Coding) = MPEG4 Part 10 = H.264... So, I cannot understand the "encode
nals correctly for AVC" part
[...] 
> +static int
> +avc_encode_nals(uint8_t *buf, int size, x264_nal_t *nals, int nnal)
> +{
> +    uint8_t *p = buf;
> +    int i;
> +
> +    for(i = 0; i < nnal; i++){
> +        int s, s_be;
> +        size -= 4; /* nal size length is set to 4 in mov_write_avcc */
> +        p += 4; /* advance to stock size */
> +        s = x264_nal_encode(p, &size, 0, nals + i);
> ... 
>  static int
>  encode_nals(uint8_t *buf, int size, x264_nal_t *nals, int nnal)
> @@ -87,7 +112,10 @@ X264_frame(AVCodecContext *ctx, uint8_t 
>      if(x264_encoder_encode(x4->enc, &nal, &nnal, &x4->pic, &pic_out))
>          return -1;
>  
> -    bufsize = encode_nals(buf, bufsize, nal, nnal);
> +    if(ctx->flags & CODEC_FLAG_GLOBAL_HEADER)
> +        bufsize = avc_encode_nals(buf, bufsize, nal, nnal);
> +    else
> +        bufsize = encode_nals(buf, bufsize, nal, nnal);
>      if(bufsize < 0)
>          return -1;
This breaks applications using extradata. Why do you need to change the
extradata format? I am not really an expert, but I think extradata
should be the "global headers", and according to some drafts I read the
current content of extradata correctly matches H.264 global headers.

Also, maybe I am wrong, but I was under the impression that
CODEC_FLAG_GLOBAL_HEADER should not change the format of the stream, but
should simply move "out of band" some parts of the stream (the global
headers, in this case). Is this wrong? Can someone of the ffmpeg
developers clarify this?

[...]
> +        /* pps */
> +        p += 2; /* advance to stock size */
> +        s -= 2;
> +        size = x264_nal_encode(p, &s, 0, &nal[2]);
> +        if (size < 0)
> +            return -1;
> +        /* write size */
> +        p -= 2;
> +        size_be = BE_16(&size);
In any case, if the patch is committed this "p += 2; .... p -= 2;" thing
should be avoided (maybe something like
x264_nal_encode(p+2, &s, 0, &nal[2]);... memcpy(p, &size_be, 2); p += size + 2;...)

Moreover, I think you are opencoding the for() loop on the NALs... Why?
Also, it seems that you are not putting the first NAL in the extradata?


Again, I apologize if I am misunderstanding something, but my impression
is that your patch does at codec level something that should be done at
format level... I see "avcC" cited in your libavcodec/x264.c patch, but
I think it really is a MOV thing? Or am I wrong?


				Luca
-- 
_____________________________________________________________________________
Copy this in your signature, if you think it is important:
                               N O    W A R ! ! !

 
 
 --
 Email.it, the professional e-mail, gratis per te: http://www.email.it/f
 
 Sponsor:
 DA NON PERDERE: Prova gratis per 15 giorni DVDNet Rent e ricevi a casa tua i migliori film!
* 
 Clicca qui: http://adv.email.it/cgi-bin/foclick.cgi?mid=4634&d=20-2