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

[Kharkov Alexander]

All comments processed, more info inline.
On 17 February 2011 01:52, Anton Khirnov <anton at khirnov.net> wrote:
> First of all, could you please get rid of tabs and use git-formatted
> patches?

attached patch done via git diff, tabs removed

>
>> 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,97 @@
>> ? ? ?return length;
>> ?}
>>
>> +static int parse_keyframes_index(AVFormatContext *s, ByteIOContext *ioc, AVStream *vstream, int64_t max_pos) {
>> + ? ?unsigned int arraylen = 0, timeslen = 0, fileposlen = 0, i;
>> + ? ?double num_val;
>> + ? ?char str_val[256];
>> + ? ?int64_t *times = NULL;
>> + ? ?int64_t *filepositions = NULL;
>> +
>> + ? ?while (url_ftell(ioc) < max_pos - 2 && amf_get_string(ioc, str_val, sizeof(str_val)) > 0) {
>> + ? ? ? ?int64_t* current_array;
>> +
>> + ? ? ? ?// Expect array object in context
>> + ? ? ? ?if (get_byte(ioc) != AMF_DATA_TYPE_ARRAY) {
>> + ? ? ? ? ? ?av_freep(&times);
>> + ? ? ? ? ? ?av_freep(&filepositions);
>> + ? ? ? ? ? ?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(*times) * arraylen);
>
> This will leak memory if the index contains more than one array of each
> type. Change to realloc maybe?
No, I think unsutable metadata structure (two arrays which you point)
should be skipped and not used for indexing at all, as it is
suspicious that structure does not conform recommendations.
But I fix the mem leaks in cases when parser return because of
unsuitable metadata structure
>
> --
> Anton Khirnov
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iEYEARECAAYFAk1cKu4ACgkQtQoSQcBnB6uLogCdETgpAHrZc0C3uTNRqbQuBJKK
> aT8AniQkZ5ElBtPsnt8rPsnHsZl3p2jD
> =n/wX
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: flvdec.c.git.diff
Type: text/x-patch
Size: 5487 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110225/b1de7b66/attachment.bin>