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

Nicolas George nicolas.george at normalesup.org
Thu Aug 1 14:36:21 CEST 2013


Le tridi 13 thermidor, an CCXXI, Stefano Sabatini a écrit :
> Changed to:
> dumo_extra+a,mp4toannexb+v.

I believe you missed my point: I meant:

bsfs:a=dump_extra,bsfs:v=mp4toannexb

But I realize that it is not possible as ':' is not an authorized option
character. "bsfs.a=dump_extra" would be possible, though.

> Not sure what you mean, I'm already duping it.

I'll explain later. (Run!)

> 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
> +",".
> +
> +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..ebcee9f 100644
> --- a/libavformat/tee.c
> +++ b/libavformat/tee.c
> @@ -27,10 +27,17 @@
>  
>  #define MAX_SLAVES 16
>  
> +typedef struct {
> +    AVFormatContext *fmt_ctx;
> +    char *bsfs;
> +    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 +89,99 @@ 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);

Remove that hunk and work directly with bsf (see below).

> +    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]

> + * BSF  ::= BSF_NAME[:STREAM_SPECIFIER]

Not relevant for this function (and obsolete).

> + */
> +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);

The string is already strdupped here, there is no need to re-dup it again in
parse_bsf().

> +
> +    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 +192,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);

Could be factored with the "f" case into a single function
steal_option(&options, "bsfs") or something (unless you go for the
"bsfs.a=...:bsfs.v=..." syntax). But not really important.

> +    }
>  
>      ret = avformat_alloc_output_context2(&avf2, NULL, format, filename);
>      if (ret < 0)
> @@ -138,6 +236,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 +245,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_mallocz(avf2->nb_streams * sizeof(*(tee_slave->bsf_ctxs)));

av_calloc to avoid overflows, however unlikely they are. And please keep
lines below 80 characters whenever conveniently possible.

> +    if (!tee_slave->bsf_ctxs)
> +        return AVERROR(ENOMEM);
> +
> +    /* generate the array of bitstream filters */
> +    if (bsfs) {

> +        tee_slave->bsfs = bsfs;

It seems to me all this string is sit there doing nothing waiting to be
freed. Maybe drop the field and free it here. That would also allow to
remove the strdup in parse_slave_bsfs().

> +        if ((ret = parse_slave_bsfs(avf, bsfs, avf2, tee_slave->bsf_ctxs)) < 0)
> +            return ret;
> +    }
>      return 0;
>  
>  fail:
> @@ -158,14 +267,48 @@ static void close_slaves(AVFormatContext *avf)
>  {
>      TeeContext *tee = avf->priv_data;
>      AVFormatContext *avf2;
> -    unsigned i;
> +    unsigned i, j;
>  
>      for (i = 0; i < tee->nb_slaves; i++) {
> -        avf2 = tee->slaves[i];
> +        avf2 = tee->slaves[i].fmt_ctx;
> +
> +        for (j = 0; j < avf2->nb_streams; j++) {
> +            AVBitStreamFilterContext *bsf_ctx = tee->slaves[i].bsf_ctxs[j];
> +            while (bsf_ctx) {
> +                bsf_ctx = bsf_ctx->next;
> +                av_bitstream_filter_close(bsf_ctx);
> +            }
> +            av_freep(&tee->slaves[i].bsfs);
> +        }
> +
>          avio_close(avf2->pb);
>          avf2->pb = NULL;
>          avformat_free_context(avf2);
> -        tee->slaves[i] = NULL;
> +        tee->slaves[i].fmt_ctx = NULL;
> +    }
> +}
> +
> +static void log_slave(TeeSlave *slave, void *log_ctx, int log_level)
> +{
> +    int i;
> +    av_log(log_ctx, log_level, "filename:'%s' format:%s\n",
> +           slave->fmt_ctx->filename, slave->fmt_ctx->oformat->name);
> +    for (i = 0; i < slave->fmt_ctx->nb_streams; i++) {
> +        AVStream *st = slave->fmt_ctx->streams[i];
> +        AVBitStreamFilterContext *bsf_ctx = slave->bsf_ctxs[i];
> +
> +        av_log(log_ctx, log_level, "    stream:%d codec:%s type:%s",
> +               i, avcodec_get_name(st->codec->codec_id),
> +               av_get_media_type_string(st->codec->codec_type));
> +        if (bsf_ctx) {
> +            av_log(log_ctx, log_level, " bsfs:");
> +            while (bsf_ctx) {
> +                av_log(log_ctx, log_level, "%s%s",
> +                       bsf_ctx->filter->name, bsf_ctx->next ? "," : "");
> +                bsf_ctx = bsf_ctx->next;
> +            }
> +        }
> +        av_log(log_ctx, log_level, "\n");
>      }
>  }
>  
> @@ -195,6 +338,7 @@ static int tee_write_header(AVFormatContext *avf)
>      for (i = 0; i < nb_slaves; i++) {
>          if ((ret = open_slave(avf, slaves[i], &tee->slaves[i])) < 0)
>              goto fail;
> +        log_slave(&tee->slaves[i], avf, AV_LOG_VERBOSE);
>          av_freep(&slaves[i]);
>      }
>  
> @@ -208,6 +352,50 @@ fail:
>      return ret;
>  }
>  

