[FFmpeg-devel] [PATCH 1/4] avcodec/avpacket: add av_packet_copy_side_data()

[James Almer]

On 9/25/2017 1:43 PM, wm4 wrote:
> On Mon, 25 Sep 2017 10:58:31 -0300
> James Almer <jamrial at gmail.com> wrote:
> 
>>> Using av_packet_copy_props() instead of introducing yet another weird
>>> function would be better.  
>>
>> It can't be used in some cases. Look at the last two patches in the set.
>> copy_props can't be used there without some extra pointless work.
> 
> Well, you need to copy the props first, before you write any fields
> that are touched by the function.

How so? The function only looks at side_data and side_data_size from a
const src packet, then writes those same two fields to the dst packet
if copying was successful. av_packet_free_side_data() also only cares
about those two fields.

I don't know why whoever wrote the code in ffmpeg.c and movenc.c didn't
use copy_props(), but a quick read hints they didn't want to copy any
other property except side data, apparently as it would break whatever
pts/duration calculations they were doing with their tmp packets.
They evidently added av_copy_packet_side_data() for a reason, and
replacing it with a saner, correctly named and less leaky/destructive
equivalent function is the entire point of this patch.

> But that's true for the various side
> data types too. What if you want to copy a specific set of side data
> values, but not others? You could add arbitrary permutations of this,
> one for every use case you could think of.

That's outside of the scope of generic convenience functions like all
these copy-every-whatever and free-every-whatever. You're supposed to
use get_side_data() to find specific elements of your choice.