Mark Thompson: > Unit types are split into three categories, depending on how their > content is managed: > * POD structure - these require no special treatment. > * Structure containing references to refcounted buffers - these can use > a common free function when the offsets of all the internal references > are known. > * More complex structures - these still require ad-hoc treatment. > > For each codec we can then maintain a table of descriptors for each set of > equivalent unit types, defining the mechanism needed to allocate/free that > unit content. This is not required to be used immediately - a new alloc > function supports this, but does not replace the old one which works without > referring to these tables. > --- > libavcodec/cbs.c | 69 +++++++++++++++++++++++++++++++++++++++ > libavcodec/cbs.h | 9 +++++ > libavcodec/cbs_internal.h | 60 ++++++++++++++++++++++++++++++++++ > 3 files changed, 138 insertions(+) > > diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c > index 0bd5e1ac5d..6cc559e545 100644 > --- a/libavcodec/cbs.c > +++ b/libavcodec/cbs.c > @@ -812,3 +812,72 @@ void ff_cbs_delete_unit(CodedBitstreamContext *ctx, > frag->units + position + 1, > (frag->nb_units - position) * sizeof(*frag->units)); > } > + > +static void cbs_default_free_unit_content(void *opaque, uint8_t *data) > +{ > + const CodedBitstreamUnitTypeDescriptor *desc = opaque; > + if (desc->content_type == CBS_CONTENT_TYPE_INTERNAL_REFS) { > + int i; > + for (i = 0; i < desc->nb_ref_offsets; i++) { > + void **ptr = (void**)(data + desc->ref_offsets[i]); > + av_buffer_unref((AVBufferRef**)(ptr + 1)); > + } > + } > + av_free(data); > +} > + > +static const CodedBitstreamUnitTypeDescriptor > + *cbs_find_unit_type_desc(CodedBitstreamContext *ctx, > + CodedBitstreamUnit *unit) > +{ > + const CodedBitstreamUnitTypeDescriptor *desc; > + int i, j; > + > + if (!ctx->codec->unit_types) > + return NULL; > + > + for (i = 0;; i++) { > + desc = &ctx->codec->unit_types[i]; > + if (desc->nb_unit_types == 0) > + break; > + if (desc->nb_unit_types == CBS_UNIT_TYPE_RANGE) { > + if (unit->type >= desc->unit_type_range_start && > + unit->type <= desc->unit_type_range_end) > + return desc; > + } else { > + for (j = 0; j < desc->nb_unit_types; j++) { > + if (desc->unit_types[j] == unit->type) > + return desc; > + } > + } > + } > + return NULL; > +} > + > +int ff_cbs_alloc_unit_content2(CodedBitstreamContext *ctx, > + CodedBitstreamUnit *unit) > +{ > + const CodedBitstreamUnitTypeDescriptor *desc; > + > + av_assert0(!unit->content && !unit->content_ref); > + > + desc = cbs_find_unit_type_desc(ctx, unit); > + if (!desc) > + return AVERROR(ENOSYS); > + > + unit->content = av_mallocz(desc->content_size); > + if (!unit->content) > + return AVERROR(ENOMEM); > + > + unit->content_ref = > + av_buffer_create(unit->content, desc->content_size, > + desc->content_free ? desc->content_free > + : cbs_default_free_unit_content, > + (void*)desc, 0); > + if (!unit->content_ref) { > + av_freep(&unit->content); > + return AVERROR(ENOMEM); > + } > + > + return 0; > +} > diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h > index cb3081e2c6..2a5959a2b0 100644 > --- a/libavcodec/cbs.h > +++ b/libavcodec/cbs.h > @@ -352,6 +352,15 @@ int ff_cbs_alloc_unit_content(CodedBitstreamContext *ctx, > size_t size, > void (*free)(void *opaque, uint8_t *content)); > > +/** > + * Allocate a new internal content buffer matching the type of the unit. > + * > + * The content will be zeroed. > + */ > +int ff_cbs_alloc_unit_content2(CodedBitstreamContext *ctx, > + CodedBitstreamUnit *unit); > + > + > /** > * Allocate a new internal data buffer of the given size in the unit. > * > diff --git a/libavcodec/cbs_internal.h b/libavcodec/cbs_internal.h > index 4c5a535ca6..615f514a85 100644 > --- a/libavcodec/cbs_internal.h > +++ b/libavcodec/cbs_internal.h > @@ -25,11 +25,71 @@ > #include "put_bits.h" > > > +enum { > + // Unit content is a simple structure. > + CBS_CONTENT_TYPE_POD, > + // Unit content contains some references to other structures, but all > + // managed via buffer reference counting. The descriptor defines the > + // structure offsets of every buffer reference. > + CBS_CONTENT_TYPE_INTERNAL_REFS, > + // Unit content is something more complex. The descriptor defines > + // special functions to manage the content. > + CBS_CONTENT_TYPE_COMPLEX, > +}; > + > +enum { > + // Maximum number of unit types described by the same unit type > + // descriptor. > + CBS_MAX_UNIT_TYPES = 16,
This is quite big and I wonder whether it would not be better to simply split the HEVC-slices into two range descriptors in order to reduce this to three. > + // Maximum number of reference buffer offsets in any one unit. > + CBS_MAX_REF_OFFSETS = 2, > + // Special value used in a unit type descriptor to indicate that it > + // applies to a large range of types rather than a set of discrete > + // values. > + CBS_UNIT_TYPE_RANGE = -1, > +}; > + > +typedef struct CodedBitstreamUnitTypeDescriptor { > + // Number of entries in the unit_types array, or the special value > + // CBS_UNIT_TYPE_RANGE to indicate that the range fields should be > + // used instead. > + int nb_unit_types; > + > + // Array of unit types that this entry describes. > + const CodedBitstreamUnitType unit_types[CBS_MAX_UNIT_TYPES]; > + > + // Start and end of unit type range, used if nb_unit_types == 0. nb_unit_types == 0 is actually used for the sentinel in the CodedBitstreamUnitTypeDescriptor-array. nb_unit_types == CBS_UNIT_TYPE_RANGE indicates that this descriptor uses ranges. > + const CodedBitstreamUnitType unit_type_range_start; > + const CodedBitstreamUnitType unit_type_range_end; The ranges could be free (size-wise) if you used a union with unit_types. > + > + // The type of content described, from CBS_CONTENT_TYPE_*. > + int content_type; Maybe make a proper type out of the CBS_CONTENT_TYPE_*-enum and use it here? > + // The size of the structure which should be allocated to contain > + // the decomposed content of this type of unit. > + size_t content_size; > + > + // Number of entries in the ref_offsets array. Only used if the > + // content_type is CBS_CONTENT_TYPE_INTERNAL_REFS. > + int nb_ref_offsets; > + // The structure must contain two adjacent elements: > + // type *field; > + // AVBufferRef *field_ref; > + // where field points to something in the buffer referred to by > + // field_ref. This offset is then set to offsetof(struct, field). > + size_t ref_offsets[CBS_MAX_REF_OFFSETS]; > + > + void (*content_free)(void *opaque, uint8_t *data); Is there a usecase for a dedicated free-function different for a unit of type CBS_CONTENT_TYPE_INTERNAL_REFS? If not, then one could use a union for this and the ref_offset stuff. > +} CodedBitstreamUnitTypeDescriptor; I wonder whether you should add const to the typedef in order to omit it everywhere else. After all, no CodedBitstreamUnitTypeDescriptor will ever be assembled during runtime. > + > typedef struct CodedBitstreamType { > enum AVCodecID codec_id; > > size_t priv_data_size; > > + // List of unit type descriptors for this codec. > + // Terminated by a descriptor with nb_unit_types equal to zero. > + const CodedBitstreamUnitTypeDescriptor *unit_types; > + > // Split frag->data into coded bitstream units, creating the > // frag->units array. Fill data but not content on each unit. > // The header argument should be set if the fragment came from > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".