| Message ID | 20200327022514.27554-1-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,v2,1/2] avcodec/avpacket: Always treat dst in av_packet_ref as uninitialized | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
Quoting Andreas Rheinhardt (2020-03-27 03:25:13) > av_packet_ref() mostly treated the destination packet dst as uninitialized, > i.e. the destination fields were simply overwritten. But if the source > packet was not reference-counted, dst->buf was treated as if it pointed > to an already allocated buffer (if != NULL) to be reallocated to the > desired size. > > The documentation did not explicitly state whether the dst will be treated > as uninitialized, but it stated that if the source packet is not refcounted, > a new buffer in dst will be allocated. This and the fact that the side-data > as well as the codepath taken in case src is refcounted always treated the > packet as uninitialized means that dst should always be treated as > uninitialized for the sake of consistency. And this behaviour has been > explicitly documented. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavcodec/avcodec.h | 2 +- > libavcodec/avpacket.c | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) ok
Anton Khirnov: > Quoting Andreas Rheinhardt (2020-03-27 03:25:13) >> av_packet_ref() mostly treated the destination packet dst as uninitialized, >> i.e. the destination fields were simply overwritten. But if the source >> packet was not reference-counted, dst->buf was treated as if it pointed >> to an already allocated buffer (if != NULL) to be reallocated to the >> desired size. >> >> The documentation did not explicitly state whether the dst will be treated >> as uninitialized, but it stated that if the source packet is not refcounted, >> a new buffer in dst will be allocated. This and the fact that the side-data >> as well as the codepath taken in case src is refcounted always treated the >> packet as uninitialized means that dst should always be treated as >> uninitialized for the sake of consistency. And this behaviour has been >> explicitly documented. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> >> --- >> libavcodec/avcodec.h | 2 +- >> libavcodec/avpacket.c | 1 + >> 2 files changed, 2 insertions(+), 1 deletion(-) > > ok > Applied, thanks. - Andreas
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index 78c483c25c..f918d20a61 100644 --- a/libavcodec/avcodec.h +++ b/libavcodec/avcodec.h @@ -4648,7 +4648,7 @@ void av_packet_free_side_data(AVPacket *pkt); * * @see av_packet_unref * - * @param dst Destination packet + * @param dst Destination packet. Will be completely overwritten. * @param src Source packet * * @return 0 on success, a negative AVERROR on error. diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c index b5667659fd..132567bc2d 100644 --- a/libavcodec/avpacket.c +++ b/libavcodec/avpacket.c @@ -615,6 +615,7 @@ int av_packet_ref(AVPacket *dst, const AVPacket *src) return ret; if (!src->buf) { + dst->buf = NULL; ret = packet_alloc(&dst->buf, src->size); if (ret < 0) goto fail;
av_packet_ref() mostly treated the destination packet dst as uninitialized, i.e. the destination fields were simply overwritten. But if the source packet was not reference-counted, dst->buf was treated as if it pointed to an already allocated buffer (if != NULL) to be reallocated to the desired size. The documentation did not explicitly state whether the dst will be treated as uninitialized, but it stated that if the source packet is not refcounted, a new buffer in dst will be allocated. This and the fact that the side-data as well as the codepath taken in case src is refcounted always treated the packet as uninitialized means that dst should always be treated as uninitialized for the sake of consistency. And this behaviour has been explicitly documented. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavcodec/avcodec.h | 2 +- libavcodec/avpacket.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)