[FFmpeg-devel,03/20] lavc: Add coded bitstream read/write API

Submitted by Mark Thompson on Sept. 12, 2017, 11:43 p.m.

Details

Message ID 20170912234410.14093-4-sw@jkqxz.net
State New
Headers show

Commit Message

Mark Thompson Sept. 12, 2017, 11:43 p.m.
(cherry picked from commit 18f1706f331bf5dd565774eae680508c8d3a97ad)
(cherry picked from commit 44cde38c8acbef7d5250e6d1b52b1020871e093b)
---
 configure                 |   1 +
 libavcodec/Makefile       |   1 +
 libavcodec/cbs.c          | 466 ++++++++++++++++++++++++++++++++++++++++++++++
 libavcodec/cbs.h          | 274 +++++++++++++++++++++++++++
 libavcodec/cbs_internal.h |  83 +++++++++
 5 files changed, 825 insertions(+)
 create mode 100644 libavcodec/cbs.c
 create mode 100644 libavcodec/cbs.h
 create mode 100644 libavcodec/cbs_internal.h

Comments

Michael Niedermayer Sept. 14, 2017, 12:42 a.m.
On Wed, Sep 13, 2017 at 12:43:53AM +0100, Mark Thompson wrote:
> (cherry picked from commit 18f1706f331bf5dd565774eae680508c8d3a97ad)
> (cherry picked from commit 44cde38c8acbef7d5250e6d1b52b1020871e093b)
> ---
>  configure                 |   1 +
>  libavcodec/Makefile       |   1 +
>  libavcodec/cbs.c          | 466 ++++++++++++++++++++++++++++++++++++++++++++++
>  libavcodec/cbs.h          | 274 +++++++++++++++++++++++++++
>  libavcodec/cbs_internal.h |  83 +++++++++
>  5 files changed, 825 insertions(+)
>  create mode 100644 libavcodec/cbs.c
>  create mode 100644 libavcodec/cbs.h
>  create mode 100644 libavcodec/cbs_internal.h
> 
> diff --git a/configure b/configure
> index 4dd11efe5e..2e84b1f892 100755
> --- a/configure
> +++ b/configure
> @@ -2112,6 +2112,7 @@ CONFIG_EXTRA="
>      blockdsp
>      bswapdsp
>      cabac
> +    cbs
>      dirac_parse
>      dvprofile
>      exif
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 999632cf9e..d9612d68d0 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -59,6 +59,7 @@ OBJS-$(CONFIG_AUDIODSP)                += audiodsp.o
>  OBJS-$(CONFIG_BLOCKDSP)                += blockdsp.o
>  OBJS-$(CONFIG_BSWAPDSP)                += bswapdsp.o
>  OBJS-$(CONFIG_CABAC)                   += cabac.o
> +OBJS-$(CONFIG_CBS)                     += cbs.o
>  OBJS-$(CONFIG_CRYSTALHD)               += crystalhd.o
>  OBJS-$(CONFIG_DCT)                     += dct.o dct32_fixed.o dct32_float.o
>  OBJS-$(CONFIG_ERROR_RESILIENCE)        += error_resilience.o
> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
> new file mode 100644
> index 0000000000..13bc25728f
> --- /dev/null
> +++ b/libavcodec/cbs.c
> @@ -0,0 +1,466 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg 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.
> + *
> + * FFmpeg 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 FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <string.h>
> +
> +#include "config.h"
> +
> +#include "libavutil/avassert.h"
> +#include "libavutil/common.h"
> +
> +#include "cbs.h"
> +#include "cbs_internal.h"
> +
> +
> +static const CodedBitstreamType *cbs_type_table[] = {
> +};
> +
> +int ff_cbs_init(CodedBitstreamContext *ctx,
> +                enum AVCodecID codec_id, void *log_ctx)
> +{
> +    const CodedBitstreamType *type;
> +    int i;
> +
> +    type = NULL;
> +    for (i = 0; i < FF_ARRAY_ELEMS(cbs_type_table); i++) {
> +        if (cbs_type_table[i]->codec_id == codec_id) {
> +            type = cbs_type_table[i];
> +            break;
> +        }
> +    }
> +    if (!type)
> +        return AVERROR(EINVAL);
> +
> +    ctx->log_ctx = log_ctx;
> +    ctx->codec   = type;
> +
> +    ctx->priv_data = av_mallocz(ctx->codec->priv_data_size);
> +    if (!ctx->priv_data)
> +        return AVERROR(ENOMEM);
> +
> +    ctx->decompose_unit_types = NULL;
> +
> +    ctx->trace_enable = 0;
> +    ctx->trace_level  = AV_LOG_TRACE;
> +
> +    return 0;
> +}
> +
> +void ff_cbs_close(CodedBitstreamContext *ctx)
> +{
> +    if (ctx->codec && ctx->codec->close)
> +        ctx->codec->close(ctx);
> +
> +    av_freep(&ctx->priv_data);
> +}
> +
> +static void cbs_unit_uninit(CodedBitstreamContext *ctx,
> +                            CodedBitstreamUnit *unit)
> +{
> +    if (ctx->codec->free_unit && unit->content && !unit->content_external)
> +        ctx->codec->free_unit(unit);
> +
> +    av_freep(&unit->data);
> +    unit->data_size = 0;
> +    unit->data_bit_padding = 0;
> +}
> +
> +void ff_cbs_fragment_uninit(CodedBitstreamContext *ctx,
> +                            CodedBitstreamFragment *frag)
> +{
> +    int i;
> +
> +    for (i = 0; i < frag->nb_units; i++)
> +        cbs_unit_uninit(ctx, &frag->units[i]);
> +    av_freep(&frag->units);
> +    frag->nb_units = 0;
> +
> +    av_freep(&frag->data);
> +    frag->data_size        = 0;
> +    frag->data_bit_padding = 0;
> +}
> +
> +static int cbs_read_fragment_content(CodedBitstreamContext *ctx,
> +                                     CodedBitstreamFragment *frag)
> +{
> +    int err, i, j;
> +
> +    for (i = 0; i < frag->nb_units; i++) {
> +        if (ctx->decompose_unit_types) {
> +            for (j = 0; j < ctx->nb_decompose_unit_types; j++) {
> +                if (ctx->decompose_unit_types[j] == frag->units[i].type)
> +                    break;
> +            }
> +            if (j >= ctx->nb_decompose_unit_types)
> +                continue;
> +        }
> +
> +        err = ctx->codec->read_unit(ctx, &frag->units[i]);
> +        if (err == AVERROR(ENOSYS)) {
> +            av_log(ctx->log_ctx, AV_LOG_WARNING,
> +                   "Decomposition unimplemented for unit %d "
> +                   "(type %d).\n", i, frag->units[i].type);
> +        } else if (err < 0) {
> +            av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to read unit %d "
> +                   "(type %d).\n", i, frag->units[i].type);
> +            return err;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
> +                          CodedBitstreamFragment *frag,
> +                          const AVCodecParameters *par)
> +{
> +    int err;
> +
> +    memset(frag, 0, sizeof(*frag));
> +
> +    frag->data      = par->extradata;
> +    frag->data_size = par->extradata_size;
> +
> +    err = ctx->codec->split_fragment(ctx, frag, 1);
> +    if (err < 0)
> +        return err;
> +
> +    frag->data      = NULL;
> +    frag->data_size = 0;
> +
> +    return cbs_read_fragment_content(ctx, frag);
> +}
> +
> +int ff_cbs_read_packet(CodedBitstreamContext *ctx,
> +                       CodedBitstreamFragment *frag,
> +                       const AVPacket *pkt)
> +{
> +    int err;
> +
> +    memset(frag, 0, sizeof(*frag));
> +
> +    frag->data      = pkt->data;
> +    frag->data_size = pkt->size;
> +
> +    err = ctx->codec->split_fragment(ctx, frag, 0);
> +    if (err < 0)
> +        return err;
> +
> +    frag->data      = NULL;
> +    frag->data_size = 0;
> +
> +    return cbs_read_fragment_content(ctx, frag);
> +}
> +
> +int ff_cbs_read(CodedBitstreamContext *ctx,
> +                CodedBitstreamFragment *frag,
> +                const uint8_t *data, size_t size)
> +{
> +    int err;
> +
> +    memset(frag, 0, sizeof(*frag));
> +
> +    // (We won't write to this during split.)
> +    frag->data      = (uint8_t*)data;
> +    frag->data_size = size;
> +
> +    err = ctx->codec->split_fragment(ctx, frag, 0);
> +    if (err < 0)
> +        return err;
> +
> +    frag->data      = NULL;
> +    frag->data_size = 0;
> +
> +    return cbs_read_fragment_content(ctx, frag);
> +}
> +
> +
> +int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx,
> +                               CodedBitstreamFragment *frag)
> +{
> +    int err, i;
> +
> +    for (i = 0; i < frag->nb_units; i++) {
> +        if (!frag->units[i].content)
> +            continue;
> +
> +        err = ctx->codec->write_unit(ctx, &frag->units[i]);
> +        if (err < 0) {
> +            av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to write unit %d "
> +                   "(type %d).\n", i, frag->units[i].type);
> +            return err;
> +        }
> +    }
> +
> +    err = ctx->codec->assemble_fragment(ctx, frag);
> +    if (err < 0) {
> +        av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to assemble fragment.\n");
> +        return err;
> +    }
> +
> +    return 0;
> +}
> +
> +int ff_cbs_write_extradata(CodedBitstreamContext *ctx,
> +                           AVCodecParameters *par,
> +                           CodedBitstreamFragment *frag)
> +{
> +    int err;
> +
> +    err = ff_cbs_write_fragment_data(ctx, frag);
> +    if (err < 0)
> +        return err;
> +
> +    av_freep(&par->extradata);
> +
> +    par->extradata = av_malloc(frag->data_size +
> +                               AV_INPUT_BUFFER_PADDING_SIZE);
> +    if (!par->extradata)
> +        return AVERROR(ENOMEM);
> +
> +    memcpy(par->extradata, frag->data, frag->data_size);
> +    memset(par->extradata + frag->data_size, 0,
> +           AV_INPUT_BUFFER_PADDING_SIZE);
> +    par->extradata_size = frag->data_size;
> +
> +    return 0;
> +}
> +
> +int ff_cbs_write_packet(CodedBitstreamContext *ctx,
> +                        AVPacket *pkt,
> +                        CodedBitstreamFragment *frag)
> +{
> +    int err;
> +
> +    err = ff_cbs_write_fragment_data(ctx, frag);
> +    if (err < 0)
> +        return err;
> +

> +    av_new_packet(pkt, frag->data_size);
> +    if (err < 0)
> +        return err;

that does not work


> +
> +    memcpy(pkt->data, frag->data, frag->data_size);
> +    pkt->size = frag->data_size;
> +
> +    return 0;
> +}
> +
> +
> +void ff_cbs_trace_header(CodedBitstreamContext *ctx,
> +                         const char *name)
> +{
> +    if (!ctx->trace_enable)
> +        return;
> +
> +    av_log(ctx->log_ctx, ctx->trace_level, "%s\n", name);
> +}
> +
> +void ff_cbs_trace_syntax_element(CodedBitstreamContext *ctx, int position,
> +                                 const char *name, const char *bits,
> +                                 int64_t value)
> +{
> +    size_t name_len, bits_len;
> +    int pad;
> +
> +    if (!ctx->trace_enable)
> +        return;
> +
> +    av_assert0(value >= INT_MIN && value <= UINT32_MAX);
> +
> +    name_len = strlen(name);
> +    bits_len = strlen(bits);
> +
> +    if (name_len + bits_len > 60)
> +        pad = bits_len + 2;
> +    else
> +        pad = 61 - name_len;
> +
> +    av_log(ctx->log_ctx, ctx->trace_level, "%-10d  %s%*s = %"PRId64"\n",
> +           position, name, pad, bits, value);
> +}
> +

