[FFmpeg-devel] [PATCH] move avc sps/pps generation in a function in movenc

[Baptiste Coudurier]

Hi

Aurelien Jacobs wrote:
> Hi,
> 
> The attached patch moves the avc sps/pps generation code in a function.
> The final goal is to reuse this function from matroskaenc (and so the
> next step will obviously be to move this func in its own file).
> Is this patch OK ?
> 
> Aurel
> 
> 
> ------------------------------------------------------------------------
> 
> Index: libavformat/movenc.c
> ===================================================================
> --- libavformat/movenc.c	(revision 11447)
> +++ libavformat/movenc.c	(working copy)
> @@ -452,10 +452,10 @@
>      return end + 3;
>  }
>  
> -static int avc_parse_nal_units(uint8_t **buf, int *size)
> +static int avc_parse_nal_units(uint8_t *buf_in, uint8_t **buf, int *size)

Is changing the prototype really necessary ?

>  {
>      ByteIOContext *pb;
> -    uint8_t *p = *buf;
> +    uint8_t *p = buf_in;
>      uint8_t *end = p + *size;
>      uint8_t *nal_start, *nal_end;
>      int ret = url_open_dyn_buf(&pb);
> @@ -475,22 +475,20 @@
>      return 0;
>  }
>  
> -static int mov_write_avcc_tag(ByteIOContext *pb, MOVTrack *track)
> +static int avc_write_sps_pps(ByteIOContext *pb, uint8_t *data, int len)

This specific formating was defined by iso and meant to be put in 'avcC'
atom in iso media, loosing this information would be sad IMHO, I'd
prefer something like isom_write_avcc.

> [...]
>
> +            int ret = avc_parse_nal_units(data, &buf, &len);
> +            if (ret < 0)
> +                return ret;

IMHO unrelated to the patch and there is another occurence which would
need to be checked.

> -            avc_parse_nal_units(&track->vosData, &track->vosLen);
> -            buf = track->vosData;
> -            end = track->vosData + track->vosLen;
> +            data = buf;

I don't think this last line is needed.

>  [...]
>  
>              /* look for sps and pps */
>              while (buf < end) {
> @@ -522,10 +520,21 @@
>              put_byte(pb, 1); /* number of pps */
>              put_be16(pb, pps_size);
>              put_buffer(pb, pps, pps_size);
> +            av_free(data);

av_free(buf) should be more clear since buf was allocated, not data,
data is parameter, this is confusing.

[...]

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
SMARTJOG S.A.                                    http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
Phone: +33 1 49966312