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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel