cbs is currently inconsistent regarding the opaque field that can be
used as a special argument to av_buffer_create in order to be used
during freeing the buffer: ff_cbs_alloc_unit_content and all the free
functions used name this parameter as if it should contain a pointer to
the unit whose content is about to be created; but both
ff_cbs_alloc_unit_content as well as ff_cbs_h264_add_sei_message
actually use a pointer to the CodedBitstreamContext as opaque. It should
actually be neither, because it is unneeded (as is evidenced by the fact
that none of the free functions use this pointer at all) and because it
ties the unit's content to the lifetime of other objects, although a
refcounted buffer is supposed to have its own lifetime that only ends
when its reference count reaches zero. This problem manifests itself in
the pointer becoming dangling.
The pointer to the unit can become dangling if another unit is added to
the fragment later as happens in the bitstream filters; in this case,
the pointer can point to the wrong unit (if the fragment's unit array
needn't be relocated) or it can point to where the array was earlier.
It can also become dangling if the unit's content is meant to survive
the resetting of the fragment it was originally read with. This applies
to the extradata of H.264 and HEVC.
The pointer to the context can become dangling if the context is closed
before the content is freed. Although this doesn't seem to happen right
now, it could happen, in particular if one uses different
CodedBitstreamContexts for in- and output.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
ff_cbs_delete_unit never fails if the index of the unit to delete is
valid, as it is with all current callers of the function. So just assert
in ff_cbs_delete_unit that the index is valid and change the return
value to void in order to remove the callers' checks for whether
ff_cbs_delete_unit failed.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Up until now, ff_cbs_write_packet always initialized the packet
structure it received without documenting this behaviour; furthermore,
the packet's buffer would (on success) be overwritten with the new
buffer without unreferencing the old. This meant that the input packet
had to be either clean (otherwise there would be memleaks) in which case
the initialization is redundant or uninitialized. ff_cbs_write_packet
was never used with uninitialized packets, so the initialization was
redundant. Worse yet, it forced callers to use more than one packet and
made it difficult to add side-data to a packet designated for output,
because said side-data could only be attached after the call to
ff_cbs_write_packet.
This has been changed. It is now allowed to use a non-blank packet.
The currently existing buffer will be unreferenced and replaced by
the new one, as will be the accompanying fields (i.e. data and size).
The rest isn't touched at all.
This change will enable us to use only one packet in the bitstream
filters that rely on CBS.
This commit also updates the documentation of ff_cbs_write_extradata
and ff_cbs_write_packet (to better describe existing behaviour and in
the latter case to also describe the new behaviour).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Currently, a fragment's unit array is constantly reallocated during
splitting of a packet. This commit changes this: One can keep the units
array by distinguishing between the number of allocated and the number
of valid units in the units array.
The more units a packet is split into, the bigger the benefit.
So MPEG-2 benefits the most; for a video coming from an NTSC-DVD
(usually 32 units per frame) the average cost of cbs_insert_unit (for a
single unit) went down from 6717 decicycles to 450 decicycles (based
upon 10 runs with 4194304 runs each); if each packet consists of only
one unit, it went down from 2425 to 448; for a H.264 video where most
packets contain nine units, it went from 4431 to 450.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@googlemail.com>
This way, every CodedBitstreamType->split_fragment() function can
safely assume the fragment passed to them will be reference counted,
potentially simplifying code.
Reviewed-by: Mark Thompson <sw@jkqxz.net>
Signed-off-by: James Almer <jamrial@gmail.com>
This saves one malloc + memcpy per packet
The CodedBitstreamFragment buffer is padded to follow the requirements
of AVPacket.
Reviewed-by: jkqxz
Signed-off-by: James Almer <jamrial@gmail.com>
This makes it easier for users of the CBS API to get alloc/free right -
all subelements use the buffer API so that it's clear how to free them.
It also allows eliding some redundant copies: the packet -> fragment copy
disappears after this change if the input packet is refcounted, and more
codec-specific cases are now possible (but not included in this patch).
This is harmless and should not be a warning - unknown units are passed
through to the write functions unchanged, and no other code will interact
with them.
Andreas Rheinhardt
James Almer
Andreas Rheinhardt
Mark Thompson