[FFmpeg-devel,1/5] avformat/smacker: Read extradata directly into extradata

The Smacker demuxer reads four consecutive 32bit values from the file
header into its demux context (as four uint32_t), converting it to
native endianness in the process and then writing these four values
later (after extradata has been allocated) to extradata as four 32bit
values (converting to little endian in the process).

This commit changes this: The stream and the extradata are allocated
earlier, so that the data destined for extradata can be read directly
into extradata.

Furthermore, given that these values are not needed for demuxing itself
they are now no longer kept as part of the demuxing context.

Finally, a check regarding the number of frames has been moved up,
too, in order to exit early before unnecessarily allocating the
stream and the extradata.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/smacker.c | 47 +++++++++++++++++--------------------------
 1 file changed, 18 insertions(+), 29 deletions(-)

Andreas Rheinhardt:
> The Smacker demuxer reads four consecutive 32bit values from the file
> header into its demux context (as four uint32_t), converting it to
> native endianness in the process and then writing these four values
> later (after extradata has been allocated) to extradata as four 32bit
> values (converting to little endian in the process).
> 
> This commit changes this: The stream and the extradata are allocated
> earlier, so that the data destined for extradata can be read directly
> into extradata.
> 
> Furthermore, given that these values are not needed for demuxing itself
> they are now no longer kept as part of the demuxing context.
> 
> Finally, a check regarding the number of frames has been moved up,
> too, in order to exit early before unnecessarily allocating the
> stream and the extradata.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavformat/smacker.c | 47 +++++++++++++++++--------------------------
>  1 file changed, 18 insertions(+), 29 deletions(-)
> 
> diff --git a/libavformat/smacker.c b/libavformat/smacker.c
> index 6de0e7a0f1..4db3ec326f 100644
> --- a/libavformat/smacker.c
> +++ b/libavformat/smacker.c
> @@ -25,10 +25,10 @@
>  
>  #include <inttypes.h>
>  
> -#include "libavutil/bswap.h"
>  #include "libavutil/channel_layout.h"
>  #include "libavutil/intreadwrite.h"
>  #include "avformat.h"
> +#include "avio_internal.h"
>  #include "internal.h"
>  
>  #define SMACKER_PAL 0x01
> @@ -51,7 +51,6 @@ typedef struct SmackerContext {
>      uint32_t flags;
>      uint32_t audio[7];
>      uint32_t treesize;
> -    uint32_t mmap_size, mclr_size, full_size, type_size;
>      uint8_t  aflags[7];
>      uint32_t rates[7];
>      uint32_t pad;
> @@ -128,6 +127,10 @@ static int smacker_read_header(AVFormatContext *s)
>      smk->flags = avio_rl32(pb);
>      if(smk->flags & SMACKER_FLAG_RING_FRAME)
>          smk->frames++;
> +    if (smk->frames > 0xFFFFFF) {
> +        av_log(s, AV_LOG_ERROR, "Too many frames: %"PRIu32"\n", smk->frames);
> +        return AVERROR_INVALIDDATA;
> +    }
>      for(i = 0; i < 7; i++)
>          smk->audio[i] = avio_rl32(pb);
>      smk->treesize = avio_rl32(pb);
> @@ -137,21 +140,25 @@ static int smacker_read_header(AVFormatContext *s)
>          return AVERROR_INVALIDDATA;
>      }
>  
> -//FIXME remove extradata "rebuilding"
> -    smk->mmap_size = avio_rl32(pb);
> -    smk->mclr_size = avio_rl32(pb);
> -    smk->full_size = avio_rl32(pb);
> -    smk->type_size = avio_rl32(pb);
> +    st = avformat_new_stream(s, NULL);
> +    if (!st)
> +        return AVERROR(ENOMEM);
> +
> +    if ((ret = ff_alloc_extradata(st->codecpar, smk->treesize + 16)) < 0) {
> +        av_log(s, AV_LOG_ERROR,
> +               "Cannot allocate %"PRIu32" bytes of extradata\n",
> +               smk->treesize + 16);
> +        return ret;
> +    }
> +    if ((ret = ffio_read_size(pb, st->codecpar->extradata, 16)) < 0)
> +        return ret;
> +
>      for(i = 0; i < 7; i++) {
>          smk->rates[i]  = avio_rl24(pb);
>          smk->aflags[i] = avio_r8(pb);
>      }
>      smk->pad = avio_rl32(pb);
>      /* setup data */
> -    if(smk->frames > 0xFFFFFF) {
> -        av_log(s, AV_LOG_ERROR, "Too many frames: %"PRIu32"\n", smk->frames);
> -        return AVERROR_INVALIDDATA;
> -    }
>      smk->frm_size = av_malloc_array(smk->frames, sizeof(*smk->frm_size));
>      smk->frm_flags = av_malloc(smk->frames);
>      if (!smk->frm_size || !smk->frm_flags) {
> @@ -171,12 +178,6 @@ static int smacker_read_header(AVFormatContext *s)
>      }
>  
>      /* init video codec */
> -    st = avformat_new_stream(s, NULL);
> -    if (!st) {
> -        av_freep(&smk->frm_size);
> -        av_freep(&smk->frm_flags);
> -        return AVERROR(ENOMEM);
> -    }
>      smk->videoindex = st->index;
>      st->codecpar->width = smk->width;
>      st->codecpar->height = smk->height;
> @@ -233,24 +234,12 @@ static int smacker_read_header(AVFormatContext *s)
>  
>  
>      /* load trees to extradata, they will be unpacked by decoder */
> -    if ((ret = ff_alloc_extradata(st->codecpar, smk->treesize + 16)) < 0) {
> -        av_log(s, AV_LOG_ERROR,
> -               "Cannot allocate %"PRIu32" bytes of extradata\n",
> -               smk->treesize + 16);
> -        av_freep(&smk->frm_size);
> -        av_freep(&smk->frm_flags);
> -        return ret;
> -    }
>      ret = avio_read(pb, st->codecpar->extradata + 16, st->codecpar->extradata_size - 16);
>      if(ret != st->codecpar->extradata_size - 16){
>          av_freep(&smk->frm_size);
>          av_freep(&smk->frm_flags);
>          return AVERROR(EIO);
>      }
> -    ((int32_t*)st->codecpar->extradata)[0] = av_le2ne32(smk->mmap_size);
> -    ((int32_t*)st->codecpar->extradata)[1] = av_le2ne32(smk->mclr_size);
> -    ((int32_t*)st->codecpar->extradata)[2] = av_le2ne32(smk->full_size);
> -    ((int32_t*)st->codecpar->extradata)[3] = av_le2ne32(smk->type_size);
>  
>      smk->curstream = -1;
>      smk->nextpos = avio_tell(pb);
> 
Any comments on this patchset? If not, I'll apply it tomorrow.

- Andreas