[FFmpeg-devel] [PATCH] lavf/tee: add support for bitstream filtering

Stefano Sabatini stefasab at gmail.com
Sun Aug 4 13:47:34 CEST 2013


On date Sunday 2013-08-04 11:53:36 +0200, Nicolas George encoded:
> Le sextidi 16 thermidor, an CCXXI, Stefano Sabatini a écrit :
> > Updated with various fixes. I'm still making my mind about adopting a
> > different syntax.
> > -- 
> > FFmpeg = Fast & Foolish Marvellous Powerful Everlasting Guru
> 
> > >From 35fed58f67befbdf9067a03110dc87d47be34b7c Mon Sep 17 00:00:00 2001
> > From: Stefano Sabatini <stefasab at gmail.com>
> > Date: Fri, 5 Jul 2013 18:33:30 +0200
> > Subject: [PATCH] lavf/tee: add support for bitstream filtering
> > 
> 
> > This allows to apply different bitstream filters to different outputs,
> > with no need to trancode.
> 

> Note: if you are not transcoding, -f tee is not necessary, -c copy will work
> just fine. -f tee, IMHO, is most useful when transcoding with expensive
> codecs.

Yes, this is indeed the use case I'm handling.

> 
> > 
> > TODO: bump minor
> > ---
> >  doc/muxers.texi   |  25 ++++++-
> >  libavformat/tee.c | 201 +++++++++++++++++++++++++++++++++++++++++++++++++++---
> >  2 files changed, 216 insertions(+), 10 deletions(-)
> > 
> > diff --git a/doc/muxers.texi b/doc/muxers.texi
> > index b0dc7e6..7c5a1c8 100644
> > --- a/doc/muxers.texi
> > +++ b/doc/muxers.texi
> > @@ -818,11 +818,34 @@ 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
> > +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 a bitstream filter
> > +followed by a "+". If the stream specifier is not specified, it will
> > +be applied to all streams in the output.
> > +
> 
> > +Several bitstream filters can be specified, separated each other by
> 
> I am not completely sure, but I believe that it should be "separated from
> each other"; it could be simply "separated by".

Removed.

