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 obscuring.) 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 forever). *: For the record, this is the structure I ended up using in this approach: 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(-) -- 2.19.1 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel