[FFmpeg-devel] [PATCH] avpacket: Set dst->side_data_elems to 0 within av_packet_copy_props.

James Almer jamrial at gmail.com
Wed Feb 14 05:11:32 EET 2018


On 2/13/2018 11:43 PM, Yusuke Nakamura wrote:
> This makes you need not call av_init_packet before av_packet_copy_props like the following.
> 
> AVPacket dst;
> av_packet_copy_props(&dst, &src);

In this scenario, dst->side_data is uninitialized, and bad things can
happen when av_packet_copy_props calls av_packet_new_side_data.

For something like this, it's best to instead just zero initialize dst.
Also, av_packet_init should *always* be called before a packet stored on
stack is used, regardless of it being zero initialized or not.

> ---
>  libavcodec/avpacket.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index 90b8215928..1a9be60e20 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -571,6 +571,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>      dst->flags                = src->flags;
>      dst->stream_index         = src->stream_index;
>  
> +    dst->side_data_elems = 0;
>      for (i = 0; i < src->side_data_elems; i++) {
>           enum AVPacketSideDataType type = src->side_data[i].type;
>           int size          = src->side_data[i].size;
> 

Afaik, the intended behavior of this function was to merge the side data
in dst with that of src, and this patch would break that.
It's admittedly not really defined and can get confusing, especially
when the old deprecated API (av_copy_packet, av_copy_packet_side_data,
av_dup_packet) do seem to just completely overwrite rather than merge.

IMO, we should first define what should happen with side data in this
function before we make any further changes to it.


More information about the ffmpeg-devel mailing list