| Message ID | 20180201104427.79756-1-martin@martin.st |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
On 01/02/2018 11:44, Martin Storsjö wrote: > We already allocate the internal extradata buffer with initialized > padding in flv_queue_extradata, but when copied into side data > (which is allocated by av_packet_new_side_data, which does allocate > the padding but doesn't initialize it), we forgot to initialize > the padding there. > --- > libavformat/flvdec.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c > index 81a71d39f4..23d320900b 100644 > --- a/libavformat/flvdec.c > +++ b/libavformat/flvdec.c > @@ -989,6 +989,10 @@ skip: > if (side) { > memcpy(side, flv->new_extradata[is_audio], > flv->new_extradata_size[is_audio]); > + // av_packet_new_side_data allocates space for padding but doesn't > + // initialize it. > + memset(side + flv->new_extradata_size[is_audio], 0, > + AV_INPUT_BUFFER_PADDING_SIZE); > av_freep(&flv->new_extradata[is_audio]); > flv->new_extradata_size[is_audio] = 0; > } > Ok.
On Thu, 1 Feb 2018, Luca Barbato wrote: > On 01/02/2018 11:44, Martin Storsjö wrote: >> We already allocate the internal extradata buffer with initialized >> padding in flv_queue_extradata, but when copied into side data >> (which is allocated by av_packet_new_side_data, which does allocate >> the padding but doesn't initialize it), we forgot to initialize >> the padding there. >> --- >> libavformat/flvdec.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c >> index 81a71d39f4..23d320900b 100644 >> --- a/libavformat/flvdec.c >> +++ b/libavformat/flvdec.c >> @@ -989,6 +989,10 @@ skip: >> if (side) { >> memcpy(side, flv->new_extradata[is_audio], >> flv->new_extradata_size[is_audio]); >> + // av_packet_new_side_data allocates space for padding but > doesn't >> + // initialize it. >> + memset(side + flv->new_extradata_size[is_audio], 0, >> + AV_INPUT_BUFFER_PADDING_SIZE); >> av_freep(&flv->new_extradata[is_audio]); >> flv->new_extradata_size[is_audio] = 0; >> } >> > > Ok. The other alternative would be to make av_packet_new_side_data zero-initialize the side data that it allocates (or at least the padding?), to avoid having to keep assumptions like these over here. (That would also match what ffmpeg does.) Would that perhaps be an even nicer solution, or does someone prefer keeping the allocation uninitialized like now? // Martin
On 01/02/2018 12:42, Martin Storsjö wrote: > The other alternative would be to make av_packet_new_side_data > zero-initialize the side data that it allocates (or at least the > padding?), to avoid having to keep assumptions like these over here. > (That would also match what ffmpeg does.) Would that perhaps be an even > nicer solution, or does someone prefer keeping the allocation > uninitialized like now? Probably shouldn't hurt and avoid some issues in the future. lu
diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c index 81a71d39f4..23d320900b 100644 --- a/libavformat/flvdec.c +++ b/libavformat/flvdec.c @@ -989,6 +989,10 @@ skip: if (side) { memcpy(side, flv->new_extradata[is_audio], flv->new_extradata_size[is_audio]); + // av_packet_new_side_data allocates space for padding but doesn't + // initialize it. + memset(side + flv->new_extradata_size[is_audio], 0, + AV_INPUT_BUFFER_PADDING_SIZE); av_freep(&flv->new_extradata[is_audio]); flv->new_extradata_size[is_audio] = 0; }