> +int ff_cbs_read_unsigned(CodedBitstreamContext *ctx, GetBitContext *gbc,
> +                         int width, const char *name, uint32_t *write_to,
> +                         uint32_t range_min, uint32_t range_max)
> +{
> +    uint32_t value;
> +    int position;
> +
> +    av_assert0(width <= 32);

if you assert validity, you should also assert non negativity


> +
> +    if (get_bits_left(gbc) < width) {
> +        av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid value at "
> +               "%s: bitstream ended.\n", name);
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    if (ctx->trace_enable)
> +        position = get_bits_count(gbc);
> +
> +    value = get_bits_long(gbc, width);
> +
> +    if (ctx->trace_enable) {
> +        char bits[33];
> +        int i;
> +        for (i = 0; i < width; i++)
> +            bits[i] = value >> (width - i - 1) & 1 ? '1' : '0';
> +        bits[i] = 0;
> +
> +        ff_cbs_trace_syntax_element(ctx, position, name, bits, value);
> +    }
> +
> +    if (value < range_min || value > range_max) {
> +        av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
> +               "%"PRIu32", but must be in [%"PRIu32",%"PRIu32"].\n",
> +               name, value, range_min, range_max);
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    *write_to = value;
> +    return 0;
> +}
> +
> +int ff_cbs_write_unsigned(CodedBitstreamContext *ctx, PutBitContext *pbc,
> +                          int width, const char *name, uint32_t value,
> +                          uint32_t range_min, uint32_t range_max)
> +{
> +    av_assert0(width <= 32);
> +
> +    if (value < range_min || value > range_max) {
> +        av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
> +               "%"PRIu32", but must be in [%"PRIu32",%"PRIu32"].\n",
> +               name, value, range_min, range_max);
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    if (put_bits_left(pbc) < width)
> +        return AVERROR(ENOSPC);
> +
> +    if (ctx->trace_enable) {
> +        char bits[33];
> +        int i;
> +        for (i = 0; i < width; i++)
> +            bits[i] = value >> (width - i - 1) & 1 ? '1' : '0';
> +        bits[i] = 0;
> +
> +        ff_cbs_trace_syntax_element(ctx, put_bits_count(pbc), name, bits, value);
> +    }
> +
> +    if (width < 32)
> +        put_bits(pbc, width, value);
> +    else
> +        put_bits32(pbc, value);
> +
> +    return 0;
> +}
> +
> +

> +static int cbs_insert_unit(CodedBitstreamContext *ctx,
> +                           CodedBitstreamFragment *frag,
> +                           int position)
> +{
> +    CodedBitstreamUnit *units;
> +
> +    units = av_malloc_array(frag->nb_units + 1, sizeof(*units));
> +    if (!units)
> +        return AVERROR(ENOMEM);
> +
> +    if (position > 0)
> +        memcpy(units, frag->units, position * sizeof(*units));
> +    if (position < frag->nb_units)
> +        memcpy(units + position + 1, frag->units + position,
> +               (frag->nb_units - position) * sizeof(*units));
> +
> +    memset(units + position, 0, sizeof(*units));
> +
> +    av_freep(&frag->units);
> +    frag->units = units;
> +    ++frag->nb_units;
> +
> +    return 0;
> +}

re-creating the whole array on each insertion seems wastefull of
CPU time


> +
> +int ff_cbs_insert_unit_content(CodedBitstreamContext *ctx,
> +                               CodedBitstreamFragment *frag,
> +                               int position, uint32_t type,
> +                               void *content)
> +{
> +    int err;
> +
> +    if (position == -1)
> +        position = frag->nb_units;
> +    av_assert0(position >= 0 && position <= frag->nb_units);
> +
> +    err = cbs_insert_unit(ctx, frag, position);
> +    if (err < 0)
> +        return err;
> +
> +    frag->units[position].type             = type;
> +    frag->units[position].content          = content;
> +    frag->units[position].content_external = 1;
> +
> +    return 0;
> +}
> +
> +int ff_cbs_insert_unit_data(CodedBitstreamContext *ctx,
> +                            CodedBitstreamFragment *frag,
> +                            int position, uint32_t type,
> +                            uint8_t *data, size_t data_size)
> +{
> +    int err;
> +
> +    if (position == -1)
> +        position = frag->nb_units;
> +    av_assert0(position >= 0 && position <= frag->nb_units);
> +
> +    err = cbs_insert_unit(ctx, frag, position);
> +    if (err < 0)
> +        return err;
> +
> +    frag->units[position].type      = type;
> +    frag->units[position].data      = data;
> +    frag->units[position].data_size = data_size;
> +
> +    return 0;
> +}
> +
> +int ff_cbs_delete_unit(CodedBitstreamContext *ctx,
> +                       CodedBitstreamFragment *frag,
> +                       int position)
> +{
> +    if (position < 0 || position >= frag->nb_units)
> +        return AVERROR(EINVAL);
> +
> +    cbs_unit_uninit(ctx, &frag->units[position]);
> +
> +    --frag->nb_units;
> +
> +    if (frag->nb_units == 0) {
> +        av_freep(&frag->units);
> +
> +    } else {
> +        memmove(frag->units + position,
> +                frag->units + position + 1,
> +                (frag->nb_units - position) * sizeof(*frag->units));
> +
> +        // Don't bother reallocating the unit array.
> +    }
> +
> +    return 0;
> +}

> diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
> new file mode 100644
> index 0000000000..eeaff379e5
> --- /dev/null
> +++ b/libavcodec/cbs.h
> @@ -0,0 +1,274 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg 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.
> + *
> + * FFmpeg 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 FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef AVCODEC_CBS_H
> +#define AVCODEC_CBS_H
> +
> +#include <stddef.h>
> +#include <stdint.h>
> +
> +#include "avcodec.h"
> +
> +
> +struct CodedBitstreamType;
> +
> +/**
> + * Coded bitstream unit structure.
> + *
> + * A bitstream unit the the smallest element of a bitstream which

the the ?


> + * is meaningful on its own.  For example, an H.264 NAL unit.
> + *
> + * See the codec-specific header for the meaning of this for any
> + * particular codec.
> + */
> +typedef struct CodedBitstreamUnit {
> +    /**
> +     * Codec-specific type of this unit.
> +     */
> +    uint32_t type;
> +
> +    /**
> +     * Pointer to the bitstream form of this unit.
> +     *
> +     * May be NULL if the unit currently only exists in decomposed form.
> +     */
> +    uint8_t *data;
> +    /**
> +     * The number of bytes in the bitstream (including any padding bits
> +     * in the final byte).
> +     */
> +    size_t   data_size;

> +    /**
> +     * The number of bits which should be ignored in the final byte.
> +     *
> +     * This supports non-byte-aligned bitstreams.
> +     */
> +    size_t   data_bit_padding;

theres no need for this to be size_t, a byte can only contain up to
8 bits


> +
> +    /**
> +     * Pointer to the decomposed form of this unit.
> +     *
> +     * The type of this structure depends on both the codec and the
> +     * type of this unit.  May be NULL if the unit only exists in
> +     * bitstream form.
> +     */
> +    void *content;
> +    /**
> +     * Whether the content was supplied externally.
> +     *
> +     * If so, it should not be freed when freeing the unit.
> +     */
> +    int   content_external;
> +} CodedBitstreamUnit;
> +
> +/**
> + * Coded bitstream fragment structure, combining one or more units.
> + *
> + * This is any sequence of units.  It need not form some greater whole,
> + * though in many cases it will.  For example, an H.264 access unit,
> + * which is composed of a sequence of H.264 NAL units.
> + */
> +typedef struct CodedBitstreamFragment {
> +    /**
> +     * Pointer to the bitstream form of this fragment.
> +     *
> +     * May be NULL if the fragment only exists as component units.
> +     */
> +    uint8_t *data;
> +    /**
> +     * The number of bytes in the bitstream.
> +     *
> +     * The number of bytes in the bitstream (including any padding bits
> +     * in the final byte).
> +     */
> +    size_t   data_size;
> +    /**
> +     * The number of bits which should be ignored in the final byte.
> +     */
> +    size_t data_bit_padding;
> +
> +    /**
> +     * Number of units in this fragment.
> +     *
> +     * This may be zero if the fragment only exists in bistream form
> +     * and has not been decomposed.

what is a biStream form ?


> +     */
> +    int              nb_units;
> +    /**
> +     * Pointer to an array of units of length nb_units.
> +     *
> +     * Must be NULL if nb_units is zero.
> +     */
> +    CodedBitstreamUnit *units;
> +} CodedBitstreamFragment;
> +
> +/**
> + * Context structure for coded bitstream operations.
> + */
> +typedef struct CodedBitstreamContext {
> +    /**
> +     * Logging context to be passed to all av_log() calls associated
> +     * with this context.
> +     */
> +    void *log_ctx;
> +
> +    /**
> +     * Internal codec-specific hooks.
> +     */
> +    const struct CodedBitstreamType *codec;
> +
> +    /**
> +     * Internal codec-specific data.
> +     *
> +     * This contains any information needed when reading/writing
> +     * bitsteams which will not necessarily be present in a fragment.
> +     * For example, for H.264 it contains all currently visible
> +     * parameter sets - they are required to determine the bitstream
> +     * syntax but need not be present in every access unit.
> +     */
> +    void *priv_data;
> +

> +    /**
> +     * Array of unit types which should be decomposed when reading.
> +     *
> +     * Types not in this list will be available in bitstream form only.
> +     * If NULL, all supported types will be decomposed.
> +     */
> +    uint32_t *decompose_unit_types;

this should not be a generic integer.
a typedef or enum would be better


> +    /**
> +     * Length of the decompose_unit_types array.
> +     */
> +    int    nb_decompose_unit_types;
> +
> +    /**
> +     * Enable trace output during read/write operations.
> +     */
> +    int trace_enable;
> +    /**
> +     * Log level to use for trace output.
> +     *
> +     * From AV_LOG_*; defaults to AV_LOG_TRACE.
> +     */
> +    int trace_level;
> +} CodedBitstreamContext;
> +
> +

> +/**
> + * Initialise a new context for the given codec.
> + */
> +int ff_cbs_init(CodedBitstreamContext *ctx,
> +                enum AVCodecID codec_id, void *log_ctx);

allocating the context within the init function would make the
API more robust with extensions to the context and potential use from
other libs.
It is also how most of our APIs work


> +
> +/**
> + * Close a context and free all internal state.
> + */
> +void ff_cbs_close(CodedBitstreamContext *ctx);
> +
> +

> +/**
> + * Read the extradata bitstream found in codec parameters into a
> + * fragment, then split into units and decompose.
> + *
> + * This also updates the internal state, so will need to be called for
> + * codecs with extradata to read parameter sets necessary for further
> + * parsing even if the fragment itself is not desired.
> + */
> +int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
> +                          CodedBitstreamFragment *frag,
> +                          const AVCodecParameters *par);

are any fields other than extradata and size needed from
AVCodecParameters ?
if not then it may be more flexible to just pass the extradata and size


> +
> +/**
> + * Read the data bitstream from a packet into a fragment, then
> + * split into units and decompose.
> + */
> +int ff_cbs_read_packet(CodedBitstreamContext *ctx,
> +                       CodedBitstreamFragment *frag,
> +                       const AVPacket *pkt);
> +
> +/**
> + * Read a bitstream from a memory region into a fragment, then
> + * split into units and decompose.
> + */
> +int ff_cbs_read(CodedBitstreamContext *ctx,
> +                CodedBitstreamFragment *frag,
> +                const uint8_t *data, size_t size);
> +
> +
> +/**
> + * Write the content of the fragment to its own internal buffer.
> + *
> + * Writes the content of all units and then assembles them into a new
> + * data buffer.  When modifying the content of decomposed units, this
> + * can be used to regenerate the bitstream form of units or the whole
> + * fragment so that it can be extracted for other use.
> + */
> +int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx,
> +                               CodedBitstreamFragment *frag);
> +

