[FFmpeg-devel] [PATCH 0/5] New version

Andreas Rheinhardt andreas.rheinhardt at googlemail.com
Sat Nov 24 03:55:33 EET 2018

I actually never liked these magic defines myself. So I followed
your proposal and in doing so I realized that it doesn't really
solve the need for these defines: One still has to differentiate the
case where a external buffer exists and the case where no such buffer
exists (because one needs a special free function for the first case,
unless one wants to create unnecessary free functions for the second
case). The only advantage such a solution has is that it is easily
extendable to the case where a struct contains more than one external
buffer. And this is a case that simply doesn't exist at the moment, so I
didn't really care about it and instead kept it simple.*

The SIZE define could be replaced by adding explicit macro arguments
whether the size element value is in bits or in bytes.

Given that one needs to have a special free callback for structures with
external buffers, it makes sense to automatically generate them together
with the copy functions. And this is what I did. These functions really
belong together. (I do not consider replacing the free functions with
macros to be obscure per se, but the old variable names was definitely

The macro is in cbs_internal so that it can be used e.g. for
MPEG2RawUserData. Of course, it could be transferred to cbs_h2645.c.

ff_cbs_make_unit_writable is still called so, although it only makes the
unit's content writable, because it takes a unit argument. I wanted to
write generic code to (optionally) make the data writable, too, but I
refrained from it after I realized that the very definition of writable
isn't clear for slice units at all: Both the unit and the unit's content
contain a data_ref that usually point to the same buffer, so that a
unit's data could be said to be writable if it owns all references to
its data. Therefore determining the writability of the data would be
codec-dependent, so I left this can of worms closed for now (and probably

*: For the record, this is the structure I ended up using in this
struct {
    size_t    data_offset;
    size_t    ref_offset;
    size_t    size_offset;
    SizeType  size_type;
    SizeScale size_scale;
    size_t    plus_size;
SizeType is the type of the element containing the size.

Andreas Rheinhardt (5):
  cbs: Add function to make content of a unit writable
  cbs: Add a macro to create functions for deep copying
  cbs_h2645: Implement copy-functions for parameter sets
  cbs_h2645: Implement functions to make a unit's content writable
  h264_redundant_pps: Make it reference-compatible

 libavcodec/cbs.c                    |  14 ++++
 libavcodec/cbs.h                    |   6 ++
 libavcodec/cbs_av1.c                |   1 +
 libavcodec/cbs_h2645.c              | 112 +++++++++++++++++++++-------
 libavcodec/cbs_internal.h           |  57 ++++++++++++++
 libavcodec/cbs_jpeg.c               |   1 +
 libavcodec/cbs_mpeg2.c              |   1 +
 libavcodec/cbs_vp9.c                |   1 +
 libavcodec/h264_redundant_pps_bsf.c |  15 +++-
 9 files changed, 176 insertions(+), 32 deletions(-)


More information about the ffmpeg-devel mailing list