> +static int filter_packet(void *log_ctx, AVPacket *pkt,
> +                         AVFormatContext *fmt_ctx, AVBitStreamFilterContext *bsf_ctx)
> +{

I do not know this API very well.

> +    AVCodecContext *enc_ctx = fmt_ctx->streams[pkt->stream_index]->codec;
> +
> +    while (bsf_ctx) {
> +        AVPacket new_pkt = *pkt;
> +        int ret = av_bitstream_filter_filter(bsf_ctx, enc_ctx, NULL,
> +                                             &new_pkt.data, &new_pkt.size,
> +                                             pkt->data, pkt->size,
> +                                             pkt->flags & AV_PKT_FLAG_KEY);
> +        if (ret == 0 && new_pkt.data != pkt->data && new_pkt.destruct) {
> +            uint8_t *buf = av_malloc(new_pkt.size + FF_INPUT_BUFFER_PADDING_SIZE);
> +            if (buf) {

> +                memcpy(buf, new_pkt.data, new_pkt.size);
> +                memset(buf + new_pkt.size, 0, FF_INPUT_BUFFER_PADDING_SIZE);
> +                new_pkt.data = buf;
> +                new_pkt.buf = NULL;
> +                ret = 1;

av_copy_packet?

> +            } else

Nit: please either braces on both branches or none.

> +                ret = AVERROR(ENOMEM);

The error path seems a bit convoluted. goto fail?

> +        }
> +
> +        if (ret > 0) {
> +            av_free_packet(pkt);
> +            new_pkt.buf = av_buffer_create(new_pkt.data, new_pkt.size,
> +                                           av_buffer_default_free, NULL, 0);
> +            if (!new_pkt.buf)
> +                return AVERROR(ENOMEM);
> +        } else if (ret < 0) {
> +            av_log(log_ctx, AV_LOG_ERROR,
> +                   "Failed to filter bitstream with filter %s for stream %d in file '%s' with codec %s\n",
> +                   bsf_ctx->filter->name, pkt->stream_index, fmt_ctx->filename,
> +                   avcodec_get_name(enc_ctx->codec_id));
> +            return ret;
> +        }
> +        *pkt = new_pkt;
> +
> +        bsf_ctx = bsf_ctx->next;
> +    }
> +
> +    return 0;
> +}
> +
>  static int tee_write_trailer(AVFormatContext *avf)
>  {
>      TeeContext *tee = avf->priv_data;
> @@ -216,7 +404,7 @@ static int tee_write_trailer(AVFormatContext *avf)
>      unsigned i;
>  
>      for (i = 0; i < tee->nb_slaves; i++) {
> -        avf2 = tee->slaves[i];
> +        avf2 = tee->slaves[i].fmt_ctx;
>          if ((ret = av_write_trailer(avf2)) < 0)
>              if (!ret_all)
>                  ret_all = ret;
> @@ -241,7 +429,7 @@ static int tee_write_packet(AVFormatContext *avf, AVPacket *pkt)
>      AVRational tb, tb2;
>  
>      for (i = 0; i < tee->nb_slaves; i++) {
> -        avf2 = tee->slaves[i];
> +        avf2 = tee->slaves[i].fmt_ctx;
>          s = pkt->stream_index;
>          if (s >= avf2->nb_streams) {
>              if (!ret_all)
> @@ -259,6 +447,8 @@ static int tee_write_packet(AVFormatContext *avf, AVPacket *pkt)
>          pkt2.pts      = av_rescale_q(pkt->pts,      tb, tb2);
>          pkt2.dts      = av_rescale_q(pkt->dts,      tb, tb2);
>          pkt2.duration = av_rescale_q(pkt->duration, tb, tb2);
> +
> +        filter_packet(avf2, &pkt2, avf2, tee->slaves[i].bsf_ctxs[s]);
>          if ((ret = av_interleaved_write_frame(avf2, &pkt2)) < 0)
>              if (!ret_all)
>                  ret_all = ret;

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130801/87d9d88a/attachment.asc>


More information about the ffmpeg-devel mailing list