[FFmpeg-devel] [PATCH] Playlist API
Michael Niedermayer
michaelni
Thu Jul 23 11:29:36 CEST 2009
On Wed, Jul 15, 2009 at 05:59:02PM -0700, Geza Kovacs wrote:
> > Iam not against an API to work with playlists programmatically, that surely
> > could be usefull, but i dont feel this is the right part to start reviewing.
> >
> > The way i see it, and iam of course open to suggestions and comments ...
> > * There are many applications using libav*, the less modifications they need
> > to support a new feature, the better.
> > From this it follows that a play list protocol&demuxer should ideally build
> > its playlist from the string provided by the user.
> > That way, something like ffmpeg -i localfile.mpg,http://www.a.com/xyz.avi
> > could work with minimal changes to ffmpeg.c
> > That of course does not preclude a API to manipulate playlists from C, but
> > that seems secodary to me because its design might be affected by the
> > other requirements ...
> >
>
> I already had that implemented using multiple input files and a "-conc"
> flag, but I've added your suggested interface as well. In the patch,
> there are thus 3 ways to input playlists (the first 2 are implemented
> with a patch against ffmpeg.c, the third will work on anything using
> libav*):
>
> 1. ffmpeg -i file1.avi,file2.mp4,file3.avi
> 2. ffmpeg -conc -i file1.avi -i file2.mp4 -i file3.mp4
> 3. ffmpeg -i playlist.m3u (pls and xspf formats also implemented in patch)
>
> > * We like to be able to transcode playlists, so that a avi+mpg can be
> > reencoded, into lets say a .mov.
> > Not only that but we also like the new .mov and the previous playlist
> > to represent their data to the user similarly so that applications dont
> > need to have 2 ways to extract things like (song)title for each part.
> > The obvious choice here would be the AVChapters of the .mov as well as
> > the AVChapters of the playlist demuxer ...
> >
>
> Also already implemented, see concatgen.c in the patch for how this is done.
[...]
> +void ff_playlist_init_playelem(PlayElem *pe)
> +{
> + int err;
> + pe->ic = av_malloc(sizeof(*(pe->ic)));
> + pe->ap = av_malloc(sizeof(*(pe->ap)));
> + memset(pe->ap, 0, sizeof(*(pe->ap)));
av_mallocz()
> + pe->ap->width = 0;
> + pe->ap->height = 0;
they should alraedy be 0
> + pe->ap->time_base = (AVRational){1, 25};
> + pe->ap->pix_fmt = 0;
> + pe->fmt = 0;
> + err = av_open_input_file(&(pe->ic), pe->filename, pe->fmt, 0, pe->ap);
> + if (err < 0)
> + av_log(pe->ic, AV_LOG_ERROR, "Error during av_open_input_file\n");
> + pe->fmt = pe->ic->iformat;
> + if (!pe->fmt)
> + av_log(pe->ic, AV_LOG_ERROR, "Input format not set\n");
> + err = av_find_stream_info(pe->ic);
> + if (err < 0)
> + av_log(pe->ic, AV_LOG_ERROR, "Could not find stream info\n");
> + if (pe->ic->pb)
> + pe->ic->pb->eof_reached = 0;
> + else
> + av_log(pe->ic, AV_LOG_ERROR, "ByteIOContext not set\n");
> +}
i doubt that this error handling is sufficient
> +
> +PlaylistContext* ff_playlist_alloc_context(void)
> +{
> + int i;
> + PlaylistContext *ctx = av_malloc(sizeof(*ctx));
> + memset(ctx, 0, sizeof(*ctx));
> + ctx->time_offsets_size = 2; // TODO don't assume we have just 2 streams
hmm
> + ctx->time_offsets = av_malloc(sizeof(*(ctx->time_offsets)) * ctx->time_offsets_size);
> + for (i = 0; i < ctx->time_offsets_size; ++i)
> + ctx->time_offsets[i] = 0;
> + return ctx;
> +}
> +
> +void ff_playlist_populate_context(AVFormatContext *s)
> +{
> + int i;
> + AVFormatContext *ic;
> + PlaylistContext *ctx = s->priv_data;
> + ff_playlist_init_playelem(ctx->pelist[ctx->pe_curidx]);
> + ic = ctx->pelist[ctx->pe_curidx]->ic;
> + ic->iformat->read_header(ic, 0);
> + s->nb_streams = ic->nb_streams;
> + for (i = 0; i < ic->nb_streams; ++i)
> + s->streams[i] = ic->streams[i];
> + s->packet_buffer = ic->packet_buffer;
> + s->packet_buffer_end = ic->packet_buffer_end;
> +}
> +
> +PlaylistContext *ff_playlist_get_context(AVFormatContext *ic)
> +{
> + if (ic && ic->iformat && ic->iformat->long_name && ic->priv_data &&
> + !strncmp(ic->iformat->long_name, "CONCAT", 6))
> + return ic->priv_data;
> + else
> + return NULL;
> +}
this function appears unused
> +
> +void ff_playlist_set_context(AVFormatContext *ic, PlaylistContext *ctx)
> +{
> + if (ic && ctx)
> + ic->priv_data = ctx;
> +}
> +
> +AVStream *ff_playlist_get_stream(PlaylistContext *ctx, int pe_idx, int stream_index)
> +{
> + if (ctx && pe_idx < ctx->pelist_size && ctx->pelist && ctx->pelist[pe_idx] &&
> + ctx->pelist[pe_idx]->ic && stream_index < ctx->pelist[pe_idx]->ic->nb_streams &&
> + ctx->pelist[pe_idx]->ic->streams && ctx->pelist[pe_idx]->ic->streams[stream_index])
> + return ctx->pelist[pe_idx]->ic->streams[stream_index];
> + else
> + return NULL;
> +}
> +
> +void ff_playlist_split_encodedstring(char *s, char sep, char ***flist_ptr, int *len_ptr)
> +{
> + char c, *ts, **flist;
> + int i, len, buflen, *sepidx;
> + buflen = sepidx = len = 0;
> + sepidx = av_fast_realloc(sepidx, &buflen, ++len);
> + sepidx[0] = 0;
> + ts = s;
> + while ((c = *ts++) != 0) {
> + if (c == sep) {
> + sepidx[len] = ts-s;
> + sepidx = av_fast_realloc(sepidx, &buflen, ++len);
> + }
> + }
> + sepidx[len] = ts-s;
> + ts = s;
> + *len_ptr = len;
> + *flist_ptr = flist = av_malloc(sizeof(*flist) * (len+1));
> + flist[len] = 0;
> + for (i = 0; i < len; ++i) {
> + flist[i] = av_malloc(sepidx[i+1]-sepidx[i]);
> + av_strlcpy(flist[i], ts+sepidx[i], sepidx[i+1]-sepidx[i]);
> + }
> + av_free(sepidx);
i think that can be done with a single loop and less code
> +}
> +
> +PlaylistContext *ff_playlist_from_encodedstring(char *s, char sep)
> +{
> + PlaylistContext *ctx;
> + char **flist;
> + int i, len;
> + char workingdir[1024];
> + getcwd(workingdir, 1024);
> + workingdir[1023] = 0;
this looks a little odd, the playlist code souldnt need to know the current
directory
[...]
> +// converts list of mixed absolute and relative paths into all absolute paths
> +void ff_playlist_relative_paths(char **flist, int len, const char *workingdir)
> +{
> + int i;
> + for (i = 0; i < len; ++i) { // determine if relative paths
> + FILE *file;
> + char *fullfpath;
> + int wdslen = strlen(workingdir);
> + int flslen = strlen(flist[i]);
> + fullfpath = av_malloc(sizeof(char) * (wdslen+flslen+2));
> + av_strlcpy(fullfpath, workingdir, wdslen+1);
> + fullfpath[wdslen] = '/';
> + fullfpath[wdslen+1] = 0;
> + av_strlcat(fullfpath, flist[i], wdslen+flslen+2);
> + if ((file = fopen(fullfpath, "r"))) {
> + fclose(file);
iam not sure if that will work with pipes
also playlist items might themselfs be playlists or on varios protocols
like http instead of local files
[...]
> +
> +/** @struct PlayElem
> + * @brief Represents each input file on a playlist.
> + */
> +typedef struct PlayElem {
> + AVFormatContext *ic; /**< AVFormatContext for this playlist item */
> + char *filename; /**< Filename with absolute path of this playlist item */
> + AVInputFormat *fmt; /**< AVInputFormat manually specified for this playlist item */
> + AVFormatParameters *ap; /**< AVFormatParameters for this playlist item */
> +} PlayElem;
cant a straight AVFormatContext be used instead of PlayElem ?
> +
> +/** @struct PlaylistContext
> + * @brief Represents the playlist and contains PlayElem for each playlist item.
> + */
> +typedef struct PlaylistContext {
> + PlayElem **pelist; /**< List of PlayElem, with each representing a playlist item */
> + int pelist_size; /**< Number of PlayElem stored in pelist */
> + int pe_curidx; /**< Index of the PlayElem that packets are being read from */
> + int time_offsets_size; /**< Number of time offsets (number of multimedia streams), 2 with audio and video. */
how does that work if the entries in a playlist have each a different
number of streams?
[...]
> +/** @file libavformat/concatgen.c
> + * @author Geza Kovacs ( gkovacs mit edu )
> + *
> + * @brief Generic functions used by playlist/concatenation demuxers
> + *
> + * @details These functions are used to read packets and seek streams
> + * for concat-type demuxers, abstracting away the playlist element switching
> + * process.
> + */
> +
> +#include "playlist.h"
> +
> +int ff_concatgen_read_packet(AVFormatContext *s,
> + AVPacket *pkt)
> +{
> + int i;
> + int ret;
> + int stream_index;
> + PlaylistContext *ctx;
> + AVFormatContext *ic;
> + char have_switched_streams = 0;
> + ctx = s->priv_data;
> + stream_index = 0;
> + retr:
for(;;)
and goto -> continue
is clearer IMHO
> + ic = ctx->pelist[ctx->pe_curidx]->ic;
> + ret = ic->iformat->read_packet(ic, pkt);
> + if (pkt) {
> + stream_index = pkt->stream_index;
> + ic = ctx->pelist[ctx->pe_curidx]->ic;
> + pkt->stream = ic->streams[pkt->stream_index];
> + }
> + if (ret >= 0) {
> + if (pkt) {
> + int64_t time_offset;
> + time_offset = av_rescale_q(ctx->time_offsets[pkt->stream_index], AV_TIME_BASE_Q, ic->streams[stream_index]->time_base);
> + av_log(ic, "%s conv stream time from %ld to %d/%d is %ld\n", ic->iformat->name, ctx->time_offsets[pkt->stream_index], ic->streams[stream_index]->time_base.num, ic->streams[stream_index]->time_base.den, time_offset);
> + // TODO changing either dts or pts leads to timing issues on h264
> + pkt->dts += time_offset;
> + if (!ic->streams[pkt->stream_index]->codec->has_b_frames)
> + pkt->pts = pkt->dts + 1;
whatever that is, it is wrong
> + }
> + } else if (ret < 0 && !have_switched_streams && ctx->pe_curidx < ctx->pelist_size - 1) {
> + // TODO switch from AVERROR_EOF to AVERROR_EOS
> + // -32 AVERROR_EOF for avi, -51 for ogg
> + av_log(ic, AV_LOG_DEBUG, "Switching stream %d to %d\n", stream_index, ctx->pe_curidx+1);
> + for (i = 0; i < ic->nb_streams && i < ctx->time_offsets_size; ++i) {
> + ctx->time_offsets[i] += av_rescale_q(ic->streams[i]->duration, ic->streams[i]->time_base, AV_TIME_BASE_Q);
> + }
> + ++ctx->pe_curidx;
> + ff_playlist_populate_context(s);
> + have_switched_streams = 1;
> + goto retr;
> + } else {
> + av_log(ic, AV_LOG_DEBUG, "Packet read error %d\n", ret);
> + }
> + return ret;
> +}
> +
> +int ff_concatgen_read_seek(AVFormatContext *s,
> + int stream_index,
> + int64_t pts,
> + int flags)
> +{
> + PlaylistContext *ctx;
> + AVFormatContext *ic;
> + ctx = s->priv_data;
> + ic = ctx->pelist[ctx->pe_curidx]->ic;
> + return ic->iformat->read_seek(ic, stream_index, pts, flags);
> +}
seeking should work over playlist items, not just within a playlist item
this doesnt look correct thus ...
[...]
> +#if CONFIG_M3U_DEMUXER
> +AVInputFormat m3u_demuxer = {
> + "m3u",
> + NULL_IF_CONFIG_SMALL("CONCAT M3U format"),
> + sizeof(PlaylistContext),
> + m3u_probe,
> + m3u_read_header,
> + ff_concatgen_read_packet,
> + ff_concatgen_read_close,
> + ff_concatgen_read_seek,
[...]
> + ff_concatgen_read_seek, //m3u_read_seek2
i doubt the same function is compatible with both APIs
[...]
> +static int ff_concatgen_read_packet(AVFormatContext *s, AVPacket *pkt);
> +
> +static int ff_concatgen_read_seek(AVFormatContext *s, int stream_index, int64_t pts, int flags);
> +
> +static int ff_concatgen_read_timestamp(AVFormatContext *s, int stream_index, int64_t *pos, int64_t pos_limit);
> +
> +static int ff_concatgen_read_close(AVFormatContext *s);
> +
> +static int ff_concatgen_read_play(AVFormatContext *s);
> +
> +static int ff_concatgen_read_pause(AVFormatContext *s);
static functions dont need a ff_ prefix
[...]
> @@ -1644,6 +1668,7 @@ static int av_encode(AVFormatContext **output_files,
> uint8_t no_packet[MAX_FILES]={0};
> int no_packet_count=0;
>
> +
> file_table= av_mallocz(nb_input_files * sizeof(AVInputFile));
> if (!file_table)
> goto fail;
cosmetics
[...]
> @@ -2859,6 +2884,43 @@ static void opt_input_file(const char *filename)
> using_stdin |= !strncmp(filename, "pipe:", 5) ||
> !strcmp(filename, "/dev/stdin");
>
> + if (concatenate_video_files) { // need to specify -conc before -i
> + if (!playlist_ctx) {
> + ic = avformat_alloc_context();
> + av_log(ic, AV_LOG_DEBUG, "Generating playlist ctx\n");
> + playlist_ctx = ff_playlist_alloc_context();
> + ff_playlist_add_path(playlist_ctx, filename);
> + ic->nb_streams = 2;
> + ic->iformat = ff_concat_alloc_demuxer();
> + ff_playlist_set_context(ic, playlist_ctx);
> + ff_playlist_populate_context(ic);
> + nb_input_files = 1;
> + input_files[0] = ic;
> + goto configcodecs;
> + }
> + else {
> + av_log(ic, AV_LOG_DEBUG, "Adding file %s to playlist\n", filename);
> + ff_playlist_add_path(playlist_ctx, filename);
> + return;
> + }
> + }
> +
> + // alternative interface for concat - specify -i item1,item2,item3
why 2 interfaces?
also does mixing concateated inputs and non concatenate inputs work?
that is if we want to combine a single moview with 3 concatenated mp3s
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
-------------- 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/20090723/3466988e/attachment.pgp>
More information about the ffmpeg-devel
mailing list