[FFmpeg-devel] [PATCH] Playlist API

[Geza Kovacs]

This updated patch should address most of your comments.

On Mon, Aug 17, 2009 at 5:13 AM, Michael Niedermayer<michaelni at gmx.at> wrote:
>
>> +{
>> + ? ?int i;
>> + ? ?int64_t total = 0;
>> + ? ?for (i = 0; i < pe_curidx; ++i) {
>> + ? ? ? ?total += durations[i];
>> + ? ?}
>> + ? ?return total;
>> +}
>> +
>
>> +int ff_playlist_stream_index_from_time(PlaylistContext *ctx,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int64_t pts,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int64_t *localpts)
>> +{
>> + ? ?int i;
>> + ? ?int64_t total;
>> + ? ?i = total = 0;
>> + ? ?while (pts >= total) {
>> + ? ? ? ?if (i >= ctx->pelist_size)
>> + ? ? ? ? ? ?break;
>> + ? ? ? ?total += ctx->durations[i++];
>> + ? ?}
>
> maybe the AVIndexEntry code could be used for these things?
>

I don't think so - I would only be using the timestamps field out of
AVIndexEntry so it would be simpler and more elegant to just use a
standard array rather than structs.

>
>> + ? ?int64_t *durations; /**< Durations, in AV_TIME_BASE units, for each playlist item */
>
> why is AVFormatContext.duration not used here?
>

Since an application might potentially want to close and free the
older child AVFormatContext once it's no longer in use, this is a more
flexible design as it only requires the current master and child
AVFormatContext to be allocated.

>
>> + ? ?int *nb_streams_list; /**< List of the number of streams in each playlist item*/
>
> why is AVFormatContext.nb_streams not used here?
>

Same reason as mentioned above.

>
>> +} PlaylistContext;
>> +
>
>
>> +/** @fn AVFormatContext *ff_playlist_alloc_formatcontext(char *filename)
>
> redundant
>
>> + * ?@brief Allocates and opens file, codecs, and streams associated with filename.
>> + * ?@param filename Null-terminated string of file to open.
>> + * ?@return Returns an allocated AVFormatContext.
>> + */
>> +AVFormatContext *ff_playlist_alloc_formatcontext(char *filename);
>

Sorry, I didn't understand what you meant by this. Could you please elaborate?

>
>> diff --git a/ffmpeg.c b/ffmpeg.c
>> index e5512c2..4b1fbdc 100644
>> --- a/ffmpeg.c
>> +++ b/ffmpeg.c
>> @@ -40,6 +40,7 @@
>> ?#include "libavutil/fifo.h"
>> ?#include "libavutil/avstring.h"
>> ?#include "libavformat/os_support.h"
>> +#include "libavformat/concat.h"
>
> either concat.h is a public header or not
> if it is, the things in it are public API, need AV prefixes, be well designed,
> the header would need to be installed, would only be allowed to itself include
> public headers, ...
> OR
> ffmpeg.c can not #include it
>

I've made avplaylist.h a public header and include that instead of concat.h.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpeg-concat.diff
Type: text/x-patch
Size: 56262 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090821/4f2b6a5c/attachment.bin>