[FFmpeg-devel] [PATCH] avcodec/avpacket: add av_packet_remove_side_data()

[Marton Balint]

On Wed, 9 Sep 2020, James Almer wrote:

> On 9/9/2020 12:47 PM, Marton Balint wrote:
>> 
>> 
>> On Tue, 8 Sep 2020, James Almer wrote:
>> 
>>> This helper removes a side data entry from the packet, maintaining the
>>> integrity and order of the remaining entries, if any.
>>>
>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>> ---
>>> Missing APIChanges entry and version bump.
>>>
>>> Couldn't find a place in the tree where it could be used right now, but
>>> it makes the API be more in line with the AVFrame one, so i decided to
>>> write it.
>>>
>>> libavcodec/avpacket.c | 24 ++++++++++++++++++++++++
>>> libavcodec/packet.h   |  8 ++++++++
>>> 2 files changed, 32 insertions(+)
>>>
>>> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
>>> index 4801163227..61ea81698c 100644
>>> --- a/libavcodec/avpacket.c
>>> +++ b/libavcodec/avpacket.c
>>> @@ -367,6 +367,30 @@ uint8_t *av_packet_get_side_data(const AVPacket
>>> *pkt, enum AVPacketSideDataType
>>>     return NULL;
>>> }
>>>
>>> +void av_packet_remove_side_data(AVPacket *pkt, enum
>>> AVPacketSideDataType type)
>>> +{
>>> +    int i, elems;
>>> +
>>> +    for (i = 0; i < pkt->side_data_elems; i++) {
>>> +        if (pkt->side_data[i].type != type)
>>> +            continue;
>>> +
>>> +        elems = pkt->side_data_elems - 1;
>>> +        av_freep(&pkt->side_data[i].data);
>>> +        pkt->side_data[i].size = 0;
>>> +
>>> +        if (i < elems) {
>>> +            memmove(&pkt->side_data[i], &pkt->side_data[i + 1],
>>> +                    (elems - i) * sizeof(*pkt->side_data));
>>> +        }
>>> +        if (!elems)
>>> +            av_freep(&pkt->side_data);
>>> +        pkt->side_data_elems = elems;
>>> +
>>> +        break;
>>> +    }
>>> +}
>> 
>> Why not use the same algorightm that is used in av_frame_remove_side_data?
>> Shorter and does not need memmove...
>
> Frame side data and packet side data are different.
>
> AVFrameSideData **side_data;
>
> vs
>
> AVPacketSideData *side_data;

Ok, but the algorithm still works, only it operates on structs and not 
pointers, right?

static void av_packet_remove_side_data(AVPacket *pkt, enum AVPacketSideDataType type)
{
     for (int i = pkt->side_data_elems - 1; i >= 0; i--) {
         AVPacketSideData *sd = &pkt->side_data[i];
         if (sd->type == type) {
             av_freep(&sd->data);
             pkt->side_data[i] = pkt->side_data[pkt->side_data_elems - 1];
             pkt->side_data_elems--;
         }
     }
}

Regards,
Marton