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

[Kharkov Alexander]

Thanks for review, agree with all your points, will fix and resent.
Details:
* "keyframes" metatag does not stated in FLV specs but often
recommeded, I found at least one more free tool which inject it FLVMDI
* index data really can be free right after it is passed to av_add_index_entry
* code spread all over just because I want to touch exsting metadata
parser behavior minmal (all this recursion, loops, etc.) that is why I
did not create separate function which simply parse keyframes object

On 20 January 2011 23:23, Vladimir Pantelic <vladoman at gmail.com> wrote:
> Kharkov Alexander wrote:
>>
>> Hi,
>>
>> Attached patch parse flvtool2 metadata "keyframes
>> (times/fileopstions)" tags and use this data
>> as index at least for seek operations.
>> It is required in case when mplayer which use this part of ffmpeg code
>> attempts to play FLV file via HTTP for example.
>> Existing implementation attempt to download ALL content up to seek point
>> before it can play it, but it is unacceptable to download unneded data
>> (whole file if you seek to the end)
>> as in case of low bandwidth seek operation will 'never' end.
>> With this patch 'mplayer' can seek fast in such situations using this
>> index.
>
>> Index: libavformat/flvdec.c
>> ===================================================================
>> --- libavformat/flvdec.c ? ? ? ?(revision 26327)
>> +++ libavformat/flvdec.c ? ? ? ?(working copy)
>> @@ -30,10 +30,51 @@
>> ?#include "avformat.h"
>> ?#include "flv.h"
>>
>> +#define FLVTOOL2_KEYFRAMES_TAG "keyframes"
>> +#define FLVTOOL2_TIMESTAMP_TAG "times"
>> +#define FLVTOOL2_BYTEOFFSET_TAG "filepositions"
>
> is this really FLVTOOL2 specific? does it have to end up
> in the tag/variable name?
>
>> +
>> ?typedef struct {
>> + ? ?int64_t byte_offset;
>> + ? ?int64_t time_offset;
>> +} FLVTool2KeyframesIndex;
>
> dito
>
>> +typedef struct {
>> + ? ?int keyframes_object_parse;
>> +
>> + ? ?int filepositions_array_parse;
>> + ? ?int fill_filepositions;
>> + ? ?int num_filepositions_processed;
>> +
>> + ? ?int times_array_parse;
>> + ? ?int fill_timestamps;
>> + ? ?int num_timestamps_processed;
>> +
>> + ? ?int num_indexed_keyframes;
>> +
>> + ? ?FLVTool2KeyframesIndex *indexes;
>> +} FLVTool2Index;
>
> dito
>
>> +typedef struct {
>> ? ? ?int wrong_dts; ///< ?wrong dts due to negative cts
>> + ? ?FLVTool2Index *flvtool2_index;
>> ?} FLVContext;
>>
>> +static void init_flv_context(FLVContext* flv)
>> +{
>> + ? ?// initialize flvtool2 "keyframes" based index
>> + ? ?flv->flvtool2_index = av_malloc(sizeof(FLVTool2Index) * 1);
>> + ? ?flv->flvtool2_index->keyframes_object_parse = 0;
>> + ? ?flv->flvtool2_index->filepositions_array_parse = 0;
>> + ? ?flv->flvtool2_index->fill_filepositions = 0;
>> + ? ?flv->flvtool2_index->num_filepositions_processed = 0;
>> + ? ?flv->flvtool2_index->times_array_parse = 0;
>> + ? ?flv->flvtool2_index->fill_timestamps = 0;
>> + ? ?flv->flvtool2_index->num_timestamps_processed = 0;
>> + ? ?flv->flvtool2_index->num_indexed_keyframes = 0;
>> + ? ?flv->flvtool2_index->indexes = NULL;
>> +};
>
> av_mallocz...
>
>> +
>> ?static int flv_probe(AVProbeData *p)
>> ?{
>> ? ? ?const uint8_t *d;
>> @@ -130,6 +171,8 @@
>> ? ? ?AMFDataType amf_type;
>> ? ? ?char str_val[256];
>> ? ? ?double num_val;
>> + ? ?unsigned int index_iter;
>> + ? ?FLVContext *flv = s->priv_data;
>>
>> ? ? ?num_val = 0;
>> ? ? ?ioc = s->pb;
>> @@ -138,7 +181,16 @@
>>
>> ? ? ?switch(amf_type) {
>> ? ? ? ? ?case AMF_DATA_TYPE_NUMBER:
>> - ? ? ? ? ? ?num_val = av_int2dbl(get_be64(ioc)); break;
>> + ? ? ? ? ? ?num_val = av_int2dbl(get_be64(ioc));
>> + ? ? ? ? ? ?if (flv->flvtool2_index->times_array_parse&& ?depth == 3&&
>> + ? ? ? ? ? ? ? ?flv->flvtool2_index->num_timestamps_processed<
>> ?flv->flvtool2_index->num_indexed_keyframes) {
>> +
>> ?flv->flvtool2_index->indexes[flv->flvtool2_index->num_timestamps_processed++].time_offset
>> = num_val;
>> + ? ? ? ? ? ?}
>> + ? ? ? ? ? ?if (flv->flvtool2_index->filepositions_array_parse&& ?depth
>> == 3
>> +&& ?flv->flvtool2_index->num_filepositions_processed<
>> ?flv->flvtool2_index->num_indexed_keyframes) {
>> +
>> ?flv->flvtool2_index->indexes[flv->flvtool2_index->num_filepositions_processed++].byte_offset
>> = num_val;
>> + ? ? ? ? ? ?}
>> + ? ? ? ? ? ?break;
>> ? ? ? ? ?case AMF_DATA_TYPE_BOOL:
>> ? ? ? ? ? ? ?num_val = get_byte(ioc); break;
>> ? ? ? ? ?case AMF_DATA_TYPE_STRING:
>> @@ -149,9 +201,21 @@
>> ? ? ? ? ? ? ?unsigned int keylen;
>>
>> ? ? ? ? ? ? ?while(url_ftell(ioc)< ?max_pos - 2&& ?(keylen =
>> get_be16(ioc))) {
>> - ? ? ? ? ? ? ? ?url_fskip(ioc, keylen); //skip key string
>> + ? ? ? ? ? ? ? ?get_buffer(ioc, str_val, keylen);
>> + ? ? ? ? ? ? ? ?str_val[keylen] = '\0';
>> + ? ? ? ? ? ? ? ?if (flv->flvtool2_index->keyframes_object_parse&&
>> ?!strcmp(FLVTOOL2_TIMESTAMP_TAG, str_val)&& ?depth == 1) {
>> + ? ? ? ? ? ? ? ? ? ?flv->flvtool2_index->times_array_parse = 1;
>> + ? ? ? ? ? ? ? ?}
>> + ? ? ? ? ? ? ? ?if (flv->flvtool2_index->keyframes_object_parse&&
>> ?!strcmp(FLVTOOL2_BYTEOFFSET_TAG, str_val)&& ?depth == 1) {
>> + ? ? ? ? ? ? ? ? ? ?flv->flvtool2_index->filepositions_array_parse = 1;
>> + ? ? ? ? ? ? ? ?}
>> +
>> + ? ? ? ? ? ? ? ?//url_fskip(ioc, keylen); //skip key string
>> ? ? ? ? ? ? ? ? ?if(amf_parse_object(s, NULL, NULL, NULL, max_pos, depth +
>> 1)< ?0)
>> ? ? ? ? ? ? ? ? ? ? ?return -1; //if we couldn't skip, bomb out.
>> +
>> + ? ? ? ? ? ? ? ?flv->flvtool2_index->times_array_parse = 0;
>> + ? ? ? ? ? ? ? ?flv->flvtool2_index->filepositions_array_parse = 0;
>
>> ? ? ? ? ? ? ?}
>> ? ? ? ? ? ? ?if(get_byte(ioc) != AMF_END_OF_OBJECT)
>> ? ? ? ? ? ? ? ? ?return -1;
>> @@ -164,9 +228,14 @@
>> ? ? ? ? ?case AMF_DATA_TYPE_MIXEDARRAY:
>> ? ? ? ? ? ? ?url_fskip(ioc, 4); //skip 32-bit max array index
>> ? ? ? ? ? ? ?while(url_ftell(ioc)< ?max_pos - 2&& ?amf_get_string(ioc,
>> str_val, sizeof(str_val))> ?0) {
>> + ? ? ? ? ? ? ? ?if (!strcmp(FLVTOOL2_KEYFRAMES_TAG, str_val)&& ?depth ==
>> 0) {
>> + ? ? ? ? ? ? ? ? ? ? flv->flvtool2_index->keyframes_object_parse = 1;
>> + ? ? ? ? ? ? ? ?}
>> ? ? ? ? ? ? ? ? ?//this is the only case in which we would want a nested
>> parse to not skip over the object
>> ? ? ? ? ? ? ? ? ?if(amf_parse_object(s, astream, vstream, str_val,
>> max_pos, depth + 1)< ?0)
>> ? ? ? ? ? ? ? ? ? ? ?return -1;
>> +
>> + ? ? ? ? ? ? ? ?flv->flvtool2_index->keyframes_object_parse = 0;
>> ? ? ? ? ? ? ?}
>> ? ? ? ? ? ? ?if(get_byte(ioc) != AMF_END_OF_OBJECT)
>> ? ? ? ? ? ? ? ? ?return -1;
>> @@ -175,10 +244,28 @@
>> ? ? ? ? ? ? ?unsigned int arraylen, i;
>>
>> ? ? ? ? ? ? ?arraylen = get_be32(ioc);
>> + ? ? ? ? ? ?if (flv->flvtool2_index->filepositions_array_parse&& ?depth
>> == 2) {
>> + ? ? ? ? ? ? ? ?// allocate array to keep indexes
>> + ? ? ? ? ? ? ? ?if (flv->flvtool2_index->indexes == NULL) {
>> + ? ? ? ? ? ? ? ? ? ?flv->flvtool2_index->indexes =
>> av_malloc(sizeof(FLVTool2KeyframesIndex) * arraylen);
>> + ? ? ? ? ? ? ? ? ? ?flv->flvtool2_index->num_indexed_keyframes =
>> arraylen;
>> + ? ? ? ? ? ? ? ?}
>> + ? ? ? ? ? ? ? ?flv->flvtool2_index->fill_filepositions = 1;
>> + ? ? ? ? ? }
>> + ? ? ? ? ? ?if (flv->flvtool2_index->times_array_parse&& ?depth == 2) {
>> + ? ? ? ? ? ? ? ?// allocate array to keep indexes
>> + ? ? ? ? ? ? ? ?if (flv->flvtool2_index->indexes == NULL) {
>> + ? ? ? ? ? ? ? ? ? ?flv->flvtool2_index->indexes =
>> av_malloc(sizeof(FLVTool2KeyframesIndex) * arraylen);
>> + ? ? ? ? ? ? ? ? ? ?flv->flvtool2_index->num_indexed_keyframes =
>> arraylen;
>> + ? ? ? ? ? ? ? ?}
>> + ? ? ? ? ? ? ? ?flv->flvtool2_index->fill_timestamps = 1;
>> + ? ? ? ? ? }
>> ? ? ? ? ? ? ?for(i = 0; i< ?arraylen&& ?url_ftell(ioc)< ?max_pos - 1; i++)
>> {
>> ? ? ? ? ? ? ? ? ?if(amf_parse_object(s, NULL, NULL, NULL, max_pos, depth +
>> 1)< ?0)
>> ? ? ? ? ? ? ? ? ? ? ?return -1; //if we couldn't skip, bomb out.
>> ? ? ? ? ? ? ?}
>> + ? ? ? ? ? ?flv->flvtool2_index->fill_filepositions = 0;
>> + ? ? ? ? ? ?flv->flvtool2_index->fill_timestamps = 0;
>> ? ? ? ? ?}
>
> I don't like how this is spaced out all over the code, but I guess this more
> due to
> how the flv reader works...
>
>> ? ? ? ? ? ? ?break;
>> ? ? ? ? ?case AMF_DATA_TYPE_DATE:
>> @@ -205,6 +292,14 @@
>> ? ? ? ? ? ? ? ? ?acodec->bit_rate = num_val * 1024.0;
>> ? ? ? ? ?} else if (amf_type == AMF_DATA_TYPE_STRING)
>> ? ? ? ? ? ? ?av_metadata_set2(&s->metadata, key, str_val, 0);
>> +
>> + ? ? ? ?for (index_iter = 0; index_iter<
>> ?flv->flvtool2_index->num_indexed_keyframes; ++index_iter) {
>> + ? ? ? ? ? ?int64_t ts =
>> flv->flvtool2_index->indexes[index_iter].time_offset;
>> + ? ? ? ? ? ?int64_t byte_offset =
>> flv->flvtool2_index->indexes[index_iter].byte_offset;
>> + ? ? ? ? ? ?av_add_index_entry(vstream, byte_offset, ts*1000, 0, 0,
>> AVINDEX_KEYFRAME);
>> + ? ? ? ? ? ?av_log(s, AV_LOG_DEBUG,
>> + ? ? ? ? ? ? ? ?"add flvtool2 index entry: timestamp: %16"PRIX64", byte
>> offset: %16"PRIX64"\n", ts, byte_offset);
>> + ? ? ? ?}
>
> you could drop the indexes right after this step to free the mem, no?
>
>> ? ? ?}
>>
>> ? ? ?return 0;
>> @@ -214,8 +309,11 @@
>> ? ? ?AMFDataType type;
>> ? ? ?AVStream *stream, *astream, *vstream;
>> ? ? ?ByteIOContext *ioc;
>> +
>> ? ? ?int i;
>> ? ? ?char buffer[11]; //only needs to hold the string "onMetaData".
>> Anything longer is something we don't want.
>> + ? ?FLVContext *flv = s->priv_data;
>> + ? ?init_flv_context(flv);
>>
>> ? ? ?astream = NULL;
>> ? ? ?vstream = NULL;
>> @@ -491,6 +589,15 @@
>> ?}
>> ?#endif
>>
>> +static int flv_read_close(AVFormatContext *s)
>> +{
>> + ? ?FLVContext *flv = s->priv_data;
>> + ? ?if (flv->flvtool2_index->indexes != NULL)
>> + ? ? ? ?av_free(flv->flvtool2_index->indexes);
>> + ? ?av_free(flv->flvtool2_index);
>> + ? ?return 0;
>> +}
>> +
>> ?AVInputFormat flv_demuxer = {
>> ? ? ?"flv",
>> ? ? ?NULL_IF_CONFIG_SMALL("FLV format"),
>> @@ -498,6 +605,7 @@
>> ? ? ?flv_probe,
>> ? ? ?flv_read_header,
>> ? ? ?flv_read_packet,
>> + ? ?flv_read_close,
>> ? ? ?.read_seek = flv_read_seek,
>> ?#if 0
>> ? ? ?.read_seek2 = flv_read_seek2,
>>
>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at mplayerhq.hu
>> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>