[FFmpeg-devel] [PATCH 2/2] avcodec/avcodec, avpacket: Return blank packet on av_packet_ref() failure

[Anton Khirnov]

Quoting Andreas Rheinhardt (2020-03-27 03:25:14)
> Up until now, it was completely unspecified what the content of the
> destination packet dst was on error. Depending upon where the error
> happened calling av_packet_unref() on dst might be dangerous.
> 
> This commit changes this by making sure that dst is blank on error, so
> unreferencing it again is safe (and still pointless). This behaviour is
> documented.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
>  doc/APIchanges        | 4 ++++
>  libavcodec/avcodec.h  | 3 ++-
>  libavcodec/avpacket.c | 7 ++++---
>  libavcodec/version.h  | 2 +-
>  4 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 8eeaec2028..f2bb2d242b 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,10 @@ libavutil:     2017-10-21
>  
>  API changes, most recent first:
>  
> +2020-03-27 - xxxxxxxxxx - lavc 58.77.100 - avcodec.h
> +  av_packet_ref() now guarantees to return the destination packet
> +  in a blank state on error.
> +
>  2020-03-10 - xxxxxxxxxx - lavc 58.75.100 - avcodec.h
>    Add AV_PKT_DATA_ICC_PROFILE.
>  
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index f918d20a61..8fc0ad92c9 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -4651,7 +4651,8 @@ void av_packet_free_side_data(AVPacket *pkt);
>   * @param dst Destination packet. Will be completely overwritten.
>   * @param src Source packet
>   *
> - * @return 0 on success, a negative AVERROR on error.
> + * @return 0 on success, a negative AVERROR on error. On error, dst
> + *         will be blank (as if returned by av_packet_alloc()).
>   */
>  int av_packet_ref(AVPacket *dst, const AVPacket *src);
>  
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index 132567bc2d..c622718a45 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -610,12 +610,13 @@ int av_packet_ref(AVPacket *dst, const AVPacket *src)
>  {
>      int ret;
>  
> +    dst->buf = NULL;
> +

I really think av_init_packet() would be more robust against future
changes. But I'm not going to push for that especially strongly, so feel
free to push whichever version you prefer.

-- 
Anton Khirnov