[FFmpeg-devel] [PATCH] Add av_copy_packet()

[Michael Niedermayer]

On Tue, Sep 18, 2012 at 01:14:48AM +0300, Andrey Utkin wrote:
> This function makes full deep copy of AVPacket contents.
> ---
>  libavcodec/avcodec.h  |    7 ++++++
>  libavcodec/avpacket.c |   58 +++++++++++++++++++++++++++++-------------------
>  2 files changed, 42 insertions(+), 23 deletions(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 62a261a..783ac96 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -3666,6 +3666,13 @@ int av_grow_packet(AVPacket *pkt, int grow_by);
>  int av_dup_packet(AVPacket *pkt);
>  
>  /**
> + * Copy packet, including contents
> + *
> + * @return 0 on success, negative AVERROR on fail
> + */
> +int av_copy_packet(AVPacket *dst, AVPacket *src);
> +
> +/**
>   * Free a packet.
>   *
>   * @param pkt packet to free
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index 17c100e..6818ef6 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -125,40 +125,52 @@ int av_grow_packet(AVPacket *pkt, int grow_by)
>          dst = data;                                                     \
>      } while (0)
>  
> -int av_dup_packet(AVPacket *pkt)
> +/* Makes duplicates of data, side_data, but does not copy any other fields */
> +static int av_copy_packet_data(AVPacket *dst, AVPacket *src)

the av_* prefix means public API, yet its static thus not public,
i think the av_ prefix should be removed


>  {
> -    AVPacket tmp_pkt;
> -
> -    if (pkt->destruct == NULL && pkt->data) {
> -        tmp_pkt = *pkt;
> +    dst->data      = NULL;
> +    dst->side_data = NULL;
> +    DUP_DATA(dst->data, src->data, dst->size, 1);
> +    dst->destruct = av_destruct_packet;

I think either the copy should be implemented with the current
av_dup_packet() making the diff much simpler or
the redundant operations like first copying and then reseting data
side data, destruct should be avoided.
also maybe you could split the diff so that variable renamings
(pkt->dst, "tmp_pkt."->"src->") would be seperate from functional
changes. This would it much clearer what is changed in each patch.

thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120920/d85d8369/attachment.asc>