[FFmpeg-devel] [PATCH] Playlist API

Michael Niedermayer michaelni
Mon Aug 31 04:20:30 CEST 2009


On Sat, Aug 29, 2009 at 04:29:50PM -0700, Geza Kovacs wrote:
> On 08/29/2009 03:11 AM, Michael Niedermayer wrote:
> > well, this sounds a little buggy ...
> > I mean first the user app can have a playlist and play that and after
> > playing such list of mp3s once or a few times the user might choose to
> > insert a few new mp3s, or remove a few but it does not seem the durations
> > or stream numbers would be updated in that case ...
> > 
> > also, it seems seeking would similarly fail with this system, but maybe
> > ive missed something ...
> > I mean, the seek code expects durations to be summed but as they are only
> > summed in read_packet they are not neccesarily summed for all elements
> > of a playlist making forward seek fail ...
> > 
> 
> Ok, I'm now briefly opening the AVFormatcontext, setting nb_streams and
> durations, then closing and freeing it when insert_item is called (and
> offsetting the sums appropriately when remove_item is called), so
> durations and nb_streams should always be set at the correct summed
> values at all times.

good but this should not be done when accessing playlists through the new API
just when accessing them through the demuxer interface.
Reading all playlist elements is likely quite timeconsuming ,for the demuxer
like interface we need it but a new API we of course are free to design it
so this is not needed
also, for playlists that already contain duration/stream count information
it as well should be skiped

One could even go as far as reading the durations on demand when an actual
seek happens but thats rather low priority


> 
> > 
> > 
> > [...]
> > 
> >> +    flist_tmp = av_realloc(ctx->flist, sizeof(*(ctx->flist)) * (++ctx->pelist_size));
> >> +    if (!flist_tmp) {
> >> +        av_log(NULL, AV_LOG_ERROR, "av_realloc error in av_playlist_insert_item\n");
> >> +        return AVERROR_NOMEM;
> >> +    } else
> >> +        ctx->flist = flist_tmp;
> >> +    durations_tmp = av_realloc(ctx->durations,
> >> +                               sizeof(*(ctx->durations)) * (ctx->pelist_size));
> >> +    if (!durations_tmp) {
> >> +        av_log(NULL, AV_LOG_ERROR, "av_realloc error in av_playlist_insert_item\n");
> >> +        return AVERROR_NOMEM;
> >> +    } else
> >> +        ctx->durations = durations_tmp;
> >> +    nb_streams_list_tmp = av_realloc(ctx->nb_streams_list,
> >> +                                     sizeof(*(ctx->nb_streams_list)) * (ctx->pelist_size));
> >> +    if (!nb_streams_list_tmp) {
> > 
> >> +        av_log(NULL, AV_LOG_ERROR, "av_realloc error in av_playlist_insert_item\n");
> >> +        return AVERROR_NOMEM;
> > 
> > Triplicated
> > 
> 
> I assume you're referring to the error message? If so, I'm now
> indicating the variable for which the realloc failed in the error.

well, that is a way to fix it but is that really usefull?
isnt a single
if(!a || !b || !c){
    av_log()
    return ...
}

simpler?

besides, av_log should have some context different from NULL so the
user can associate the messages to something, there could be multiple
demuxers, playlists or otherwise ...

ill review the patch tomorrow or ASAP

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is not what we do, but why we do it that matters.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090831/78f26c98/attachment.pgp>



More information about the ffmpeg-devel mailing list