> +/**
> + * Write the bitstream of a fragment to the extradata in codec parameters.
> + */
> +int ff_cbs_write_extradata(CodedBitstreamContext *ctx,
> +                           AVCodecParameters *par,
> +                           CodedBitstreamFragment *frag);

This is missing documentation about allocation. Will the call allocate
the extradata, should the caller allocate, is it allowed to be allocated
before the call ?

Also similar to above, passing a pointer and pointer to size may be
more flexible


> +
> +/**
> + * Write the bitstream of a fragment to a packet.
> + */
> +int ff_cbs_write_packet(CodedBitstreamContext *ctx,
> +                        AVPacket *pkt,
> +                        CodedBitstreamFragment *frag);
> +
> +
> +/**
> + * Free all allocated memory in a fragment.
> + */
> +void ff_cbs_fragment_uninit(CodedBitstreamContext *ctx,
> +                            CodedBitstreamFragment *frag);
> +
> +
> +/**
> + * Insert a new unit into a fragment with the given content.
> + *
> + * The content structure continues to be owned by the caller, and
> + * will not be freed when the unit is.
> + */
> +int ff_cbs_insert_unit_content(CodedBitstreamContext *ctx,
> +                               CodedBitstreamFragment *frag,
> +                               int position, uint32_t type,
> +                               void *content);
> +
> +/**
> + * Insert a new unit into a fragment with the given data bitstream.
> + *
> + * The data buffer will be owned by the unit after this operation.
> + */
> +int ff_cbs_insert_unit_data(CodedBitstreamContext *ctx,
> +                            CodedBitstreamFragment *frag,
> +                            int position, uint32_t type,
> +                            uint8_t *data, size_t data_size);
> +
> +/**
> + * Delete a unit from a fragment and free all memory it uses.
> + */
> +int ff_cbs_delete_unit(CodedBitstreamContext *ctx,
> +                       CodedBitstreamFragment *frag,
> +                       int position);
> +
> +
> +#endif /* AVCODEC_CBS_H */
> diff --git a/libavcodec/cbs_internal.h b/libavcodec/cbs_internal.h
> new file mode 100644
> index 0000000000..3bbc3355c2
> --- /dev/null
> +++ b/libavcodec/cbs_internal.h
> @@ -0,0 +1,83 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg 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.
> + *
> + * FFmpeg 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 FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef AVCODEC_CBS_INTERNAL_H
> +#define AVCODEC_CBS_INTERNAL_H
> +
> +#include "avcodec.h"
> +#include "cbs.h"
> +#include "get_bits.h"
> +#include "put_bits.h"
> +
> +
> +typedef struct CodedBitstreamType {
> +    enum AVCodecID codec_id;
> +
> +    size_t priv_data_size;
> +

> +    // Split frag->data into coded bitstream units, creating the
> +    // frag->units array.  Fill data but not content on each unit.
> +    int (*split_fragment)(CodedBitstreamContext *ctx,
> +                          CodedBitstreamFragment *frag,
> +                          int header);

The "header" parameter lacks documentation


> +
> +    // Read the unit->data bitstream and decompose it, creating
> +    // unit->content.
> +    int (*read_unit)(CodedBitstreamContext *ctx,
> +                     CodedBitstreamUnit *unit);
> +
> +    // Write the unit->data bitstream from unit->content.
> +    int (*write_unit)(CodedBitstreamContext *ctx,
> +                      CodedBitstreamUnit *unit);
> +
> +    // Read the data from all of frag->units and assemble it into
> +    // a bitstream for the whole fragment.
> +    int (*assemble_fragment)(CodedBitstreamContext *ctx,
> +                             CodedBitstreamFragment *frag);
> +
> +    // Free the content and data of a single unit.
> +    void (*free_unit)(CodedBitstreamUnit *unit);
> +
> +    // Free the codec internal state.
> +    void (*close)(CodedBitstreamContext *ctx);
> +} CodedBitstreamType;
> +
> +
> +// Helper functions for trace output.
> +
> +void ff_cbs_trace_header(CodedBitstreamContext *ctx,
> +                         const char *name);
> +
> +void ff_cbs_trace_syntax_element(CodedBitstreamContext *ctx,
> +                                 int position, const char *name,
> +                                 const char *bitstring, int64_t value);
> +
> +
> +// Helper functions for read/write of common bitstream elements, including
> +// generation of trace output.
> +
> +int ff_cbs_read_unsigned(CodedBitstreamContext *ctx, GetBitContext *gbc,
> +                         int width, const char *name, uint32_t *write_to,
> +                         uint32_t range_min, uint32_t range_max);
> +
> +int ff_cbs_write_unsigned(CodedBitstreamContext *ctx, PutBitContext *pbc,
> +                          int width, const char *name, uint32_t value,
> +                          uint32_t range_min, uint32_t range_max);
> +
> +
> +#endif /* AVCODEC_CBS_INTERNAL_H */
> -- 
> 2.11.0
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Mark Thompson Sept. 14, 2017, 7:31 a.m.
On 14/09/17 01:42, Michael Niedermayer wrote:
> On Wed, Sep 13, 2017 at 12:43:53AM +0100, Mark Thompson wrote:
>> (cherry picked from commit 18f1706f331bf5dd565774eae680508c8d3a97ad)
>> (cherry picked from commit 44cde38c8acbef7d5250e6d1b52b1020871e093b)
>> ---
>>  configure                 |   1 +
>>  libavcodec/Makefile       |   1 +
>>  libavcodec/cbs.c          | 466 ++++++++++++++++++++++++++++++++++++++++++++++
>>  libavcodec/cbs.h          | 274 +++++++++++++++++++++++++++
>>  libavcodec/cbs_internal.h |  83 +++++++++
>>  5 files changed, 825 insertions(+)
>>  create mode 100644 libavcodec/cbs.c
>>  create mode 100644 libavcodec/cbs.h
>>  create mode 100644 libavcodec/cbs_internal.h
>>
>> diff --git a/configure b/configure
>> index 4dd11efe5e..2e84b1f892 100755
>> --- a/configure
>> +++ b/configure
>> @@ -2112,6 +2112,7 @@ CONFIG_EXTRA="
>>      blockdsp
>>      bswapdsp
>>      cabac
>> +    cbs
>>      dirac_parse
>>      dvprofile
>>      exif
>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>> index 999632cf9e..d9612d68d0 100644
>> --- a/libavcodec/Makefile
>> +++ b/libavcodec/Makefile
>> @@ -59,6 +59,7 @@ OBJS-$(CONFIG_AUDIODSP)                += audiodsp.o
>>  OBJS-$(CONFIG_BLOCKDSP)                += blockdsp.o
>>  OBJS-$(CONFIG_BSWAPDSP)                += bswapdsp.o
>>  OBJS-$(CONFIG_CABAC)                   += cabac.o
>> +OBJS-$(CONFIG_CBS)                     += cbs.o
>>  OBJS-$(CONFIG_CRYSTALHD)               += crystalhd.o
>>  OBJS-$(CONFIG_DCT)                     += dct.o dct32_fixed.o dct32_float.o
>>  OBJS-$(CONFIG_ERROR_RESILIENCE)        += error_resilience.o
>> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
>> new file mode 100644
>> index 0000000000..13bc25728f
>> --- /dev/null
>> +++ b/libavcodec/cbs.c
>> @@ -0,0 +1,466 @@
>> +/*
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg 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.
>> + *
>> + * FFmpeg 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 FFmpeg; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> + */
>> +
>> +#include <string.h>
>> +
>> +#include "config.h"
>> +
>> +#include "libavutil/avassert.h"
>> +#include "libavutil/common.h"
>> +
>> +#include "cbs.h"
>> +#include "cbs_internal.h"
>> +
>> +
>> +static const CodedBitstreamType *cbs_type_table[] = {
>> +};
>> +
>> +int ff_cbs_init(CodedBitstreamContext *ctx,
>> +                enum AVCodecID codec_id, void *log_ctx)
>> +{
>> +    const CodedBitstreamType *type;
>> +    int i;
>> +
>> +    type = NULL;
>> +    for (i = 0; i < FF_ARRAY_ELEMS(cbs_type_table); i++) {
>> +        if (cbs_type_table[i]->codec_id == codec_id) {
>> +            type = cbs_type_table[i];
>> +            break;
>> +        }
>> +    }
>> +    if (!type)
>> +        return AVERROR(EINVAL);
>> +
>> +    ctx->log_ctx = log_ctx;
>> +    ctx->codec   = type;
>> +
>> +    ctx->priv_data = av_mallocz(ctx->codec->priv_data_size);
>> +    if (!ctx->priv_data)
>> +        return AVERROR(ENOMEM);
>> +
>> +    ctx->decompose_unit_types = NULL;
>> +
>> +    ctx->trace_enable = 0;
>> +    ctx->trace_level  = AV_LOG_TRACE;
>> +
>> +    return 0;
>> +}
>> +
>> +void ff_cbs_close(CodedBitstreamContext *ctx)
>> +{
>> +    if (ctx->codec && ctx->codec->close)
>> +        ctx->codec->close(ctx);
>> +
>> +    av_freep(&ctx->priv_data);
>> +}
>> +
>> +static void cbs_unit_uninit(CodedBitstreamContext *ctx,
>> +                            CodedBitstreamUnit *unit)
>> +{
>> +    if (ctx->codec->free_unit && unit->content && !unit->content_external)
>> +        ctx->codec->free_unit(unit);
>> +
>> +    av_freep(&unit->data);
>> +    unit->data_size = 0;
>> +    unit->data_bit_padding = 0;
>> +}
>> +
>> +void ff_cbs_fragment_uninit(CodedBitstreamContext *ctx,
>> +                            CodedBitstreamFragment *frag)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < frag->nb_units; i++)
>> +        cbs_unit_uninit(ctx, &frag->units[i]);
>> +    av_freep(&frag->units);
>> +    frag->nb_units = 0;
>> +
>> +    av_freep(&frag->data);
>> +    frag->data_size        = 0;
>> +    frag->data_bit_padding = 0;
>> +}
>> +
>> +static int cbs_read_fragment_content(CodedBitstreamContext *ctx,
>> +                                     CodedBitstreamFragment *frag)
>> +{
>> +    int err, i, j;
>> +
>> +    for (i = 0; i < frag->nb_units; i++) {
>> +        if (ctx->decompose_unit_types) {
>> +            for (j = 0; j < ctx->nb_decompose_unit_types; j++) {
>> +                if (ctx->decompose_unit_types[j] == frag->units[i].type)
>> +                    break;
>> +            }
>> +            if (j >= ctx->nb_decompose_unit_types)
>> +                continue;
>> +        }
>> +
>> +        err = ctx->codec->read_unit(ctx, &frag->units[i]);
>> +        if (err == AVERROR(ENOSYS)) {
>> +            av_log(ctx->log_ctx, AV_LOG_WARNING,
>> +                   "Decomposition unimplemented for unit %d "
>> +                   "(type %d).\n", i, frag->units[i].type);
>> +        } else if (err < 0) {
>> +            av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to read unit %d "
>> +                   "(type %d).\n", i, frag->units[i].type);
>> +            return err;
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
>> +                          CodedBitstreamFragment *frag,
>> +                          const AVCodecParameters *par)
>> +{
>> +    int err;
>> +
>> +    memset(frag, 0, sizeof(*frag));
>> +
>> +    frag->data      = par->extradata;
>> +    frag->data_size = par->extradata_size;
>> +
>> +    err = ctx->codec->split_fragment(ctx, frag, 1);
>> +    if (err < 0)
>> +        return err;
>> +
>> +    frag->data      = NULL;
>> +    frag->data_size = 0;
>> +
>> +    return cbs_read_fragment_content(ctx, frag);
>> +}
>> +
>> +int ff_cbs_read_packet(CodedBitstreamContext *ctx,
>> +                       CodedBitstreamFragment *frag,
>> +                       const AVPacket *pkt)
>> +{
>> +    int err;
>> +
>> +    memset(frag, 0, sizeof(*frag));
>> +
>> +    frag->data      = pkt->data;
>> +    frag->data_size = pkt->size;
>> +
>> +    err = ctx->codec->split_fragment(ctx, frag, 0);
>> +    if (err < 0)
>> +        return err;
>> +
>> +    frag->data      = NULL;
>> +    frag->data_size = 0;
>> +
>> +    return cbs_read_fragment_content(ctx, frag);
>> +}
>> +
>> +int ff_cbs_read(CodedBitstreamContext *ctx,
>> +                CodedBitstreamFragment *frag,
>> +                const uint8_t *data, size_t size)
>> +{
>> +    int err;
>> +
>> +    memset(frag, 0, sizeof(*frag));
>> +
>> +    // (We won't write to this during split.)
>> +    frag->data      = (uint8_t*)data;
>> +    frag->data_size = size;
>> +
>> +    err = ctx->codec->split_fragment(ctx, frag, 0);
>> +    if (err < 0)
>> +        return err;
>> +
>> +    frag->data      = NULL;
>> +    frag->data_size = 0;
>> +
>> +    return cbs_read_fragment_content(ctx, frag);
>> +}
>> +
>> +
>> +int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx,
>> +                               CodedBitstreamFragment *frag)
>> +{
>> +    int err, i;
>> +
>> +    for (i = 0; i < frag->nb_units; i++) {
>> +        if (!frag->units[i].content)
>> +            continue;
>> +
>> +        err = ctx->codec->write_unit(ctx, &frag->units[i]);
>> +        if (err < 0) {
>> +            av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to write unit %d "
>> +                   "(type %d).\n", i, frag->units[i].type);
>> +            return err;
>> +        }
>> +    }
>> +
>> +    err = ctx->codec->assemble_fragment(ctx, frag);
>> +    if (err < 0) {
>> +        av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to assemble fragment.\n");
>> +        return err;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +int ff_cbs_write_extradata(CodedBitstreamContext *ctx,
>> +                           AVCodecParameters *par,
>> +                           CodedBitstreamFragment *frag)
>> +{
>> +    int err;
>> +
>> +    err = ff_cbs_write_fragment_data(ctx, frag);
>> +    if (err < 0)
>> +        return err;
>> +
>> +    av_freep(&par->extradata);
>> +
>> +    par->extradata = av_malloc(frag->data_size +
>> +                               AV_INPUT_BUFFER_PADDING_SIZE);
>> +    if (!par->extradata)
>> +        return AVERROR(ENOMEM);
>> +
>> +    memcpy(par->extradata, frag->data, frag->data_size);
>> +    memset(par->extradata + frag->data_size, 0,
>> +           AV_INPUT_BUFFER_PADDING_SIZE);
>> +    par->extradata_size = frag->data_size;
>> +
>> +    return 0;
>> +}
>> +
>> +int ff_cbs_write_packet(CodedBitstreamContext *ctx,
>> +                        AVPacket *pkt,
>> +                        CodedBitstreamFragment *frag)
>> +{
>> +    int err;
>> +
>> +    err = ff_cbs_write_fragment_data(ctx, frag);
>> +    if (err < 0)
>> +        return err;
>> +
> 
>> +    av_new_packet(pkt, frag->data_size);
>> +    if (err < 0)
>> +        return err;
> 
> that does not work

I think I'm missing something.  Can you be more precise about what doesn't work?

>> +
>> +    memcpy(pkt->data, frag->data, frag->data_size);
>> +    pkt->size = frag->data_size;
>> +
>> +    return 0;
>> +}
>> +
>> +
>> +void ff_cbs_trace_header(CodedBitstreamContext *ctx,
>> +                         const char *name)
>> +{
>> +    if (!ctx->trace_enable)
>> +        return;
>> +
>> +    av_log(ctx->log_ctx, ctx->trace_level, "%s\n", name);
>> +}
>> +
>> +void ff_cbs_trace_syntax_element(CodedBitstreamContext *ctx, int position,
>> +                                 const char *name, const char *bits,
>> +                                 int64_t value)
>> +{
>> +    size_t name_len, bits_len;
>> +    int pad;
>> +
>> +    if (!ctx->trace_enable)
>> +        return;
>> +
>> +    av_assert0(value >= INT_MIN && value <= UINT32_MAX);
>> +
>> +    name_len = strlen(name);
>> +    bits_len = strlen(bits);
>> +
>> +    if (name_len + bits_len > 60)
>> +        pad = bits_len + 2;
>> +    else
>> +        pad = 61 - name_len;
>> +
>> +    av_log(ctx->log_ctx, ctx->trace_level, "%-10d  %s%*s = %"PRId64"\n",
>> +           position, name, pad, bits, value);
>> +}
>> +
> 
>> +int ff_cbs_read_unsigned(CodedBitstreamContext *ctx, GetBitContext *gbc,
>> +                         int width, const char *name, uint32_t *write_to,
>> +                         uint32_t range_min, uint32_t range_max)
>> +{
>> +    uint32_t value;
>> +    int position;
>> +
>> +    av_assert0(width <= 32);
> 
> if you assert validity, you should also assert non negativity

Ok, changed.  (Also in _write below.)

>> +
>> +    if (get_bits_left(gbc) < width) {
>> +        av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid value at "
>> +               "%s: bitstream ended.\n", name);
>> +        return AVERROR_INVALIDDATA;
>> +    }
>> +
>> +    if (ctx->trace_enable)
>> +        position = get_bits_count(gbc);
>> +
>> +    value = get_bits_long(gbc, width);
>> +
>> +    if (ctx->trace_enable) {
>> +        char bits[33];
>> +        int i;
>> +        for (i = 0; i < width; i++)
>> +            bits[i] = value >> (width - i - 1) & 1 ? '1' : '0';
>> +        bits[i] = 0;
>> +
>> +        ff_cbs_trace_syntax_element(ctx, position, name, bits, value);
>> +    }
>> +
>> +    if (value < range_min || value > range_max) {
>> +        av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
>> +               "%"PRIu32", but must be in [%"PRIu32",%"PRIu32"].\n",
>> +               name, value, range_min, range_max);
>> +        return AVERROR_INVALIDDATA;
>> +    }
>> +
>> +    *write_to = value;
>> +    return 0;
>> +}
>> +
>> +int ff_cbs_write_unsigned(CodedBitstreamContext *ctx, PutBitContext *pbc,
>> +                          int width, const char *name, uint32_t value,
>> +                          uint32_t range_min, uint32_t range_max)
>> +{
>> +    av_assert0(width <= 32);
>> +
>> +    if (value < range_min || value > range_max) {
>> +        av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
>> +               "%"PRIu32", but must be in [%"PRIu32",%"PRIu32"].\n",
>> +               name, value, range_min, range_max);
>> +        return AVERROR_INVALIDDATA;
>> +    }
>> +
>> +    if (put_bits_left(pbc) < width)
>> +        return AVERROR(ENOSPC);
>> +
>> +    if (ctx->trace_enable) {
>> +        char bits[33];
>> +        int i;
>> +        for (i = 0; i < width; i++)
>> +            bits[i] = value >> (width - i - 1) & 1 ? '1' : '0';
>> +        bits[i] = 0;
>> +
>> +        ff_cbs_trace_syntax_element(ctx, put_bits_count(pbc), name, bits, value);
>> +    }
>> +
>> +    if (width < 32)
>> +        put_bits(pbc, width, value);
>> +    else
>> +        put_bits32(pbc, value);
>> +
>> +    return 0;
>> +}
>> +
>> +
> 
>> +static int cbs_insert_unit(CodedBitstreamContext *ctx,
>> +                           CodedBitstreamFragment *frag,
>> +                           int position)
>> +{
>> +    CodedBitstreamUnit *units;
>> +
>> +    units = av_malloc_array(frag->nb_units + 1, sizeof(*units));
>> +    if (!units)
>> +        return AVERROR(ENOMEM);
>> +
>> +    if (position > 0)
>> +        memcpy(units, frag->units, position * sizeof(*units));
>> +    if (position < frag->nb_units)
>> +        memcpy(units + position + 1, frag->units + position,
>> +               (frag->nb_units - position) * sizeof(*units));
>> +
>> +    memset(units + position, 0, sizeof(*units));
>> +
>> +    av_freep(&frag->units);
>> +    frag->units = units;
>> +    ++frag->nb_units;
>> +
>> +    return 0;
>> +}
> 
> re-creating the whole array on each insertion seems wastefull of
> CPU time

