[FFmpeg-devel] [PATCH] lavf/tee: add support for bitstream filtering
Stefano Sabatini
stefasab at gmail.com
Fri Aug 16 16:48:37 CEST 2013
On date Friday 2013-08-16 12:40:48 +0200, Nicolas George encoded:
> Le sextidi 26 thermidor, an CCXXI, Stefano Sabatini a écrit :
> > This allows to apply different bitstream filters to different outputs,
> > with no need to trancode.
> >
> > TODO: bump minor
> > ---
> > doc/muxers.texi | 19 ++++-
> > libavformat/tee.c | 208 +++++++++++++++++++++++++++++++++++++++++++++++++-----
> > 2 files changed, 208 insertions(+), 19 deletions(-)
> >
> > diff --git a/doc/muxers.texi b/doc/muxers.texi
> > index d204188..0f4db3c 100644
> > --- a/doc/muxers.texi
> > +++ b/doc/muxers.texi
> > @@ -852,11 +852,28 @@ leading or trailing spaces or any special character, it must be
> > escaped (see the ``Quoting and escaping'' section in the ffmpeg-utils
> > manual).
> >
> > -Options can be specified for each slave by prepending them as a list of
> > +Muxer options can be specified for each slave by prepending them as a list of
> > @var{key}=@var{value} pairs separated by ':', between square brackets. If
> > the options values contain a special character or the ':' separator, they
> > must be escaped; note that this is a second level escaping.
> >
> > +The following special options are also recognized:
> > + at table @option
> > + at item f
> > +Specify the format name. Useful if it cannot be guessed from the
> > +output name suffix.
> > +
> > + at item bsfs[/@var{spec}]
> > +Specify a list of bitstream filters to apply to the specified
> > +output. It is possible to specify to which streams a given bitstream
> > +filter applies, by appending a stream specifier to the option
> > +separated by @code{/} . If the stream specifier is not specified, the
> > +bistream filters will be applied to all streams in the output.
> > +
> > +Several bitstream filters can be specified, separated by ",".
> > + at end example
> > + at end table
> > +
> > Example: encode something and both archive it in a WebM file and stream it
> > as MPEG-TS over UDP (the streams need to be explicitly mapped):
> >
> > diff --git a/libavformat/tee.c b/libavformat/tee.c
> > index 7b8b371..e959731 100644
> > --- a/libavformat/tee.c
> > +++ b/libavformat/tee.c
> > @@ -27,10 +27,15 @@
> >
> > #define MAX_SLAVES 16
> >
> > +typedef struct {
> > + AVFormatContext *avf;
> > + AVBitStreamFilterContext **bsfs; ///< bitstream filters per stream
> > +} TeeSlave;
> > +
> > typedef struct TeeContext {
> > const AVClass *class;
> > unsigned nb_slaves;
> > - AVFormatContext *slaves[MAX_SLAVES];
> > + TeeSlave slaves[MAX_SLAVES];
> > } TeeContext;
> >
> > static const char *const slave_delim = "|";
> > @@ -82,7 +87,53 @@ fail:
> > return ret;
> > }
> >
> > -static int open_slave(AVFormatContext *avf, char *slave, AVFormatContext **ravf)
> > +/**
> > + * Parse list of bitstream filters and add them to the list of filters
> > + * pointed to by bsfs.
> > + *
> > + * The list must be specified in the form:
> > + * BSFS ::= BSF[,BSFS]
> > + */
> > +static int parse_bsfs(void *log_ctx, const char *bsfs_spec,
> > + AVBitStreamFilterContext **bsfs)
> > +{
> > + char *bsf_name, *buf, *saveptr;
> > + int ret;
> > +
> > + if (!(buf = av_strdup(bsfs_spec)))
> > + return AVERROR(ENOMEM);
> > +
> > + while (bsf_name = av_strtok(buf, ",", &saveptr)) {
> > + AVBitStreamFilterContext *bsf = av_bitstream_filter_init(bsf_name);
> > +
> > + if (!bsf) {
> > + av_log(log_ctx, AV_LOG_ERROR,
> > + "Cannot initialize bitstream filter with name '%s', "
> > + "unknown filter or internal error happened\n",
> > + bsf_name);
> > + ret = AVERROR_UNKNOWN;
> > + goto end;
> > + }
> > +
>
> > + /* append bsf context to the list of bsf contexts */
> > + if (!*bsfs) {
> > + *bsfs = bsf;
> > + } else {
> > + AVBitStreamFilterContext *bsf1 = *bsfs;
> > + while (bsf1->next)
> > + bsf1 = bsf1->next;
> > + bsf1->next = bsf;
> > + }
>
> Simpler:
>
> *bsfs = bsf;
> bsfs = &bsf->next;
>
> > +
> > + buf = NULL;
> > + }
> > +
> > +end:
> > + av_free(buf);
> > + return ret;
> > +}
> > +
> > +static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
> > {
> > int i, ret;
> > AVDictionary *options = NULL;
> > @@ -102,14 +153,13 @@ static int open_slave(AVFormatContext *avf, char *slave, AVFormatContext **ravf)
> >
> > ret = avformat_alloc_output_context2(&avf2, NULL, format, filename);
> > if (ret < 0)
> > - goto fail;
> > - av_free(format);
> > + goto end;
> >
> > for (i = 0; i < avf->nb_streams; i++) {
> > st = avf->streams[i];
> > if (!(st2 = avformat_new_stream(avf2, NULL))) {
> > ret = AVERROR(ENOMEM);
> > - goto fail;
> > + goto end;
> > }
> > st2->id = st->id;
> > st2->r_frame_rate = st->r_frame_rate;
> > @@ -122,34 +172,79 @@ static int open_slave(AVFormatContext *avf, char *slave, AVFormatContext **ravf)
> > st2->avg_frame_rate = st->avg_frame_rate;
> > av_dict_copy(&st2->metadata, st->metadata, 0);
> > if ((ret = avcodec_copy_context(st2->codec, st->codec)) < 0)
> > - goto fail;
> > + goto end;
> > }
> >
> > if (!(avf2->oformat->flags & AVFMT_NOFILE)) {
> > if ((ret = avio_open(&avf2->pb, filename, AVIO_FLAG_WRITE)) < 0) {
> > av_log(avf, AV_LOG_ERROR, "Slave '%s': error opening: %s\n",
> > slave, av_err2str(ret));
> > - goto fail;
> > + goto end;
> > }
> > }
> >
> > if ((ret = avformat_write_header(avf2, &options)) < 0) {
> > av_log(avf, AV_LOG_ERROR, "Slave '%s': error writing header: %s\n",
> > slave, av_err2str(ret));
> > - goto fail;
> > + goto end;
> > + }
> > +
> > + tee_slave->avf = avf2;
> > + tee_slave->bsfs = av_calloc(avf2->nb_streams, sizeof(TeeSlave));
> > + if (!tee_slave->bsfs) {
> > + ret = AVERROR(ENOMEM);
> > + goto end;
> > + }
>
> > + memset(tee_slave->bsfs, 0, avf2->nb_streams * sizeof(TeeSlave));
>
> Unnecessary, calloc already does it.
>
> > +
> > + entry = NULL;
> > + while (entry = av_dict_get(options, "bsfs", NULL, AV_DICT_IGNORE_SUFFIX)) {
> > + const char *spec = entry->key + strlen("bsfs");
>
> > + if (*spec)
> > + spec++; /* consume separator */
>
> Possibly: check that this is indeed the separator.
Changed.
> > +
> > + for (i = 0; i < avf2->nb_streams; i++) {
> > + ret = avformat_match_stream_specifier(avf2, avf2->streams[i], spec);
> > + if (ret < 0) {
> > + av_log(avf, AV_LOG_ERROR,
> > + "Invalid stream specifier '%s' in bsfs option '%s' for slave "
> > + "output '%s'\n", spec, entry->key, filename);
> > + goto end;
> > + }
> > +
> > + if (ret > 0) {
> > + av_log(avf, AV_LOG_DEBUG, "spec:%s bsfs:%s matches stream %d of slave "
> > + "output '%s'\n", spec, entry->value, i, filename);
> > + if (tee_slave->bsfs[i]) {
> > + av_log(avf, AV_LOG_WARNING,
> > + "Duplicate bsfs specification associated to stream %d of slave "
> > + "output '%s', filters will be ignored\n", i, filename);
> > + continue;
> > + }
> > + ret = parse_bsfs(avf, entry->value, &tee_slave->bsfs[i]);
> > + if (ret < 0) {
> > + av_log(avf, AV_LOG_ERROR,
> > + "Error parsing bitstream filter sequence '%s' associated to "
> > + "stream %d of slave output '%s'\n", entry->value, i, filename);
> > + goto end;
> > + }
> > + }
> > + }
> > +
>
> > + entry->value = NULL;
> > + av_dict_set(&options, entry->key, NULL, 0);
>
> I suspect entry->value is leaking now.
Should be fixed now.
[...]
> No other remarks, thanks.
Thanks for the extensive review. I'll push the attached patch soon if
I read no more comments.
--
FFmpeg = Friendly and Fast Martial Purposeless EnGine
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavf-tee-add-support-for-bitstream-filtering.patch
Type: text/x-diff
Size: 12731 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130816/0fecf0d0/attachment.bin>
More information about the ffmpeg-devel
mailing list