[FFmpeg-devel] [PATCH] Playlist API

[Geza Kovacs]

On 08/24/2009 03:54 AM, Michael Niedermayer wrote:
>> Alternatively, if you meant store an array of sums, that would work for
>> seeking backwards,
> 
> that was what i meant
> 

Ok, switched to summed durations array.

> my question is still the same, what is the point of a dozen functions over
> a single one adding the entries of a list into a playlist.
> such function could add a single file, it could add "file1,file2,..."
> it could add them to an existing context and could start with NULL as
> context and return a newly created one.
> This does seem simpler to me, maybe it is not but you arent really
> saying why you think so if you do.
> 

Ok, av_playlist_add_filelist and av_playlist_add_path should now do
that. Since the return value is used for the error code, the newly
allocated AVFormatContext is accessible via
ctx->formatcontext_list[ctx->pelist_size-1].

I'm opting for having av_playlist_alloc_concat_formatcontext allocate
the master AVFormatContext and AVPlaylistContext beforehand since I
don't want to have to check for null AVPlaylistContext every time it's
used (and since it allows for ff_concat_alloc_demuxer to be kept outside
the public headers, since it's only used when constructing a master
AVFormatContext).

===Comments from other email===

>> +AVFormatContext *av_playlist_alloc_formatcontext(char *filename)
>> +{
>> +    int err;
>> +    AVFormatContext *ic = avformat_alloc_context();
>> +    err = av_open_input_file(&ic, filename, ic->iformat, 0, NULL);
>> +    if (err < 0) {
>> +        av_log(ic, AV_LOG_ERROR, "Error during av_open_input_file\n");
>> +        av_free(ic);
>> +        return NULL;
>> +    }
>> +    err = av_find_stream_info(ic);
>> +    if (err < 0) {
>> +        av_log(ic, AV_LOG_ERROR, "Could not find stream info\n");
>> +        av_free(ic);
>> +        return NULL;
>> +    }
>
> i think this contains memleaks if av_find_stream_info fails, i guess
> you should try valgrind on your code, maybe there are more such issues
>

Is there anything along the lines of avformat_free_context which frees
the AVFormatContext and all its children, kind of like a destructor in
C++, or is there some design decision on why something like that can't
be part of the public API? Or couldn't av_find_stream_info use a 'if
(err) goto fail' where fail cleans up after itself? I really don't think
that adding a bunch of frees to clean up the AVFormatContext's members
every time some operation fails is really elegant nor maintainable (each
block of frees would to be changed every time AVFormatContext and its
members have some members added or removed).

Anyhow valgrind's memcheck seemed to be warning mostly about not
checking for realloc failures, which I've fixed.

> but i see now that there is a more tricky issue above that, namely
> the av_playlist_time_offset is rescaled to the stream timebase and that
> is not accurate ... for example a 1 frame per second video timebase=1
> and a audio streams that has a 1/44100 timebase could end up with up to
> 0.5 sec desync
> That is, its not just a academic rounding problem.
> But i dont know how to solve this ATM, so i suggest unless you have a
> great idea to leave this as a known bug for later, it must be fixed but
> i need to think about it a little first how to fix it ...
>

Perhaps I could store the durations in native stream time base in one
array, then have another array that stores the scaling ratios for each
native time base, and just convert directly to the target stream time
base when needed, thus avoiding having to convert to AV_TIME_BASE at all?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpeg-concat.diff
Type: text/x-diff
Size: 59058 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090825/a48ab7c0/attachment.diff>