[FFmpeg-devel] [PATCH 1/4] avcodec/avpacket: add av_packet_copy_side_data()

[James Almer]

On 9/25/2017 6:50 AM, wm4 wrote:
> On Sun, 24 Sep 2017 22:06:09 -0300
> James Almer <jamrial at gmail.com> wrote:
> 
>> It's added as a replacemet of av_copy_packet_side_data() using the proper
>> av_packet_* namespace, and differs from it in the following ways:
>>
>> - Side data already allocated and copied is properly freed in case of failure.
>> - The dst packet is unchanged in case of failure instead of being unreffed.
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>>  doc/APIchanges        |  3 +++
>>  libavcodec/avcodec.h  | 16 ++++++++++++++++
>>  libavcodec/avpacket.c | 41 +++++++++++++++++++++++++++++++++++++++++
>>  libavcodec/version.h  |  4 ++--
>>  4 files changed, 62 insertions(+), 2 deletions(-)
>>
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index d06144f1e9..2d3f470186 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -15,6 +15,9 @@ libavutil:     2015-08-28
>>  
>>  API changes, most recent first:
>>  
>> +2017-09-xx - xxxxxxx - lavc 57.107.100 - avcodec.h
>> +  Add av_packet_copy_side_data().
>> +
>>  2017-xx-xx - xxxxxxx - lavu 55.76.100 / 56.6.0 - pixdesc.h
>>    Add av_color_range_from_name(), av_color_primaries_from_name(),
>>    av_color_transfer_from_name(), av_color_space_from_name(), and
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index 07d9f3e255..655e6abbde 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -4624,7 +4624,10 @@ int av_copy_packet(AVPacket *dst, const AVPacket *src);
>>   * Copy packet side data
>>   *
>>   * @return 0 on success, negative AVERROR on fail
>> + *
>> + * @deprecated Use av_packet_copy_side_data
>>   */
>> +attribute_deprecated
>>  int av_copy_packet_side_data(AVPacket *dst, const AVPacket *src);
>>  
>>  /**
>> @@ -4664,6 +4667,19 @@ uint8_t* av_packet_new_side_data(AVPacket *pkt, enum AVPacketSideDataType type,
>>  int av_packet_add_side_data(AVPacket *pkt, enum AVPacketSideDataType type,
>>                              uint8_t *data, size_t size);
>>  
>> +/**
>> + * Copy packet side data from src to dst.
>> + *
>> + * @see av_packet_copy_props
>> + * @see av_packet_free_side_data
>> + *
>> + * @param dst Destination packet
>> + * @param src Source packet
>> + *
>> + * @return 0 on success, a negative AVERROR on error. On failure, dst is unchanged.
>> + */
>> +int av_packet_copy_side_data(AVPacket *dst, const AVPacket *src);
>> +
>>  /**
>>   * Shrink the already allocated side data buffer
>>   *
>> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
>> index b07180eac8..33e325c9ed 100644
>> --- a/libavcodec/avpacket.c
>> +++ b/libavcodec/avpacket.c
>> @@ -582,6 +582,47 @@ int av_packet_shrink_side_data(AVPacket *pkt, enum AVPacketSideDataType type,
>>      return AVERROR(ENOENT);
>>  }
>>  
>> +int av_packet_copy_side_data(AVPacket *dst, const AVPacket *src)
>> +{
>> +    AVPacketSideData *side_data;
>> +    int i, side_data_elems = 0;
>> +
>> +    if (!src->side_data_elems)
>> +        return 0;
>> +
>> +    side_data = av_malloc_array(src->side_data_elems, sizeof(*src->side_data));
>> +    if (!side_data)
>> +        return AVERROR(ENOMEM);
>> +
>> +    for (i = 0; i < src->side_data_elems; i++) {
>> +        AVPacketSideData *sd_src = &src->side_data[i];
>> +        AVPacketSideData *sd_dst = &side_data[i];
>> +
>> +        if ((unsigned)sd_src->size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE)
>> +            goto fail;
>> +
>> +        sd_dst->data = av_mallocz(sd_src->size + AV_INPUT_BUFFER_PADDING_SIZE);
>> +        if (!sd_dst->data)
>> +            goto fail;
>> +
>> +        memcpy(sd_dst->data, sd_src->data, sd_src->size);
>> +        sd_dst->size = sd_src->size;
>> +        sd_dst->type = sd_src->type;
>> +        side_data_elems++;
>> +    }
>> +    dst->side_data = side_data;
>> +    dst->side_data_elems = side_data_elems;
>> +
>> +    return 0;
>> +
>> +fail:
>> +    for (i = 0; i < side_data_elems; i++)
>> +        av_free(side_data[i].data);
>> +    av_free(side_data);
>> +
>> +    return AVERROR(ENOMEM);;
>> +}
>> +
>>  int av_packet_copy_props(AVPacket *dst, const AVPacket *src)
>>  {
>>      int i;
>> diff --git a/libavcodec/version.h b/libavcodec/version.h
>> index e1224752bd..10d9ac4eb3 100644
>> --- a/libavcodec/version.h
>> +++ b/libavcodec/version.h
>> @@ -28,8 +28,8 @@
>>  #include "libavutil/version.h"
>>  
>>  #define LIBAVCODEC_VERSION_MAJOR  57
>> -#define LIBAVCODEC_VERSION_MINOR 106
>> -#define LIBAVCODEC_VERSION_MICRO 101
>> +#define LIBAVCODEC_VERSION_MINOR 107
>> +#define LIBAVCODEC_VERSION_MICRO 100
>>  
>>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
>>                                                 LIBAVCODEC_VERSION_MINOR, \
> 
> Using av_packet_copy_props() instead of introducing yet another weird
> function would be better.

It can't be used in some cases. Look at the last two patches in the set.
copy_props can't be used there without some extra pointless work.
This functionality is only part of what copy_props does. I'm simply
splitting it out from it.