On Thu, 25 Feb 2016 16:05:42 +0100
Anton Khirnov <[email protected]> wrote:
> Deprecate the current bitstream filtering API.
> ---
> configure | 2 +-
> libavcodec/Makefile | 2 +
> libavcodec/avcodec.h | 178 +++++++++++++++++++++++++++++++--
> libavcodec/bitstream_filters.c | 51 ++++++++++
> libavcodec/bsf.c | 217
> +++++++++++++++++++++++++++++++++++++++++
> libavcodec/bsf.h | 31 ++++++
> libavcodec/version.h | 3 +
> 7 files changed, 475 insertions(+), 9 deletions(-)
> create mode 100644 libavcodec/bitstream_filters.c
> create mode 100644 libavcodec/bsf.c
> create mode 100644 libavcodec/bsf.h
>
> diff --git a/configure b/configure
> index d8b8c07..0f59433 100755
> --- a/configure
> +++ b/configure
> @@ -2517,7 +2517,6 @@ ENCODER_LIST=$(find_things encoder ENC
> libavcodec/allcodecs.c)
> DECODER_LIST=$(find_things decoder DEC libavcodec/allcodecs.c)
> HWACCEL_LIST=$(find_things hwaccel HWACCEL libavcodec/allcodecs.c)
> PARSER_LIST=$(find_things parser PARSER libavcodec/allcodecs.c)
> -BSF_LIST=$(find_things bsf BSF libavcodec/allcodecs.c)
> MUXER_LIST=$(find_things muxer _MUX libavformat/allformats.c)
> DEMUXER_LIST=$(find_things demuxer DEMUX libavformat/allformats.c)
> OUTDEV_LIST=$(find_things outdev OUTDEV libavdevice/alldevices.c)
> @@ -2531,6 +2530,7 @@ find_things_extern(){
> sed -n "s/^[^#]*extern.*$pattern *ff_\([^ ]*\)_$thing;/\1_$thing/p"
> "$file"
> }
>
> +BSF_LIST=$(find_things_extern bsf AVBitStreamFilter
> libavcodec/bitstream_filters.c)
> PROTOCOL_LIST=$(find_things_extern protocol URLProtocol
> libavformat/protocols.c)
>
> ALL_COMPONENTS="
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 256dee3..ba1661d 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -19,6 +19,8 @@ OBJS = allcodecs.o
> \
> avpicture.o \
> bitstream.o \
> bitstream_filter.o \
> + bitstream_filters.o \
> + bsf.o \
> codec_desc.o \
> d3d11va.o \
> dirac.o \
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 33de8ec..f6d474f 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -4692,35 +4692,197 @@ int av_get_audio_frame_duration(AVCodecContext
> *avctx, int frame_bytes);
> */
> int av_get_audio_frame_duration2(AVCodecParameters *par, int frame_bytes);
>
> -
> +#if FF_API_OLD_BSF
> typedef struct AVBitStreamFilterContext {
> void *priv_data;
> struct AVBitStreamFilter *filter;
> AVCodecParserContext *parser;
> struct AVBitStreamFilterContext *next;
> } AVBitStreamFilterContext;
> +#endif
> +
> +typedef struct AVBSFInternal AVBSFInternal;
> +
> +typedef struct AVBSFContext {
> + /**
> + * A class for logging and AVOptions
> + */
> + const AVClass *av_class;
> +
> + /**
> + * The bitstream filter this context is an instance of.
> + */
> + const struct AVBitStreamFilter *filter;
> +
> + /**
> + * Opaque libavcodec internal data. Must not be touched by the caller in
> any
> + * way.
> + */
> + AVBSFInternal *internal;
> +
> + /**
> + * Opaque filter-specific private data. If filter->priv_class is
> non-NULL,
> + * this is an AVOptions-enabled struct.
> + */
> + void *priv_data;
Why is this not in internal?
> +
> + /**
> + * Codec id this bitstream filter was initialized for.
> + * Set by libavcodec in av_bsf_alloc().
> + */
> + enum AVCodecID codec_id;
> +
> + /**
> + * Parameters of the input stream. Set by the caller before
> av_bsf_init().
> + */
> + AVCodecParameters *par_in;
Why is this not an argument to av_bsf_init() then? What about
mid-stream changes? Are they maybe handled by side-data? If so, how does
it interact with seeking (imagine seeking across format changes)?
> +
> + /**
> + * Parameters of the output stream. Set by the filter in av_bsf_init().
> + */
> + AVCodecParameters *par_out;
This and codec_id also makes me nervous... why have read-only fields
in a public struct? Can this change during filtering? If it's supposed
to be immutable, then you know some BSFs will start writing to it
anyway.
>
> + /**
> + * The timebase used for the timestamps of the input packets. Set by the
> + * caller before av_bsf_init().
> + */
> + AVRational time_base_in;
> +
> + /**
> + * The timebase used for the timestamps of the output packets. Set by the
> + * filter in av_bsf_init().
> + */
> + AVRational time_base_out;
> +} AVBSFContext;
>
> typedef struct AVBitStreamFilter {
> const char *name;
> +
> + /**
> + * A list of codec ids supported by the filter, terminated by
> + * AV_CODEC_ID_NONE.
> + * May be NULL, in that case the bitstream filter works with any codec
> id.
> + */
> + const enum AVCodecID *codec_ids;
> +
> + /**
> + * A class for the private data, used to declare bitstream filter private
> + * AVOptions. This field is NULL for bitstream filters that do not
> declare
> + * any options.
> + *
> + * If this field is non-NULL, the first member of the filter private data
> + * must be a pointer to AVClass, which will be set by libavcodec generic
> + * code to this class.
> + */
> + const AVClass *priv_class;
> +
> + /*****************************************************************
> + * No fields below this line are part of the public API. They
> + * may not be used outside of libavcodec and can be changed and
> + * removed at will.
> + * New public fields should be added right above.
> + *****************************************************************
> + */
> +
> int priv_data_size;
So priv_data_size and those callbacks were public before?
> - int (*filter)(AVBitStreamFilterContext *bsfc,
> - AVCodecContext *avctx, const char *args,
> - uint8_t **poutbuf, int *poutbuf_size,
> - const uint8_t *buf, int buf_size, int keyframe);
> - void (*close)(AVBitStreamFilterContext *bsfc);
> - struct AVBitStreamFilter *next;
> + int (*init)(AVBSFContext *ctx);
> + int (*filter)(AVBSFContext *ctx, AVPacket *pkt);
> + void (*close)(AVBSFContext *ctx);
> } AVBitStreamFilter;
>
> +#if FF_API_OLD_BSF
> +/**
> + * @deprecated the old bitstream filtering API (using
> AVBitStreamFilterContext)
> + * is deprecated. Use the new bitstream filtering API (using AVBSFContext).
> + */
> +attribute_deprecated
> void av_register_bitstream_filter(AVBitStreamFilter *bsf);
> +attribute_deprecated
> AVBitStreamFilterContext *av_bitstream_filter_init(const char *name);
> +attribute_deprecated
> int av_bitstream_filter_filter(AVBitStreamFilterContext *bsfc,
> AVCodecContext *avctx, const char *args,
> uint8_t **poutbuf, int *poutbuf_size,
> const uint8_t *buf, int buf_size, int
> keyframe);
> +attribute_deprecated
> void av_bitstream_filter_close(AVBitStreamFilterContext *bsf);
> -
> +attribute_deprecated
> AVBitStreamFilter *av_bitstream_filter_next(const AVBitStreamFilter *f);
> +#endif
I would appreciate it if API weren't "hard deprecated" immediately, but
only after say 1 release with the new API has been made.
> +
> +/**
> + * @return a bitstream filter with the specified name or NULL if no such
> + * bitstream filter exists.
> + */
> +const AVBitStreamFilter *av_bsf_get_by_name(const char *name);
> +
> +/**
> + * Iterate over all registered bitstream filters.
> + *
> + * @param opaque a pointer where libavcodec will store the iteration state.
> Must
> + * point to NULL to start the iteration.
> + *
> + * @return the next registered bitstream filter or NULL when the iteration is
> + * finished
> + */
> +const AVBitStreamFilter *av_bsf_next(void **opaque);
Not sure why that needs a new function. That opaque thing is weird.
> +
> +/**
> + * Allocate a context for a given bitstream filter. The caller must fill in
> the
> + * context parameters as described in the documentation and then call
> + * av_bsf_init() before sending any data to the filter.
> + *
> + * @param filter the filter for which to allocate an instance.
> + * @param ctx a pointer into which the pointer to the newly-allocated context
> + * will be written. It must be freed with av_bsf_free() after the
> + * filtering is done.
> + * @param codec_id a codec id identifying the kind of data that will be sent
> to
> + * the filter.
Why not just make it take a codec par?
> + *
> + * @return newly-allocated bitstream filter context or NULL on failure
Wrong doxygen.
> + */
> +int av_bsf_alloc(const AVBitStreamFilter *filter, AVBSFContext **ctx,
> + enum AVCodecID codec_id);
> +
> +/**
> + * Prepare the filter for use, after all the parameters and options have been
> + * set.
> + */
> +int av_bsf_init(AVBSFContext *ctx);
> +
> +/**
> + * Submit a packet for filtering.
> + *
> + * @param pkt the packet to filter. The bitstream filter will take ownership
> of
> + * the packet and reset the contents of pkt. pkt is not touched if an error
> occurs.
> + *
> + * @return 0 on success, a negative AVERROR on error.
> + */
> +int av_bsf_send_packet(AVBSFContext *ctx, AVPacket *pkt);
What happens if the user writes too many packets? Should it return
EAGAIN?
How do you signal EOF? How can the receive API signal EOF if you can't
signal it on input?
Why is there no way to reset EOF?
> +
> +/**
> + * Retrieve a filtered packet.
> + *
> + * @param[out] pkt this struct will be filled with the contents of the
> filtered
> + * packet. It is owned by the caller and must be freed using
> + * av_packet_unref() when it is no longer needed.
What happens on error? Does *pkt need to be initialized in any way?
(Such clarifications are missing from many Libav APIs.)
> + *
> + * @return 0 on success. AVERROR(EAGAIN) if more packets need to be sent to
> the
> + * filter (using av_bsf_send_packet()) to get more output. AVERROR_EOF if
> there
> + * will be no further output from the filter. Another negative AVERROR value
> if
> + * an error occurs.
> + *
> + * @note one input packet may result in several output packets, so after
> adding
> + * a packet with av_bsf_send_packet(), this function needs to be called
> + * repeatedly until it stops returning 0.
You might also add that the reverse can happen, merging packets.
> + */
> +int av_bsf_receive_packet(AVBSFContext *ctx, AVPacket *pkt);
> +
> +/**
> + * Free a bitstream filter context and everything associated with it; write
> NULL
> + * into the supplied pointer.
> + */
> +void av_bsf_free(AVBSFContext **ctx);
>
> /* memory */
>
> diff --git a/libavcodec/bitstream_filters.c b/libavcodec/bitstream_filters.c
> new file mode 100644
> index 0000000..5e406d7
> --- /dev/null
> +++ b/libavcodec/bitstream_filters.c
> @@ -0,0 +1,51 @@
> +/*
> + * This file is part of Libav.
> + *
> + * Libav is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * Libav is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with Libav; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> + */
> +
> +#include "config.h"
> +
> +#include "libavutil/common.h"
> +
> +#include "libavcodec/avcodec.h"
> +
> +static const AVBitStreamFilter *bitstream_filters[] = {
> + NULL,
> +};
> +
> +const AVBitStreamFilter *av_bsf_next(void **opaque)
> +{
> + uintptr_t i = (uintptr_t)*opaque;
> + const AVBitStreamFilter *f = bitstream_filters[i];
> +
> + if (f)
> + *opaque = (void*)(i + 1);
> +
> + return f;
> +}
> +
> +const AVBitStreamFilter *av_bsf_get_by_name(const char *name)
> +{
> + int i;
> +
> + for (i = 0; bitstream_filters[i]; i++) {
> + const AVBitStreamFilter *f = bitstream_filters[i];
> + if (!strcmp(f->name, name))
> + return f;
> + }
> +
> + return NULL;
> +}
> diff --git a/libavcodec/bsf.c b/libavcodec/bsf.c
> new file mode 100644
> index 0000000..ed4aef8
> --- /dev/null
> +++ b/libavcodec/bsf.c
> @@ -0,0 +1,217 @@
> +/*
> + * This file is part of Libav.
> + *
> + * Libav is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * Libav is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with Libav; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> + */
> +
> +#include <string.h>
> +
> +#include "libavutil/fifo.h"
> +#include "libavutil/log.h"
> +#include "libavutil/mem.h"
> +#include "libavutil/opt.h"
> +
> +#include "avcodec.h"
> +#include "bsf.h"
> +
> +struct AVBSFInternal {
> + AVFifoBuffer *fifo;
> + int eof;
> +};
> +
> +void av_bsf_free(AVBSFContext **pctx)
> +{
> + AVBSFContext *ctx;
> +
> + if (!pctx || !*pctx)
> + return;
> + ctx = *pctx;
> +
> + if (ctx->filter->close)
> + ctx->filter->close(ctx);
> + if (ctx->filter->priv_class && ctx->priv_data)
> + av_opt_free(ctx->priv_data);
> +
> + av_opt_free(ctx);
> +
> + if (ctx->internal) {
> + while (ctx->internal->fifo && av_fifo_size(ctx->internal->fifo)) {
> + AVPacket *pkt;
> + av_fifo_generic_read(ctx->internal->fifo, &pkt, sizeof(pkt),
> NULL);
> + av_packet_free(&pkt);
> + }
> + av_fifo_free(ctx->internal->fifo);
> + }
> + av_freep(&ctx->internal);
> + av_freep(&ctx->priv_data);
> +
> + avcodec_parameters_free(&ctx->par_in);
> + avcodec_parameters_free(&ctx->par_out);
> +
> + av_freep(pctx);
> +}
> +
> +static const AVClass bsf_class = {
> + .class_name = "AVBSFContext",
> + .item_name = av_default_item_name,
> + .version = LIBAVUTIL_VERSION_INT,
> +};
> +
> +int av_bsf_alloc(const AVBitStreamFilter *filter, AVBSFContext **pctx,
> + enum AVCodecID codec_id)
> +{
> + AVBSFContext *ctx;
> + int i, ret;
> +
> + ctx = av_mallocz(sizeof(*ctx));
> + if (!ctx)
> + return AVERROR(ENOMEM);
> +
> + ctx->av_class = &bsf_class;
I would have thought it depends on the filter...
> + ctx->filter = filter;
> +
> + /* check that the codec is supported */
> + if (filter->codec_ids) {
> + for (i = 0; filter->codec_ids[i] != AV_CODEC_ID_NONE; i++)
> + if (codec_id == filter->codec_ids[i])
> + break;
> + if (filter->codec_ids[i] == AV_CODEC_ID_NONE) {
> + const AVCodecDescriptor *desc = avcodec_descriptor_get(codec_id);
> + av_log(ctx, AV_LOG_ERROR, "Codec '%s' (%d) is not supported by
> the "
> + "bitstream filter '%s'. Supported codecs are: ",
> + desc ? desc->name : "unknown", codec_id, filter->name);
> + for (i = 0; filter->codec_ids[i] != AV_CODEC_ID_NONE; i++) {
> + desc = avcodec_descriptor_get(filter->codec_ids[i]);
> + av_log(ctx, AV_LOG_ERROR, "%s (%d) ",
> + desc ? desc->name : "unknown", filter->codec_ids[i]);
> + }
> + av_log(ctx, AV_LOG_ERROR, "\n");
> + ret = AVERROR(EINVAL);
> + goto fail;
> + }
> + }
> +
> + ctx->par_in = avcodec_parameters_alloc();
> + ctx->par_out = avcodec_parameters_alloc();
> + if (!ctx->par_in || !ctx->par_out) {
> + ret = AVERROR(ENOMEM);
> + goto fail;
> + }
> +
> + ctx->internal = av_mallocz(sizeof(*ctx->internal));
> + if (!ctx->internal) {
> + ret = AVERROR(ENOMEM);
> + goto fail;
> + }
> +
> + ctx->internal->fifo = av_fifo_alloc(8 * sizeof(AVPacket*));
> + if (!ctx->internal->fifo) {
> + ret = AVERROR(ENOMEM);
> + goto fail;
> + }
> +
> + av_opt_set_defaults(ctx);
> +
> + /* allocate priv data and init private options */
> + if (filter->priv_data_size) {
> + ctx->priv_data = av_mallocz(filter->priv_data_size);
> + if (!ctx->priv_data) {
> + ret = AVERROR(ENOMEM);
> + goto fail;
> + }
> + if (filter->priv_class) {
> + *(const AVClass **)ctx->priv_data = filter->priv_class;
> + av_opt_set_defaults(ctx->priv_data);
> + }
> + }
> +
> + *pctx = ctx;
> + return 0;
> +fail:
> + av_bsf_free(&ctx);
> + return ret;
> +}
> +
> +int av_bsf_init(AVBSFContext *ctx)
> +{
> + int ret;
> +
> + /* initialize output parameters to be the same as input
> + * init below might overwrite that */
> + ret = avcodec_parameters_copy(ctx->par_out, ctx->par_in);
> + if (ret < 0)
> + return ret;
> +
> + ctx->time_base_out = ctx->time_base_in;
> +
> + if (ctx->filter->init) {
> + ret = ctx->filter->init(ctx);
> + if (ret < 0)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +int av_bsf_send_packet(AVBSFContext *ctx, AVPacket *pkt)
> +{
> + AVPacket *pkt_internal;
> +
> + if (!pkt || !pkt->data) {
> + ctx->internal->eof = 1;
> + return 0;
> + }
> +
> + if (ctx->internal->eof) {
> + av_log(ctx, AV_LOG_ERROR, "A non-NULL packet sent after an EOF.\n");
> + return AVERROR(EINVAL);
> + }
> +
> + if (av_fifo_space(ctx->internal->fifo) < sizeof(pkt)) {
> + int ret = av_fifo_realloc2(ctx->internal->fifo,
> + av_fifo_size(ctx->internal->fifo) + 8 *
> sizeof(pkt));
> + if (ret < 0)
> + return ret;
> + }
> +
> + pkt_internal = av_packet_alloc();
> + if (!pkt_internal)
> + return AVERROR(ENOMEM);
> +
> + av_packet_move_ref(pkt_internal, pkt);
> + av_fifo_generic_write(ctx->internal->fifo, &pkt_internal,
> sizeof(pkt_internal), NULL);
> +
> + return 0;
> +}
> +
> +int av_bsf_receive_packet(AVBSFContext *ctx, AVPacket *pkt)
> +{
> + return ctx->filter->filter(ctx, pkt);
> +}
> +
> +int ff_bsf_get_packet(AVBSFContext *ctx, AVPacket **pkt)
> +{
> + AVBSFInternal *in = ctx->internal;
> +
> + if (in->eof)
> + return AVERROR_EOF;
> +
> + if (av_fifo_size(in->fifo) < sizeof(*pkt))
> + return AVERROR(EAGAIN);
> +
> + av_fifo_generic_read(in->fifo, pkt, sizeof(*pkt), NULL);
> +
> + return 0;
> +}
> diff --git a/libavcodec/bsf.h b/libavcodec/bsf.h
> new file mode 100644
> index 0000000..cb18a05
> --- /dev/null
> +++ b/libavcodec/bsf.h
> @@ -0,0 +1,31 @@
> +/*
> + * This file is part of Libav.
> + *
> + * Libav is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * Libav is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with Libav; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> + */
> +
> +#ifndef AVCODEC_BSF_H
> +#define AVCODEC_BSF_H
> +
> +#include "avcodec.h"
> +
> +/**
> + * Called by the biststream filters to get the next packet for filtering.
> + * The filter is responsible for either freeing the packet or passing it to
> the
> + * caller.
> + */
> +int ff_bsf_get_packet(AVBSFContext *ctx, AVPacket **pkt);
> +
> +#endif /* AVCODEC_BSF_H */
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index d247c09..59a0d40 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -197,5 +197,8 @@
> #ifndef FF_API_PRIVATE_OPT
> #define FF_API_PRIVATE_OPT (LIBAVCODEC_VERSION_MAJOR < 59)
> #endif
> +#ifndef FF_API_OLD_BSF
> +#define FF_API_OLD_BSF (LIBAVCODEC_VERSION_MAJOR < 59)
> +#endif
>
> #endif /* AVCODEC_VERSION_H */
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