I'm not sure there is any reason to make something less clean to do better - the expected number of units per fragment is not high.

>> +
>> +int ff_cbs_insert_unit_content(CodedBitstreamContext *ctx,
>> +                               CodedBitstreamFragment *frag,
>> +                               int position, uint32_t type,
>> +                               void *content)
>> +{
>> +    int err;
>> +
>> +    if (position == -1)
>> +        position = frag->nb_units;
>> +    av_assert0(position >= 0 && position <= frag->nb_units);
>> +
>> +    err = cbs_insert_unit(ctx, frag, position);
>> +    if (err < 0)
>> +        return err;
>> +
>> +    frag->units[position].type             = type;
>> +    frag->units[position].content          = content;
>> +    frag->units[position].content_external = 1;
>> +
>> +    return 0;
>> +}
>> +
>> +int ff_cbs_insert_unit_data(CodedBitstreamContext *ctx,
>> +                            CodedBitstreamFragment *frag,
>> +                            int position, uint32_t type,
>> +                            uint8_t *data, size_t data_size)
>> +{
>> +    int err;
>> +
>> +    if (position == -1)
>> +        position = frag->nb_units;
>> +    av_assert0(position >= 0 && position <= frag->nb_units);
>> +
>> +    err = cbs_insert_unit(ctx, frag, position);
>> +    if (err < 0)
>> +        return err;
>> +
>> +    frag->units[position].type      = type;
>> +    frag->units[position].data      = data;
>> +    frag->units[position].data_size = data_size;
>> +
>> +    return 0;
>> +}
>> +
>> +int ff_cbs_delete_unit(CodedBitstreamContext *ctx,
>> +                       CodedBitstreamFragment *frag,
>> +                       int position)
>> +{
>> +    if (position < 0 || position >= frag->nb_units)
>> +        return AVERROR(EINVAL);
>> +
>> +    cbs_unit_uninit(ctx, &frag->units[position]);
>> +
>> +    --frag->nb_units;
>> +
>> +    if (frag->nb_units == 0) {
>> +        av_freep(&frag->units);
>> +
>> +    } else {
>> +        memmove(frag->units + position,
>> +                frag->units + position + 1,
>> +                (frag->nb_units - position) * sizeof(*frag->units));
>> +
>> +        // Don't bother reallocating the unit array.
>> +    }
>> +
>> +    return 0;
>> +}
> 
>> diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
>> new file mode 100644
>> index 0000000000..eeaff379e5
>> --- /dev/null
>> +++ b/libavcodec/cbs.h
>> @@ -0,0 +1,274 @@
>> +/*
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg 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.
>> + *
>> + * FFmpeg 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 FFmpeg; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> + */
>> +
>> +#ifndef AVCODEC_CBS_H
>> +#define AVCODEC_CBS_H
>> +
>> +#include <stddef.h>
>> +#include <stdint.h>
>> +
>> +#include "avcodec.h"
>> +
>> +
>> +struct CodedBitstreamType;
>> +
>> +/**
>> + * Coded bitstream unit structure.
>> + *
>> + * A bitstream unit the the smallest element of a bitstream which
> 
> the the ?

Fixed.

>> + * is meaningful on its own.  For example, an H.264 NAL unit.
>> + *
>> + * See the codec-specific header for the meaning of this for any
>> + * particular codec.
>> + */
>> +typedef struct CodedBitstreamUnit {
>> +    /**
>> +     * Codec-specific type of this unit.
>> +     */
>> +    uint32_t type;
>> +
>> +    /**
>> +     * Pointer to the bitstream form of this unit.
>> +     *
>> +     * May be NULL if the unit currently only exists in decomposed form.
>> +     */
>> +    uint8_t *data;
>> +    /**
>> +     * The number of bytes in the bitstream (including any padding bits
>> +     * in the final byte).
>> +     */
>> +    size_t   data_size;
> 
>> +    /**
>> +     * The number of bits which should be ignored in the final byte.
>> +     *
>> +     * This supports non-byte-aligned bitstreams.
>> +     */
>> +    size_t   data_bit_padding;
> 
> theres no need for this to be size_t, a byte can only contain up to
> 8 bits

It gets used with data_size, so the type matching is nice.  (Also making it smaller would save no space due to padding unless the structure were rearranged, which would be less clear.)

>> +
>> +    /**
>> +     * Pointer to the decomposed form of this unit.
>> +     *
>> +     * The type of this structure depends on both the codec and the
>> +     * type of this unit.  May be NULL if the unit only exists in
>> +     * bitstream form.
>> +     */
>> +    void *content;
>> +    /**
>> +     * Whether the content was supplied externally.
>> +     *
>> +     * If so, it should not be freed when freeing the unit.
>> +     */
>> +    int   content_external;
>> +} CodedBitstreamUnit;
>> +
>> +/**
>> + * Coded bitstream fragment structure, combining one or more units.
>> + *
>> + * This is any sequence of units.  It need not form some greater whole,
>> + * though in many cases it will.  For example, an H.264 access unit,
>> + * which is composed of a sequence of H.264 NAL units.
>> + */
>> +typedef struct CodedBitstreamFragment {
>> +    /**
>> +     * Pointer to the bitstream form of this fragment.
>> +     *
>> +     * May be NULL if the fragment only exists as component units.
>> +     */
>> +    uint8_t *data;
>> +    /**
>> +     * The number of bytes in the bitstream.
>> +     *
>> +     * The number of bytes in the bitstream (including any padding bits
>> +     * in the final byte).
>> +     */
>> +    size_t   data_size;
>> +    /**
>> +     * The number of bits which should be ignored in the final byte.
>> +     */
>> +    size_t data_bit_padding;
>> +
>> +    /**
>> +     * Number of units in this fragment.
>> +     *
>> +     * This may be zero if the fragment only exists in bistream form
>> +     * and has not been decomposed.
> 
> what is a biStream form ?

Fixed.

>> +     */
>> +    int              nb_units;
>> +    /**
>> +     * Pointer to an array of units of length nb_units.
>> +     *
>> +     * Must be NULL if nb_units is zero.
>> +     */
>> +    CodedBitstreamUnit *units;
>> +} CodedBitstreamFragment;
>> +
>> +/**
>> + * Context structure for coded bitstream operations.
>> + */
>> +typedef struct CodedBitstreamContext {
>> +    /**
>> +     * Logging context to be passed to all av_log() calls associated
>> +     * with this context.
>> +     */
>> +    void *log_ctx;
>> +
>> +    /**
>> +     * Internal codec-specific hooks.
>> +     */
>> +    const struct CodedBitstreamType *codec;
>> +
>> +    /**
>> +     * Internal codec-specific data.
>> +     *
>> +     * This contains any information needed when reading/writing
>> +     * bitsteams which will not necessarily be present in a fragment.
>> +     * For example, for H.264 it contains all currently visible
>> +     * parameter sets - they are required to determine the bitstream
>> +     * syntax but need not be present in every access unit.
>> +     */
>> +    void *priv_data;
>> +
> 
>> +    /**
>> +     * Array of unit types which should be decomposed when reading.
>> +     *
>> +     * Types not in this list will be available in bitstream form only.
>> +     * If NULL, all supported types will be decomposed.
>> +     */
>> +    uint32_t *decompose_unit_types;
> 
> this should not be a generic integer.
> a typedef or enum would be better

It's H.264 nal unit types union H.265 nal unit types union MPEG-2 start codes (union any other codec that gets added).

What type should that be?  I chose uint32_t to make it fixed-size and hopefully large enough to be able to make sense for any future codec.

>> +    /**
>> +     * Length of the decompose_unit_types array.
>> +     */
>> +    int    nb_decompose_unit_types;
>> +
>> +    /**
>> +     * Enable trace output during read/write operations.
>> +     */
>> +    int trace_enable;
>> +    /**
>> +     * Log level to use for trace output.
>> +     *
>> +     * From AV_LOG_*; defaults to AV_LOG_TRACE.
>> +     */
>> +    int trace_level;
>> +} CodedBitstreamContext;
>> +
>> +
> 
>> +/**
>> + * Initialise a new context for the given codec.
>> + */
>> +int ff_cbs_init(CodedBitstreamContext *ctx,
>> +                enum AVCodecID codec_id, void *log_ctx);
> 
> allocating the context within the init function would make the
> API more robust with extensions to the context and potential use from
> other libs.
> It is also how most of our APIs work