> > +",".
> > +
> > +The BNF description of the bitstream filters specification is given by:
> > + at example
> > + at var{BSF} ::= @var{BSF_NAME}[+ at var{STREAM_SPECIFIER}]
> > + at var{BSFS} ::= @var{BSF}[, at var{BSFS}]
> > + 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..90947c7 100644
> > --- a/libavformat/tee.c
> > +++ b/libavformat/tee.c
> > @@ -27,10 +27,16 @@
> >  
> >  #define MAX_SLAVES 16
> >  
> > +typedef struct {
> > +    AVFormatContext *fmt_ctx;
> > +    AVBitStreamFilterContext **bsf_ctxs; ///< bitstream filters per stream
> > +} TeeSlave;
> > +
> >  typedef struct TeeContext {
> >      const AVClass *class;
> >      unsigned nb_slaves;
> > -    AVFormatContext *slaves[MAX_SLAVES];
> > +    TeeSlave slaves[MAX_SLAVES];
> > +    char *bsfs;
> >  } TeeContext;
> >  
> >  static const char *const slave_delim     = "|";
> > @@ -82,13 +88,98 @@ fail:
> >      return ret;
> >  }
> >  
> > -static int open_slave(AVFormatContext *avf, char *slave, AVFormatContext **ravf)
> > +/**
> > + * Parse bitstream filter, in the form:
> > + * BSF ::= BSF_NAME[+STREAM_SPECIFIER]
> > + */
> > +static int parse_bsf(void *log_ctx, char *bsf,
> > +                     AVFormatContext *fmt_ctx,
> > +                     AVBitStreamFilterContext **bsf_ctxs)
> > +{
> > +    int i, ret = 0;
> > +    char *bsf1 = av_strdup(bsf);
> > +    char *bsf_name, *spec;
> > +
> > +    if (!bsf1)
> > +        return AVERROR(ENOMEM);
> > +    bsf_name = av_strtok(bsf1, "+", &spec);
> > +
> > +    /* select the streams matched by the specifier */
> > +    for (i = 0; i < fmt_ctx->nb_streams; i++) {
> > +        AVBitStreamFilterContext *bsf_ctx;
> > +
> > +        if (spec && spec[0]) {
> > +            ret = avformat_match_stream_specifier(fmt_ctx, fmt_ctx->streams[i], spec);
> > +            if (ret < 0) {
> > +                av_log(log_ctx, AV_LOG_ERROR,
> > +                       "Invalid stream specifier for bitstream filter '%s'\n", bsf);
> > +                goto end;
> > +            }
> > +
> > +            if (ret == 0) /* no match */
> > +                continue;
> > +        }
> > +
> > +        bsf_ctx = av_bitstream_filter_init(bsf_name);
> > +        if (!bsf_ctx) {
> > +            av_log(log_ctx, AV_LOG_ERROR,
> > +                   "Cannot create bitstream filter with name '%s' for stream %d, "
> > +                   "unknown filter or internal error happened\n",
> > +                   bsf_name, i);
> > +            ret = AVERROR_UNKNOWN;
> > +            goto end;
> > +        }
> > +
> > +        /* append bsf context to the list of per-stream bsf contexts */
> > +        if (!bsf_ctxs[i]) {
> > +            bsf_ctxs[i] = bsf_ctx;
> > +        } else {
> > +            AVBitStreamFilterContext *bsf_ctx1 = bsf_ctxs[i];
> > +            while (bsf_ctx1->next)
> > +                bsf_ctx1 = bsf_ctx1->next;
> > +            bsf_ctx1->next = bsf_ctx;
> > +        }
> > +    }
> > +
> > +end:
> > +    av_free(bsf1);
> > +    return ret;
> > +}
> > +
> > +/**
> > + * Parse list of bitstream filters in the form:
> > + * BSFS ::= BSF[,BSFS]
> > + */
> > +static int parse_slave_bsfs(void *log_ctx, char *slave_bsfs,
> > +                            AVFormatContext *fmt_ctx,
> > +                            AVBitStreamFilterContext **bsf_ctx)
> > +{
> > +    char *bsf, *slave_bsfs1 = av_strdup(slave_bsfs);
> > +    char *saveptr, *buf = slave_bsfs1;
> > +    int ret;
> > +
> > +    if (!slave_bsfs1)
> > +        return AVERROR(ENOMEM);
> > +
> > +    while (bsf = av_strtok(buf, ",", &saveptr)) {
> > +        buf = NULL;
> > +        ret = parse_bsf(log_ctx, bsf, fmt_ctx, bsf_ctx);
> > +        if (ret < 0)
> > +            goto end;
> > +    }
> > +
> > +end:
> > +    av_free(slave_bsfs1);
> > +    return ret;
> > +}
> > +
> > +static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
> >  {
> >      int i, ret;
> >      AVDictionary *options = NULL;
> >      AVDictionaryEntry *entry;
> >      char *filename;
> > -    char *format = NULL;
> > +    char *format = NULL, *bsfs = NULL;
> >      AVFormatContext *avf2 = NULL;
> >      AVStream *st, *st2;
> >  
> > @@ -99,6 +190,11 @@ static int open_slave(AVFormatContext *avf, char *slave, AVFormatContext **ravf)
> >          entry->value = NULL; /* prevent it from being freed */
> >          av_dict_set(&options, "f", NULL, 0);
> >      }
> > +    if ((entry = av_dict_get(options, "bsfs", NULL, 0))) {
> > +        bsfs = entry->value;
> > +        entry->value = NULL; /* prevent it from being freed */
> > +        av_dict_set(&options, "bsfs", NULL, 0);
> > +    }
> >  
> >      ret = avformat_alloc_output_context2(&avf2, NULL, format, filename);
> >      if (ret < 0)
> > @@ -138,6 +234,7 @@ static int open_slave(AVFormatContext *avf, char *slave, AVFormatContext **ravf)
> >                 slave, av_err2str(ret));
> >          goto fail;
> >      }
> > +
> >      if (options) {
> >          entry = NULL;
> >          while ((entry = av_dict_get(options, "", entry, AV_DICT_IGNORE_SUFFIX)))
> > @@ -146,7 +243,17 @@ static int open_slave(AVFormatContext *avf, char *slave, AVFormatContext **ravf)
> >          goto fail;
> >      }
> >  
> > -    *ravf = avf2;
> > +    tee_slave->fmt_ctx = avf2;
> > +    tee_slave->bsf_ctxs = av_calloc(avf2->nb_streams, sizeof(TeeSlave));
> > +    if (!tee_slave->bsf_ctxs)
> > +        return AVERROR(ENOMEM);
> > +    memset(tee_slave->bsf_ctxs, 0, avf2->nb_streams * sizeof(TeeSlave));
> > +
> 
> > +    /* generate the array of bitstream filters */
> > +    if (bsfs) {
> > +        if ((ret = parse_slave_bsfs(avf, bsfs, avf2, tee_slave->bsf_ctxs)) < 0)
> > +            return ret;
> > +    }
> 

> I suspect bsfs leaks here.

Fixed.

> And since it belongs exclusively to this code,
> you could remove all the av_strdup() in the parsing code, that would make it
> simpler (or do you have a reason not to?).

Complicates the logic, since I don't want to track if the function was
called or not, dupping the string internally seems to me easier to handle.

[...]
 
> No more remarks for now.

I want to think more about the syntax thing. Updated patch in
attachment.
-- 
FFmpeg = Fabulous and Freak Monstrous Power Empowered Geek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavf-tee-add-support-for-bitstream-filtering.patch
Type: text/x-diff
Size: 12884 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130804/4b0147c4/attachment.bin>


More information about the ffmpeg-devel mailing list