On Tue, 20 Oct 2015 15:07:42 +0200
Luca Barbato <[email protected]> wrote:

> As documented, `av_dup_packet` is broken by design, `av_packet_ref`
> matches the AVFrame ref-counted API and can be safely used instead.
> ---
>  doc/APIchanges            |  2 ++
>  libavcodec/avcodec.h      |  5 ++++-
>  libavcodec/avpacket.c     |  4 ++++
>  libavformat/matroskaenc.c |  7 +------
>  libavformat/mux.c         | 10 ++++------
>  5 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 72c1376..70bacb6 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -16,6 +16,8 @@ API changes, most recent first:
>  2015-xx-xx - xxxxxxx - lavc 57.0.0 - avcodec.h
>    Deprecate av_free_packet, av_packet_unref replaces it
>    and resets the packet in a more consistent way.
> +  Deprecate av_dup_packet, it is a no-op for most cases,
> +  use av_packet_ref to make a non-refcounted AVPacket refcounted.
>  
>  2015-xx-xx - xxxxxxx - lavc 57.0.0 - qsv.h
>    Add an API for allocating opaque surfaces.
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 4362aeb..a370e3e 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -3463,12 +3463,15 @@ int av_grow_packet(AVPacket *pkt, int grow_by);
>   */
>  int av_packet_from_data(AVPacket *pkt, uint8_t *data, int size);
>  
> +#if FF_API_AVPACKET_OLD_API
>  /**
>   * @warning This is a hack - the packet memory allocation stuff is broken. 
> The
>   * packet is allocated if it was not really allocated.
> + *
> + * @deprecated Use av_packet_ref
>   */
> +attribute_deprecated
>  int av_dup_packet(AVPacket *pkt);
> -#if FF_API_AVPACKET_OLD_API
>  /**
>   * Free a packet.
>   *
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index eaea061..cec5bf8 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -128,6 +128,8 @@ int av_packet_from_data(AVPacket *pkt, uint8_t *data, int 
> size)
>      return 0;
>  }
>  
> +#if FF_API_AVPACKET_OLD_API
> +FF_DISABLE_DEPRECATION_WARNINGS
>  #define ALLOC_MALLOC(data, size) data = av_malloc(size)
>  #define ALLOC_BUF(data, size)                \
>  do {                                         \
> @@ -187,6 +189,8 @@ failed_alloc:
>      av_packet_unref(pkt);
>      return AVERROR(ENOMEM);
>  }
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
>  
>  void av_packet_free_side_data(AVPacket *pkt)
>  {
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index f1bd272..baffd54 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -1608,12 +1608,7 @@ static int mkv_write_packet(AVFormatContext *s, 
> AVPacket *pkt)
>      // buffer an audio packet to ensure the packet containing the video
>      // keyframe's timecode is contained in the same cluster for WebM
>      if (codec_type == AVMEDIA_TYPE_AUDIO) {
> -        mkv->cur_audio_pkt = *pkt;
> -        if (pkt->buf) {
> -            mkv->cur_audio_pkt.buf = av_buffer_ref(pkt->buf);
> -            ret = mkv->cur_audio_pkt.buf ? 0 : AVERROR(ENOMEM);
> -        } else
> -            ret = av_dup_packet(&mkv->cur_audio_pkt);
> +        ret = av_packet_ref(&mkv->cur_audio_pkt, pkt);
>      } else
>          ret = mkv_write_packet_internal(s, pkt);
>      return ret;
> diff --git a/libavformat/mux.c b/libavformat/mux.c
> index e86d202..4a81e36 100644
> --- a/libavformat/mux.c
> +++ b/libavformat/mux.c
> @@ -416,12 +416,8 @@ int ff_interleave_add_packet(AVFormatContext *s, 
> AVPacket *pkt,
>      this_pktl      = av_mallocz(sizeof(AVPacketList));
>      if (!this_pktl)
>          return AVERROR(ENOMEM);
> -    this_pktl->pkt = *pkt;
> -    pkt->buf       = NULL;
> -    pkt->side_data = NULL;
> -    pkt->side_data_elems = 0;
> -    // Duplicate the packet if it uses non-allocated memory
> -    if ((ret = av_dup_packet(&this_pktl->pkt)) < 0) {
> +
> +    if ((ret = av_packet_ref(&this_pktl->pkt, pkt)) < 0) {
>          av_free(this_pktl);
>          return ret;
>      }
> @@ -450,6 +446,8 @@ next_non_null:
>      s->streams[pkt->stream_index]->last_in_packet_buffer =
>          *next_point                                      = this_pktl;
>  
> +    av_packet_unref(pkt);
> +
>      return 0;
>  }
>  

Again, are you sure these packers are unreffed/reset before calling
av_packet_ref() on them?

Seems like it might be fine for matroskaenc, though it uses
"mkv->cur_audio_pkt.size > 0" as a check whether the packet is set
(would break with 0-sized packets).
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to