This API is currently completely libavcodec internal, and I would like to keep it that way for a while at least.  Therefore, there are no extensions and no use from other libs.

Still, I can change it.

>> +
>> +/**
>> + * Close a context and free all internal state.
>> + */
>> +void ff_cbs_close(CodedBitstreamContext *ctx);
>> +
>> +
> 
>> +/**
>> + * Read the extradata bitstream found in codec parameters into a
>> + * fragment, then split into units and decompose.
>> + *
>> + * This also updates the internal state, so will need to be called for
>> + * codecs with extradata to read parameter sets necessary for further
>> + * parsing even if the fragment itself is not desired.
>> + */
>> +int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
>> +                          CodedBitstreamFragment *frag,
>> +                          const AVCodecParameters *par);
> 
> are any fields other than extradata and size needed from
> AVCodecParameters ?
> if not then it may be more flexible to just pass the extradata and size

Not currently, but possibly in future.  It's distinguished from ff_cbs_read() by this form (and needs to be to at least some degree, because of AVCC/HVCC headers).

>> +
>> +/**
>> + * Read the data bitstream from a packet into a fragment, then
>> + * split into units and decompose.
>> + */
>> +int ff_cbs_read_packet(CodedBitstreamContext *ctx,
>> +                       CodedBitstreamFragment *frag,
>> +                       const AVPacket *pkt);
>> +
>> +/**
>> + * Read a bitstream from a memory region into a fragment, then
>> + * split into units and decompose.
>> + */
>> +int ff_cbs_read(CodedBitstreamContext *ctx,
>> +                CodedBitstreamFragment *frag,
>> +                const uint8_t *data, size_t size);
>> +
>> +
>> +/**
>> + * Write the content of the fragment to its own internal buffer.
>> + *
>> + * Writes the content of all units and then assembles them into a new
>> + * data buffer.  When modifying the content of decomposed units, this
>> + * can be used to regenerate the bitstream form of units or the whole
>> + * fragment so that it can be extracted for other use.
>> + */
>> +int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx,
>> +                               CodedBitstreamFragment *frag);
>> +
> 
>> +/**
>> + * Write the bitstream of a fragment to the extradata in codec parameters.
>> + */
>> +int ff_cbs_write_extradata(CodedBitstreamContext *ctx,
>> +                           AVCodecParameters *par,
>> +                           CodedBitstreamFragment *frag);
> 
> This is missing documentation about allocation. Will the call allocate
> the extradata, should the caller allocate, is it allowed to be allocated
> before the call ?
> 
> Also similar to above, passing a pointer and pointer to size may be
> more flexible

Add " * This replaces any existing extradata in the structure.".

For both of these, the separate extradata API makes callers in bitstream filters a bit simpler, which is I think worth it.

(There is no cbs_write(pointer, size) function because it currently makes more sense as cbs_write_fragment_data() and then manipulate frag->data and frag->data_size directly.  Could consider something further if there are any callers wanting it.)

>> +
>> +/**
>> + * Write the bitstream of a fragment to a packet.
>> + */
>> +int ff_cbs_write_packet(CodedBitstreamContext *ctx,
>> +                        AVPacket *pkt,
>> +                        CodedBitstreamFragment *frag);
>> +
>> +
>> +/**
>> + * Free all allocated memory in a fragment.
>> + */
>> +void ff_cbs_fragment_uninit(CodedBitstreamContext *ctx,
>> +                            CodedBitstreamFragment *frag);
>> +
>> +
>> +/**
>> + * Insert a new unit into a fragment with the given content.
>> + *
>> + * The content structure continues to be owned by the caller, and
>> + * will not be freed when the unit is.
>> + */
>> +int ff_cbs_insert_unit_content(CodedBitstreamContext *ctx,
>> +                               CodedBitstreamFragment *frag,
>> +                               int position, uint32_t type,
>> +                               void *content);
>> +
>> +/**
>> + * Insert a new unit into a fragment with the given data bitstream.
>> + *
>> + * The data buffer will be owned by the unit after this operation.
>> + */
>> +int ff_cbs_insert_unit_data(CodedBitstreamContext *ctx,
>> +                            CodedBitstreamFragment *frag,
>> +                            int position, uint32_t type,
>> +                            uint8_t *data, size_t data_size);
>> +
>> +/**
>> + * Delete a unit from a fragment and free all memory it uses.
>> + */
>> +int ff_cbs_delete_unit(CodedBitstreamContext *ctx,
>> +                       CodedBitstreamFragment *frag,
>> +                       int position);
>> +
>> +
>> +#endif /* AVCODEC_CBS_H */
>> diff --git a/libavcodec/cbs_internal.h b/libavcodec/cbs_internal.h
>> new file mode 100644
>> index 0000000000..3bbc3355c2
>> --- /dev/null
>> +++ b/libavcodec/cbs_internal.h
>> @@ -0,0 +1,83 @@
>> +/*
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg 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.
>> + *
>> + * FFmpeg 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 FFmpeg; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> + */
>> +
>> +#ifndef AVCODEC_CBS_INTERNAL_H
>> +#define AVCODEC_CBS_INTERNAL_H
>> +
>> +#include "avcodec.h"
>> +#include "cbs.h"
>> +#include "get_bits.h"
>> +#include "put_bits.h"
>> +
>> +
>> +typedef struct CodedBitstreamType {
>> +    enum AVCodecID codec_id;
>> +
>> +    size_t priv_data_size;
>> +
> 
>> +    // Split frag->data into coded bitstream units, creating the
>> +    // frag->units array.  Fill data but not content on each unit.
>> +    int (*split_fragment)(CodedBitstreamContext *ctx,
>> +                          CodedBitstreamFragment *frag,
>> +                          int header);
> 
> The "header" parameter lacks documentation

Add "// header is set if the fragment came from a header block, which may require different parsing for some codecs.".

>> +
>> +    // Read the unit->data bitstream and decompose it, creating
>> +    // unit->content.
>> +    int (*read_unit)(CodedBitstreamContext *ctx,
>> +                     CodedBitstreamUnit *unit);
>> +
>> +    // Write the unit->data bitstream from unit->content.
>> +    int (*write_unit)(CodedBitstreamContext *ctx,
>> +                      CodedBitstreamUnit *unit);
>> +
>> +    // Read the data from all of frag->units and assemble it into
>> +    // a bitstream for the whole fragment.
>> +    int (*assemble_fragment)(CodedBitstreamContext *ctx,
>> +                             CodedBitstreamFragment *frag);
>> +
>> +    // Free the content and data of a single unit.
>> +    void (*free_unit)(CodedBitstreamUnit *unit);
>> +
>> +    // Free the codec internal state.
>> +    void (*close)(CodedBitstreamContext *ctx);
>> +} CodedBitstreamType;
>> +
>> +
>> +// Helper functions for trace output.
>> +
>> +void ff_cbs_trace_header(CodedBitstreamContext *ctx,
>> +                         const char *name);
>> +
>> +void ff_cbs_trace_syntax_element(CodedBitstreamContext *ctx,
>> +                                 int position, const char *name,
>> +                                 const char *bitstring, int64_t value);
>> +
>> +
>> +// Helper functions for read/write of common bitstream elements, including
>> +// generation of trace output.
>> +
>> +int ff_cbs_read_unsigned(CodedBitstreamContext *ctx, GetBitContext *gbc,
>> +                         int width, const char *name, uint32_t *write_to,
>> +                         uint32_t range_min, uint32_t range_max);
>> +
>> +int ff_cbs_write_unsigned(CodedBitstreamContext *ctx, PutBitContext *pbc,
>> +                          int width, const char *name, uint32_t value,
>> +                          uint32_t range_min, uint32_t range_max);
>> +
>> +
>> +#endif /* AVCODEC_CBS_INTERNAL_H */
>> -- 
>> 2.11.0
>>

