[FFmpeg-devel] [PATCH] support for flvtool2 "keyframes based" generated index in FLV format decoder

Anton Khirnov anton
Wed Feb 9 20:15:03 CET 2011


> Index: libavformat/flvdec.c
> ===================================================================
> --- libavformat/flvdec.c	(revision 26402)
> +++ libavformat/flvdec.c	(working copy)
> @@ -30,6 +30,10 @@
>  #include "avformat.h"
>  #include "flv.h"
>  
> +#define METADATA_KEYFRAMES_TAG "keyframes"
> +#define METADATA_KEYFRAMES_TIMESTAMP_TAG "times"
> +#define METADATA_KEYFRAMES_BYTEOFFSET_TAG "filepositions"
> +
>  typedef struct {
>      int wrong_dts; ///< wrong dts due to negative cts
>  } FLVContext;
> @@ -124,6 +128,63 @@
>      return length;
>  }
>  
> +static int try_parse_keyframes_index(AVFormatContext *s, ByteIOContext *ioc, AVStream *vstream, int64_t max_pos) {

nit: the 'try_' part carries no information, get rid of it.

> +    unsigned int keylen, arraylen = 0, timeslen = 0, fileposlen = 0, i;
> +    double num_val;
> +    AMFDataType amf_type;
> +    char str_val[256];
> +    int64_t *times = 0;
> +    int64_t *filepositions = 0;

You mean NULL

> +
> +    while (url_ftell(ioc) < max_pos - 2 && (keylen = get_be16(ioc))) {
> +        int64_t* current_array;
> +        // Read array name from context
> +        get_buffer(ioc, str_val, keylen);
> +        str_val[keylen] = '\0';

Buffer overflow.

> +
> +        // Expect array object in context
> +        amf_type = get_byte(ioc);
> +        if (amf_type != AMF_DATA_TYPE_ARRAY)

This value isn't used anywhere else, so you can use if (get_byte() ...)
and get rid of amf_type. Same below.

> +            return -1;
> +
> +        arraylen = get_be32(ioc);
> +        /*
> +         * Expect only 'times' or 'filepositions' sub-arrays in other case refuse to use such metadata
> +         * for indexing
> +         */
> +        if (!strcmp(METADATA_KEYFRAMES_TIMESTAMP_TAG, str_val)) {
> +            times = av_mallocz(sizeof(int64_t) * arraylen);

Use sizeof(*times). Also you should check that malloc succeeded (it
might easily fail with a corrupted file).

> +            timeslen = arraylen;
> +            current_array = times;
> +        } else if (!strcmp(METADATA_KEYFRAMES_BYTEOFFSET_TAG, str_val)) {
> +            filepositions = av_mallocz(sizeof(int64_t) * arraylen);
> +            fileposlen = arraylen;
> +            current_array = filepositions;
> +        } else
> +            // unexpected metatag inside keyframes, will not use such metadata for indexing
> +            return -1;
> +
> +        for(i = 0; i < arraylen && url_ftell(ioc) < max_pos - 1; i++) {
> +            amf_type = get_byte(ioc);
> +            if (amf_type != AMF_DATA_TYPE_NUMBER)
> +                return -1;

Memleak. Same below.

> +            num_val = av_int2dbl(get_be64(ioc));
> +            current_array[i] = num_val;
> +        }
> +    }
> +    if(timeslen != fileposlen)
> +        // times->filepositions arrays have different size, will not use such metadata for indexing
> +        return -1;
> +
> +    for(i = 0; i < arraylen; ++i) {
> +        av_add_index_entry(vstream, filepositions[i], times[i]*1000, 0, 0, AVINDEX_KEYFRAME);
> +    }
> +
> +    av_freep(&times);
> +    av_freep(&filepositions);
> +    return 0;
> +}
> +
>  static int amf_parse_object(AVFormatContext *s, AVStream *astream, AVStream *vstream, const char *key, int64_t max_pos, int depth) {
>      AVCodecContext *acodec, *vcodec;
>      ByteIOContext *ioc;
> @@ -148,6 +209,17 @@
>          case AMF_DATA_TYPE_OBJECT: {
>              unsigned int keylen;
>  
> +            if (!strcmp(METADATA_KEYFRAMES_TAG, key) && depth == 1) {
> +                int64_t cur_ioc_pos = url_ftell(ioc);
> +                /*
> +                 * Try parse keyframes metatag and fill AV index.
> +                 * Reset stream to initial state after that to run default parser
> +                 * to not break default parser behavior
> +                 */
> +                if(try_parse_keyframes_index(s, ioc, vstream, max_pos) < 0)
> +                    url_fseek(ioc, cur_ioc_pos, SEEK_SET);
> +            }
> +
>              while(url_ftell(ioc) < max_pos - 2 && (keylen = get_be16(ioc))) {
>                  url_fskip(ioc, keylen); //skip key string
>                  if(amf_parse_object(s, NULL, NULL, NULL, max_pos, depth + 1) < 0)

-- 
Anton Khirnov
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110209/913191d7/attachment.pgp>



More information about the ffmpeg-devel mailing list