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

[Yusuke Nakamura]

2018-02-14 12:11 GMT+09:00 James Almer <jamrial at gmail.com>:

> 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.
>

Ooops! Sorry, originally it is for the case using  av_packet_ref not
av_packet_copy_props.
av_packet_ref expects the dst packet is inited by av_packet_init? If so,
should be documented on
av_packet_ref docs I think. I'm trapped that.


> > ---
> >  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.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>