Thanks,

- Mark
Michael Niedermayer Sept. 14, 2017, 8:28 p.m.
On Thu, Sep 14, 2017 at 08:31:28AM +0100, Mark Thompson wrote:
> On 14/09/17 01:42, Michael Niedermayer wrote:
> > On Wed, Sep 13, 2017 at 12:43:53AM +0100, Mark Thompson wrote:
[...]

> >> +int ff_cbs_write_packet(CodedBitstreamContext *ctx,
> >> +                        AVPacket *pkt,
> >> +                        CodedBitstreamFragment *frag)
> >> +{
> >> +    int err;
> >> +
> >> +    err = ff_cbs_write_fragment_data(ctx, frag);
> >> +    if (err < 0)
> >> +        return err;
> >> +
> > 
> >> +    av_new_packet(pkt, frag->data_size);
> >> +    if (err < 0)
> >> +        return err;
> > 
> > that does not work
> 
> I think I'm missing something.  Can you be more precise about what doesn't work?

you ignore the return code of av_new_packet()
the check does nothing


[...]


> >> +     */
> >> +    int              nb_units;
> >> +    /**
> >> +     * Pointer to an array of units of length nb_units.
> >> +     *
> >> +     * Must be NULL if nb_units is zero.
> >> +     */
> >> +    CodedBitstreamUnit *units;
> >> +} CodedBitstreamFragment;
> >> +
> >> +/**
> >> + * Context structure for coded bitstream operations.
> >> + */
> >> +typedef struct CodedBitstreamContext {
> >> +    /**
> >> +     * Logging context to be passed to all av_log() calls associated
> >> +     * with this context.
> >> +     */
> >> +    void *log_ctx;
> >> +
> >> +    /**
> >> +     * Internal codec-specific hooks.
> >> +     */
> >> +    const struct CodedBitstreamType *codec;
> >> +
> >> +    /**
> >> +     * Internal codec-specific data.
> >> +     *
> >> +     * This contains any information needed when reading/writing
> >> +     * bitsteams which will not necessarily be present in a fragment.
> >> +     * For example, for H.264 it contains all currently visible
> >> +     * parameter sets - they are required to determine the bitstream
> >> +     * syntax but need not be present in every access unit.
> >> +     */
> >> +    void *priv_data;
> >> +
> > 
> >> +    /**
> >> +     * Array of unit types which should be decomposed when reading.
> >> +     *
> >> +     * Types not in this list will be available in bitstream form only.
> >> +     * If NULL, all supported types will be decomposed.
> >> +     */
> >> +    uint32_t *decompose_unit_types;
> > 
> > this should not be a generic integer.
> > a typedef or enum would be better
> 
> It's H.264 nal unit types union H.265 nal unit types union MPEG-2 start codes (union any other codec that gets added).
> 
> What type should that be?  I chose uint32_t to make it fixed-size and hopefully large enough to be able to make sense for any future codec.

a typedef based type would be better than a litterally hardcoded
one. A hardcoded type would be duplicatedly hardcoded in many
places ...

Also please document that the type is codec specific (it is IIUC)

thx

[...]
Mark Thompson Sept. 14, 2017, 8:51 p.m.
On 14/09/17 21:28, Michael Niedermayer wrote:
> On Thu, Sep 14, 2017 at 08:31:28AM +0100, Mark Thompson wrote:
>> On 14/09/17 01:42, Michael Niedermayer wrote:
>>> On Wed, Sep 13, 2017 at 12:43:53AM +0100, Mark Thompson wrote:
> [...]
> 
>>>> +int ff_cbs_write_packet(CodedBitstreamContext *ctx,
>>>> +                        AVPacket *pkt,
>>>> +                        CodedBitstreamFragment *frag)
>>>> +{
>>>> +    int err;
>>>> +
>>>> +    err = ff_cbs_write_fragment_data(ctx, frag);
>>>> +    if (err < 0)
>>>> +        return err;
>>>> +
>>>
>>>> +    av_new_packet(pkt, frag->data_size);
>>>> +    if (err < 0)
>>>> +        return err;
>>>
>>> that does not work
>>
>> I think I'm missing something.  Can you be more precise about what doesn't work?
> 
> you ignore the return code of av_new_packet()
> the check does nothing

Duh, oops.  I spent a while looking for some subtlety in av_new_packet() and missed the obvious.  Sorry!

>>>> +     */
>>>> +    int              nb_units;
>>>> +    /**
>>>> +     * Pointer to an array of units of length nb_units.
>>>> +     *
>>>> +     * Must be NULL if nb_units is zero.
>>>> +     */
>>>> +    CodedBitstreamUnit *units;
>>>> +} CodedBitstreamFragment;
>>>> +
>>>> +/**
>>>> + * Context structure for coded bitstream operations.
>>>> + */
>>>> +typedef struct CodedBitstreamContext {
>>>> +    /**
>>>> +     * Logging context to be passed to all av_log() calls associated
>>>> +     * with this context.
>>>> +     */
>>>> +    void *log_ctx;
>>>> +
>>>> +    /**
>>>> +     * Internal codec-specific hooks.
>>>> +     */
>>>> +    const struct CodedBitstreamType *codec;
>>>> +
>>>> +    /**
>>>> +     * Internal codec-specific data.
>>>> +     *
>>>> +     * This contains any information needed when reading/writing
>>>> +     * bitsteams which will not necessarily be present in a fragment.
>>>> +     * For example, for H.264 it contains all currently visible
>>>> +     * parameter sets - they are required to determine the bitstream
>>>> +     * syntax but need not be present in every access unit.
>>>> +     */
>>>> +    void *priv_data;
>>>> +
>>>
>>>> +    /**
>>>> +     * Array of unit types which should be decomposed when reading.
>>>> +     *
>>>> +     * Types not in this list will be available in bitstream form only.
>>>> +     * If NULL, all supported types will be decomposed.
>>>> +     */
>>>> +    uint32_t *decompose_unit_types;
>>>
>>> this should not be a generic integer.
>>> a typedef or enum would be better
>>
>> It's H.264 nal unit types union H.265 nal unit types union MPEG-2 start codes (union any other codec that gets added).
>>
>> What type should that be?  I chose uint32_t to make it fixed-size and hopefully large enough to be able to make sense for any future codec.
> 
> a typedef based type would be better than a litterally hardcoded
> one. A hardcoded type would be duplicatedly hardcoded in many
> places ...

So have "typedef uint32_t CBSUnitType;"?  I'm not really sure this adds anything since every implementation is using its own enum anyway, but ok.

> Also please document that the type is codec specific (it is IIUC)

Yes; above there is:

"""
typedef struct CodedBitstreamUnit {
    /**
     * Codec-specific type of this unit.
     */
    uint32_t type;
"""

Thanks,

- Mark
Michael Niedermayer Sept. 14, 2017, 9:51 p.m.
On Thu, Sep 14, 2017 at 09:51:31PM +0100, Mark Thompson wrote:
> On 14/09/17 21:28, Michael Niedermayer wrote:
> > On Thu, Sep 14, 2017 at 08:31:28AM +0100, Mark Thompson wrote:
> >> On 14/09/17 01:42, Michael Niedermayer wrote:
> >>> On Wed, Sep 13, 2017 at 12:43:53AM +0100, Mark Thompson wrote:
> > [...]
> > 
> >>>> +int ff_cbs_write_packet(CodedBitstreamContext *ctx,
> >>>> +                        AVPacket *pkt,
> >>>> +                        CodedBitstreamFragment *frag)
> >>>> +{
> >>>> +    int err;
> >>>> +
> >>>> +    err = ff_cbs_write_fragment_data(ctx, frag);
> >>>> +    if (err < 0)
> >>>> +        return err;
> >>>> +
> >>>
> >>>> +    av_new_packet(pkt, frag->data_size);
> >>>> +    if (err < 0)
> >>>> +        return err;
> >>>
> >>> that does not work
> >>
> >> I think I'm missing something.  Can you be more precise about what doesn't work?
> > 
> > you ignore the return code of av_new_packet()
> > the check does nothing
> 
> Duh, oops.  I spent a while looking for some subtlety in av_new_packet() and missed the obvious.  Sorry!
> 
> >>>> +     */
> >>>> +    int              nb_units;
> >>>> +    /**
> >>>> +     * Pointer to an array of units of length nb_units.
> >>>> +     *
> >>>> +     * Must be NULL if nb_units is zero.
> >>>> +     */
> >>>> +    CodedBitstreamUnit *units;
> >>>> +} CodedBitstreamFragment;
> >>>> +
> >>>> +/**
> >>>> + * Context structure for coded bitstream operations.
> >>>> + */
> >>>> +typedef struct CodedBitstreamContext {
> >>>> +    /**
> >>>> +     * Logging context to be passed to all av_log() calls associated
> >>>> +     * with this context.
> >>>> +     */
> >>>> +    void *log_ctx;
> >>>> +
> >>>> +    /**
> >>>> +     * Internal codec-specific hooks.
> >>>> +     */
> >>>> +    const struct CodedBitstreamType *codec;
> >>>> +
> >>>> +    /**
> >>>> +     * Internal codec-specific data.
> >>>> +     *
> >>>> +     * This contains any information needed when reading/writing
> >>>> +     * bitsteams which will not necessarily be present in a fragment.
> >>>> +     * For example, for H.264 it contains all currently visible
> >>>> +     * parameter sets - they are required to determine the bitstream
> >>>> +     * syntax but need not be present in every access unit.
> >>>> +     */
> >>>> +    void *priv_data;
> >>>> +
> >>>
> >>>> +    /**
> >>>> +     * Array of unit types which should be decomposed when reading.
> >>>> +     *
> >>>> +     * Types not in this list will be available in bitstream form only.
> >>>> +     * If NULL, all supported types will be decomposed.
> >>>> +     */
> >>>> +    uint32_t *decompose_unit_types;
> >>>
> >>> this should not be a generic integer.
> >>> a typedef or enum would be better
> >>
> >> It's H.264 nal unit types union H.265 nal unit types union MPEG-2 start codes (union any other codec that gets added).
> >>
> >> What type should that be?  I chose uint32_t to make it fixed-size and hopefully large enough to be able to make sense for any future codec.
> > 
> > a typedef based type would be better than a litterally hardcoded
> > one. A hardcoded type would be duplicatedly hardcoded in many
> > places ...
> 
> So have "typedef uint32_t CBSUnitType;"?  I'm not really sure this adds anything since every implementation is using its own enum anyway, but ok.

duplication is bad
consider you ever want to change the underlaying type ...

[...]

Patch hide | download patch | download mbox

diff --git a/configure b/configure
index 4dd11efe5e..2e84b1f892 100755
--- a/configure
+++ b/configure
@@ -2112,6 +2112,7 @@  CONFIG_EXTRA="
     blockdsp
     bswapdsp
     cabac
+    cbs
     dirac_parse
     dvprofile
     exif
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 999632cf9e..d9612d68d0 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -59,6 +59,7 @@  OBJS-$(CONFIG_AUDIODSP)                += audiodsp.o
 OBJS-$(CONFIG_BLOCKDSP)                += blockdsp.o
 OBJS-$(CONFIG_BSWAPDSP)                += bswapdsp.o
 OBJS-$(CONFIG_CABAC)                   += cabac.o
+OBJS-$(CONFIG_CBS)                     += cbs.o
 OBJS-$(CONFIG_CRYSTALHD)               += crystalhd.o
 OBJS-$(CONFIG_DCT)                     += dct.o dct32_fixed.o dct32_float.o
 OBJS-$(CONFIG_ERROR_RESILIENCE)        += error_resilience.o
diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
new file mode 100644
index 0000000000..13bc25728f
--- /dev/null
+++ b/libavcodec/cbs.c
@@ -0,0 +1,466 @@ 
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg 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.
+ *
+ * FFmpeg 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 FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <string.h>
+
+#include "config.h"
+
+#include "libavutil/avassert.h"
+#include "libavutil/common.h"
+
+#include "cbs.h"
+#include "cbs_internal.h"
+
+
+static const CodedBitstreamType *cbs_type_table[] = {
+};
+
+int ff_cbs_init(CodedBitstreamContext *ctx,
+                enum AVCodecID codec_id, void *log_ctx)
+{
+    const CodedBitstreamType *type;
+    int i;
+
+    type = NULL;
+    for (i = 0; i < FF_ARRAY_ELEMS(cbs_type_table); i++) {
+        if (cbs_type_table[i]->codec_id == codec_id) {
+            type = cbs_type_table[i];
+            break;
+        }
+    }
+    if (!type)
+        return AVERROR(EINVAL);
+
+    ctx->log_ctx = log_ctx;
+    ctx->codec   = type;
+
+    ctx->priv_data = av_mallocz(ctx->codec->priv_data_size);
+    if (!ctx->priv_data)
+        return AVERROR(ENOMEM);
+
+    ctx->decompose_unit_types = NULL;
+
+    ctx->trace_enable = 0;
+    ctx->trace_level  = AV_LOG_TRACE;
+
+    return 0;
+}
+
+void ff_cbs_close(CodedBitstreamContext *ctx)
+{
+    if (ctx->codec && ctx->codec->close)
+        ctx->codec->close(ctx);
+
+    av_freep(&ctx->priv_data);
+}
+
+static void cbs_unit_uninit(CodedBitstreamContext *ctx,
+                            CodedBitstreamUnit *unit)
+{
+    if (ctx->codec->free_unit && unit->content && !unit->content_external)
+        ctx->codec->free_unit(unit);
+
+    av_freep(&unit->data);
+    unit->data_size = 0;
+    unit->data_bit_padding = 0;
+}
+
+void ff_cbs_fragment_uninit(CodedBitstreamContext *ctx,
+                            CodedBitstreamFragment *frag)
+{
+    int i;
+
+    for (i = 0; i < frag->nb_units; i++)
+        cbs_unit_uninit(ctx, &frag->units[i]);
+    av_freep(&frag->units);
+    frag->nb_units = 0;
+
+    av_freep(&frag->data);
+    frag->data_size        = 0;
+    frag->data_bit_padding = 0;
+}
+
+static int cbs_read_fragment_content(CodedBitstreamContext *ctx,
+                                     CodedBitstreamFragment *frag)
+{
+    int err, i, j;
+
+    for (i = 0; i < frag->nb_units; i++) {
+        if (ctx->decompose_unit_types) {
+            for (j = 0; j < ctx->nb_decompose_unit_types; j++) {
+                if (ctx->decompose_unit_types[j] == frag->units[i].type)
+                    break;
+            }
+            if (j >= ctx->nb_decompose_unit_types)
+                continue;
+        }
+
+        err = ctx->codec->read_unit(ctx, &frag->units[i]);
+        if (err == AVERROR(ENOSYS)) {
+            av_log(ctx->log_ctx, AV_LOG_WARNING,
+                   "Decomposition unimplemented for unit %d "
+                   "(type %d).\n", i, frag->units[i].type);
+        } else if (err < 0) {
+            av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to read unit %d "
+                   "(type %d).\n", i, frag->units[i].type);
+            return err;
+        }
+    }
+
+    return 0;
+}
+
+int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
+                          CodedBitstreamFragment *frag,
+                          const AVCodecParameters *par)
+{
+    int err;
+
+    memset(frag, 0, sizeof(*frag));
+
+    frag->data      = par->extradata;
+    frag->data_size = par->extradata_size;
+
+    err = ctx->codec->split_fragment(ctx, frag, 1);
+    if (err < 0)
+        return err;
+
+    frag->data      = NULL;
+    frag->data_size = 0;
+
+    return cbs_read_fragment_content(ctx, frag);
+}
+
+int ff_cbs_read_packet(CodedBitstreamContext *ctx,
+                       CodedBitstreamFragment *frag,
+                       const AVPacket *pkt)
+{
+    int err;
+
+    memset(frag, 0, sizeof(*frag));
+
+    frag->data      = pkt->data;
+    frag->data_size = pkt->size;
+
+    err = ctx->codec->split_fragment(ctx, frag, 0);
+    if (err < 0)
+        return err;
+
+    frag->data      = NULL;
+    frag->data_size = 0;
+
+    return cbs_read_fragment_content(ctx, frag);
+}
+
+int ff_cbs_read(CodedBitstreamContext *ctx,
+                CodedBitstreamFragment *frag,
+                const uint8_t *data, size_t size)
+{
+    int err;
+
+    memset(frag, 0, sizeof(*frag));
+
+    // (We won't write to this during split.)
+    frag->data      = (uint8_t*)data;
+    frag->data_size = size;
+
+    err = ctx->codec->split_fragment(ctx, frag, 0);
+    if (err < 0)
+        return err;
+
+    frag->data      = NULL;
+    frag->data_size = 0;
+
+    return cbs_read_fragment_content(ctx, frag);
+}
+
+
+int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx,
+                               CodedBitstreamFragment *frag)
+{
+    int err, i;
+
+    for (i = 0; i < frag->nb_units; i++) {
+        if (!frag->units[i].content)
+            continue;
+
+        err = ctx->codec->write_unit(ctx, &frag->units[i]);
+        if (err < 0) {
+            av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to write unit %d "
+                   "(type %d).\n", i, frag->units[i].type);
+            return err;
+        }
+    }
+
+    err = ctx->codec->assemble_fragment(ctx, frag);
+    if (err < 0) {
+        av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to assemble fragment.\n");
+        return err;
+    }
+
+    return 0;
+}
+
+int ff_cbs_write_extradata(CodedBitstreamContext *ctx,
+                           AVCodecParameters *par,
+                           CodedBitstreamFragment *frag)
+{
+    int err;
+
+    err = ff_cbs_write_fragment_data(ctx, frag);
+    if (err < 0)
+        return err;
+
+    av_freep(&par->extradata);
+
+    par->extradata = av_malloc(frag->data_size +
+                               AV_INPUT_BUFFER_PADDING_SIZE);
+    if (!par->extradata)
+        return AVERROR(ENOMEM);
+
+    memcpy(par->extradata, frag->data, frag->data_size);
+    memset(par->extradata + frag->data_size, 0,
+           AV_INPUT_BUFFER_PADDING_SIZE);
+    par->extradata_size = frag->data_size;
+
+    return 0;
+}
+
+int ff_cbs_write_packet(CodedBitstreamContext *ctx,
+                        AVPacket *pkt,
+                        CodedBitstreamFragment *frag)
+{
+    int err;
+
+    err = ff_cbs_write_fragment_data(ctx, frag);
+    if (err < 0)
+        return err;
+
+    av_new_packet(pkt, frag->data_size);
+    if (err < 0)
+        return err;
+
+    memcpy(pkt->data, frag->data, frag->data_size);
+    pkt->size = frag->data_size;
+
+    return 0;
+}
+
+
+void ff_cbs_trace_header(CodedBitstreamContext *ctx,
+                         const char *name)
+{
+    if (!ctx->trace_enable)
+        return;
+
+    av_log(ctx->log_ctx, ctx->trace_level, "%s\n", name);
+}
+
+void ff_cbs_trace_syntax_element(CodedBitstreamContext *ctx, int position,
+                                 const char *name, const char *bits,
+                                 int64_t value)
+{
+    size_t name_len, bits_len;
+    int pad;
+
+    if (!ctx->trace_enable)
+        return;
+
+    av_assert0(value >= INT_MIN && value <= UINT32_MAX);
+
+    name_len = strlen(name);
+    bits_len = strlen(bits);
+
+    if (name_len + bits_len > 60)
+        pad = bits_len + 2;
+    else
+        pad = 61 - name_len;
+
+    av_log(ctx->log_ctx, ctx->trace_level, "%-10d  %s%*s = %"PRId64"\n",
+           position, name, pad, bits, value);
+}
+
+int ff_cbs_read_unsigned(CodedBitstreamContext *ctx, GetBitContext *gbc,
+                         int width, const char *name, uint32_t *write_to,
+                         uint32_t range_min, uint32_t range_max)
+{
+    uint32_t value;
+    int position;
+
+    av_assert0(width <= 32);
+
+    if (get_bits_left(gbc) < width) {
+        av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid value at "
+               "%s: bitstream ended.\n", name);
+        return AVERROR_INVALIDDATA;
+    }
+
+    if (ctx->trace_enable)
+        position = get_bits_count(gbc);
+
+    value = get_bits_long(gbc, width);
+
+    if (ctx->trace_enable) {
+        char bits[33];
+        int i;
+        for (i = 0; i < width; i++)
+            bits[i] = value >> (width - i - 1) & 1 ? '1' : '0';
+        bits[i] = 0;
+
+        ff_cbs_trace_syntax_element(ctx, position, name, bits, value);
+    }
+
+    if (value < range_min || value > range_max) {
+        av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
+               "%"PRIu32", but must be in [%"PRIu32",%"PRIu32"].\n",
+               name, value, range_min, range_max);
+        return AVERROR_INVALIDDATA;
+    }
+
+    *write_to = value;
+    return 0;
+}
+
+int ff_cbs_write_unsigned(CodedBitstreamContext *ctx, PutBitContext *pbc,
+                          int width, const char *name, uint32_t value,
+                          uint32_t range_min, uint32_t range_max)
+{
+    av_assert0(width <= 32);
+
+    if (value < range_min || value > range_max) {
+        av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
+               "%"PRIu32", but must be in [%"PRIu32",%"PRIu32"].\n",
+               name, value, range_min, range_max);
+        return AVERROR_INVALIDDATA;
+    }
+
+    if (put_bits_left(pbc) < width)
+        return AVERROR(ENOSPC);
+
+    if (ctx->trace_enable) {
+        char bits[33];
+        int i;
+        for (i = 0; i < width; i++)
+            bits[i] = value >> (width - i - 1) & 1 ? '1' : '0';
+        bits[i] = 0;
+
+        ff_cbs_trace_syntax_element(ctx, put_bits_count(pbc), name, bits, value);
+    }
+
+    if (width < 32)
+        put_bits(pbc, width, value);
+    else
+        put_bits32(pbc, value);
+
+    return 0;
+}
+
+
+static int cbs_insert_unit(CodedBitstreamContext *ctx,
+                           CodedBitstreamFragment *frag,
+                           int position)
+{
+    CodedBitstreamUnit *units;
+
+    units = av_malloc_array(frag->nb_units + 1, sizeof(*units));
+    if (!units)
+        return AVERROR(ENOMEM);
+
+    if (position > 0)
+        memcpy(units, frag->units, position * sizeof(*units));
+    if (position < frag->nb_units)
+        memcpy(units + position + 1, frag->units + position,
+               (frag->nb_units - position) * sizeof(*units));
+
+    memset(units + position, 0, sizeof(*units));
+
+    av_freep(&frag->units);
+    frag->units = units;
+    ++frag->nb_units;
+
+    return 0;
+}
+
+int ff_cbs_insert_unit_content(CodedBitstreamContext *ctx,
+                               CodedBitstreamFragment *frag,
+                               int position, uint32_t type,
+                               void *content)
+{
+    int err;
+
+    if (position == -1)
+        position = frag->nb_units;
+    av_assert0(position >= 0 && position <= frag->nb_units);
+
+    err = cbs_insert_unit(ctx, frag, position);
+    if (err < 0)
+        return err;
+
+    frag->units[position].type             = type;
+    frag->units[position].content          = content;
+    frag->units[position].content_external = 1;
+
+    return 0;
+}
+
+int ff_cbs_insert_unit_data(CodedBitstreamContext *ctx,
+                            CodedBitstreamFragment *frag,
+                            int position, uint32_t type,
+                            uint8_t *data, size_t data_size)
+{
+    int err;
+
+    if (position == -1)
+        position = frag->nb_units;
+    av_assert0(position >= 0 && position <= frag->nb_units);
+
+    err = cbs_insert_unit(ctx, frag, position);
+    if (err < 0)
+        return err;
+
+    frag->units[position].type      = type;
+    frag->units[position].data      = data;
+    frag->units[position].data_size = data_size;
+
+    return 0;
+}
+
+int ff_cbs_delete_unit(CodedBitstreamContext *ctx,
+                       CodedBitstreamFragment *frag,
+                       int position)
+{
+    if (position < 0 || position >= frag->nb_units)
+        return AVERROR(EINVAL);
+
+    cbs_unit_uninit(ctx, &frag->units[position]);
+
+    --frag->nb_units;
+
+    if (frag->nb_units == 0) {
+        av_freep(&frag->units);
+
+    } else {
+        memmove(frag->units + position,
+                frag->units + position + 1,
+                (frag->nb_units - position) * sizeof(*frag->units));
+
+        // Don't bother reallocating the unit array.
+    }
+
+    return 0;
+}
diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
new file mode 100644
index 0000000000..eeaff379e5
--- /dev/null
+++ b/libavcodec/cbs.h
@@ -0,0 +1,274 @@ 
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg 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.
+ *
+ * FFmpeg 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 FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef AVCODEC_CBS_H
+#define AVCODEC_CBS_H
+
+#include <stddef.h>
+#include <stdint.h>
+
+#include "avcodec.h"
+
+
+struct CodedBitstreamType;
+
+/**
+ * Coded bitstream unit structure.
+ *
+ * A bitstream unit the the smallest element of a bitstream which
+ * is meaningful on its own.  For example, an H.264 NAL unit.
+ *
+ * See the codec-specific header for the meaning of this for any
+ * particular codec.
+ */
+typedef struct CodedBitstreamUnit {
+    /**
+     * Codec-specific type of this unit.
+     */
+    uint32_t type;
+
+    /**
+     * Pointer to the bitstream form of this unit.
+     *
+     * May be NULL if the unit currently only exists in decomposed form.
+     */
+    uint8_t *data;
+    /**
+     * The number of bytes in the bitstream (including any padding bits
+     * in the final byte).
+     */
+    size_t   data_size;
+    /**
+     * The number of bits which should be ignored in the final byte.
+     *
+     * This supports non-byte-aligned bitstreams.
+     */
+    size_t   data_bit_padding;
+
+    /**
+     * Pointer to the decomposed form of this unit.
+     *
+     * The type of this structure depends on both the codec and the
+     * type of this unit.  May be NULL if the unit only exists in
+     * bitstream form.
+     */
+    void *content;
+    /**
+     * Whether the content was supplied externally.
+     *
+     * If so, it should not be freed when freeing the unit.
+     */
+    int   content_external;
+} CodedBitstreamUnit;
+
+/**
+ * Coded bitstream fragment structure, combining one or more units.
+ *
+ * This is any sequence of units.  It need not form some greater whole,
+ * though in many cases it will.  For example, an H.264 access unit,
+ * which is composed of a sequence of H.264 NAL units.
+ */
+typedef struct CodedBitstreamFragment {
+    /**
+     * Pointer to the bitstream form of this fragment.
+     *
+     * May be NULL if the fragment only exists as component units.
+     */
+    uint8_t *data;
+    /**
+     * The number of bytes in the bitstream.
+     *
+     * The number of bytes in the bitstream (including any padding bits
+     * in the final byte).
+     */
+    size_t   data_size;
+    /**
+     * The number of bits which should be ignored in the final byte.
+     */
+    size_t data_bit_padding;
+
+    /**
+     * Number of units in this fragment.
+     *
+     * This may be zero if the fragment only exists in bistream form
+     * and has not been decomposed.
+     */
+    int              nb_units;
+    /**
+     * Pointer to an array of units of length nb_units.
+     *
+     * Must be NULL if nb_units is zero.
+     */
+    CodedBitstreamUnit *units;
+} CodedBitstreamFragment;
+
+/**
+ * Context structure for coded bitstream operations.
+ */
+typedef struct CodedBitstreamContext {
+    /**
+     * Logging context to be passed to all av_log() calls associated
+     * with this context.
+     */
+    void *log_ctx;
+
+    /**
+     * Internal codec-specific hooks.
+     */
+    const struct CodedBitstreamType *codec;
+
+    /**
+     * Internal codec-specific data.
+     *
+     * This contains any information needed when reading/writing
+     * bitsteams which will not necessarily be present in a fragment.
+     * For example, for H.264 it contains all currently visible
+     * parameter sets - they are required to determine the bitstream
+     * syntax but need not be present in every access unit.
+     */
+    void *priv_data;
+
+    /**
+     * Array of unit types which should be decomposed when reading.
+     *
+     * Types not in this list will be available in bitstream form only.
+     * If NULL, all supported types will be decomposed.
+     */
+    uint32_t *decompose_unit_types;
+    /**
+     * Length of the decompose_unit_types array.
+     */
+    int    nb_decompose_unit_types;
+
+    /**
+     * Enable trace output during read/write operations.
+     */
+    int trace_enable;
+    /**
+     * Log level to use for trace output.
+     *
+     * From AV_LOG_*; defaults to AV_LOG_TRACE.
+     */
+    int trace_level;
+} CodedBitstreamContext;
+
+
+/**
+ * Initialise a new context for the given codec.
+ */
+int ff_cbs_init(CodedBitstreamContext *ctx,
+                enum AVCodecID codec_id, void *log_ctx);
+
+/**
+ * Close a context and free all internal state.
+ */
+void ff_cbs_close(CodedBitstreamContext *ctx);
+
+
+/**
+ * Read the extradata bitstream found in codec parameters into a
+ * fragment, then split into units and decompose.
+ *
+ * This also updates the internal state, so will need to be called for
+ * codecs with extradata to read parameter sets necessary for further
+ * parsing even if the fragment itself is not desired.
+ */
+int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
+                          CodedBitstreamFragment *frag,
+                          const AVCodecParameters *par);
+
+/**
+ * Read the data bitstream from a packet into a fragment, then
+ * split into units and decompose.
+ */
+int ff_cbs_read_packet(CodedBitstreamContext *ctx,
+                       CodedBitstreamFragment *frag,
+                       const AVPacket *pkt);
+
+/**
+ * Read a bitstream from a memory region into a fragment, then
+ * split into units and decompose.
+ */
+int ff_cbs_read(CodedBitstreamContext *ctx,
+                CodedBitstreamFragment *frag,
+                const uint8_t *data, size_t size);
+
+
+/**
+ * Write the content of the fragment to its own internal buffer.
+ *
+ * Writes the content of all units and then assembles them into a new
+ * data buffer.  When modifying the content of decomposed units, this
+ * can be used to regenerate the bitstream form of units or the whole
+ * fragment so that it can be extracted for other use.
+ */
+int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx,
+                               CodedBitstreamFragment *frag);
+
+/**
+ * Write the bitstream of a fragment to the extradata in codec parameters.
+ */
+int ff_cbs_write_extradata(CodedBitstreamContext *ctx,
+                           AVCodecParameters *par,
+                           CodedBitstreamFragment *frag);
+
+/**
+ * Write the bitstream of a fragment to a packet.
+ */
+int ff_cbs_write_packet(CodedBitstreamContext *ctx,
+                        AVPacket *pkt,
+                        CodedBitstreamFragment *frag);
+
+
+/**
+ * Free all allocated memory in a fragment.
+ */
+void ff_cbs_fragment_uninit(CodedBitstreamContext *ctx,
+                            CodedBitstreamFragment *frag);
+
+
+/**
+ * Insert a new unit into a fragment with the given content.
+ *
+ * The content structure continues to be owned by the caller, and
+ * will not be freed when the unit is.
+ */
+int ff_cbs_insert_unit_content(CodedBitstreamContext *ctx,
+                               CodedBitstreamFragment *frag,
+                               int position, uint32_t type,
+                               void *content);
+
+/**
+ * Insert a new unit into a fragment with the given data bitstream.
+ *
+ * The data buffer will be owned by the unit after this operation.
+ */
+int ff_cbs_insert_unit_data(CodedBitstreamContext *ctx,
+                            CodedBitstreamFragment *frag,
+                            int position, uint32_t type,
+                            uint8_t *data, size_t data_size);
+
+/**
+ * Delete a unit from a fragment and free all memory it uses.
+ */
+int ff_cbs_delete_unit(CodedBitstreamContext *ctx,
+                       CodedBitstreamFragment *frag,
+                       int position);
+
+
+#endif /* AVCODEC_CBS_H */
diff --git a/libavcodec/cbs_internal.h b/libavcodec/cbs_internal.h
new file mode 100644
index 0000000000..3bbc3355c2
--- /dev/null
+++ b/libavcodec/cbs_internal.h
@@ -0,0 +1,83 @@ 
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg 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.
+ *
+ * FFmpeg 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 FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef AVCODEC_CBS_INTERNAL_H
+#define AVCODEC_CBS_INTERNAL_H
+
+#include "avcodec.h"
+#include "cbs.h"
+#include "get_bits.h"
+#include "put_bits.h"
+
+
+typedef struct CodedBitstreamType {
+    enum AVCodecID codec_id;
+
+    size_t priv_data_size;
+
+    // Split frag->data into coded bitstream units, creating the
+    // frag->units array.  Fill data but not content on each unit.
+    int (*split_fragment)(CodedBitstreamContext *ctx,
+                          CodedBitstreamFragment *frag,
+                          int header);
+
+    // Read the unit->data bitstream and decompose it, creating
+    // unit->content.
+    int (*read_unit)(CodedBitstreamContext *ctx,
+                     CodedBitstreamUnit *unit);
+
+    // Write the unit->data bitstream from unit->content.
+    int (*write_unit)(CodedBitstreamContext *ctx,
+                      CodedBitstreamUnit *unit);
+
+    // Read the data from all of frag->units and assemble it into
+    // a bitstream for the whole fragment.
+    int (*assemble_fragment)(CodedBitstreamContext *ctx,
+                             CodedBitstreamFragment *frag);
+
+    // Free the content and data of a single unit.
+    void (*free_unit)(CodedBitstreamUnit *unit);
+
+    // Free the codec internal state.
+    void (*close)(CodedBitstreamContext *ctx);
+} CodedBitstreamType;
+
+
+// Helper functions for trace output.
+
+void ff_cbs_trace_header(CodedBitstreamContext *ctx,
+                         const char *name);
+
+void ff_cbs_trace_syntax_element(CodedBitstreamContext *ctx,
+                                 int position, const char *name,
+                                 const char *bitstring, int64_t value);
+
+
+// Helper functions for read/write of common bitstream elements, including
+// generation of trace output.
+
+int ff_cbs_read_unsigned(CodedBitstreamContext *ctx, GetBitContext *gbc,
+                         int width, const char *name, uint32_t *write_to,
+                         uint32_t range_min, uint32_t range_max);
+
+int ff_cbs_write_unsigned(CodedBitstreamContext *ctx, PutBitContext *pbc,
+                          int width, const char *name, uint32_t value,
+                          uint32_t range_min, uint32_t range_max);
+
+
+#endif /* AVCODEC_CBS_INTERNAL_H */