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

[James Almer]

On 3/19/2018 1:49 PM, James Almer wrote:
> On 3/19/2018 1:32 PM, wm4 wrote:
>> On Mon, 19 Mar 2018 12:42:16 -0300
>> James Almer <jamrial at gmail.com> wrote:
>>
>>> Useful as well to quickly make a packet reference counted when it
>>> isn't already so.
>>>
>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>> ---
>>>  libavcodec/avcodec.h  | 11 +++++++++++
>>>  libavcodec/avpacket.c | 24 ++++++++++++++++++++++++
>>>  2 files changed, 35 insertions(+)
>>>
>>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>>> index a8322fb62a..a78017f1fb 100644
>>> --- a/libavcodec/avcodec.h
>>> +++ b/libavcodec/avcodec.h
>>> @@ -4518,6 +4518,17 @@ void av_packet_move_ref(AVPacket *dst, AVPacket *src);
>>>   */
>>>  int av_packet_copy_props(AVPacket *dst, const AVPacket *src);
>>>  
>>> +/**
>>> + * Create a writable reference for the data described by a given packet,
>>> + * avoiding data copy if possible.
>>> + *
>>> + * @param pkt Packet whose data should be made writable.
>>> + *
>>> + * @return 0 on success, a negative AVERROR on failure. On failure, the
>>> + *         packet is unchanged.
>>> + */
>>> +int av_packet_make_writable(AVPacket *pkt);
>>> +
>>>  /**
>>>   * Convert valid timing fields (timestamps / durations) in a packet from one
>>>   * timebase to another. Timestamps with unknown values (AV_NOPTS_VALUE) will be
>>> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
>>> index fe8113ab76..0693ca6f62 100644
>>> --- a/libavcodec/avpacket.c
>>> +++ b/libavcodec/avpacket.c
>>> @@ -652,6 +652,30 @@ void av_packet_move_ref(AVPacket *dst, AVPacket *src)
>>>      src->size = 0;
>>>  }
>>>  
>>> +int av_packet_make_writable(AVPacket *pkt)
>>> +{
>>> +    AVBufferRef *buf = NULL;
>>> +    int ret;
>>> +
>>> +    if (pkt->buf && av_buffer_is_writable(pkt->buf))
>>> +        return 0;
>>> +
>>> +    if (!pkt->data)
>>> +        return AVERROR(EINVAL);
>>> +
>>> +    ret = packet_alloc(&buf, pkt->size);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +    if (pkt->size)
>>> +        memcpy(buf->data, pkt->data, pkt->size);
>>> +
>>> +    av_buffer_unref(&pkt->buf);
>>> +    pkt->buf  = buf;
>>> +    pkt->data = buf->data;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>  void av_packet_rescale_ts(AVPacket *pkt, AVRational src_tb, AVRational dst_tb)
>>>  {
>>>      if (pkt->pts != AV_NOPTS_VALUE)
>>
>> Why not just call av_buffer_make_writable()? This code seems fine too,
>> though.
> 
> That would require duplicating all or most of these checks every time
> you need to make sure a packet is writable. pkt->buf may be NULL,
> pkt->buf may already be writable, pkt->data may be NULL, the like.
> 
> I figured a helper function like this where you just call one function
> that is "make this packet writable, whatever the current state is" would
> come in handy and simplify some parts of the tree. See the two commits i
> mentioned in my reply to Paul, or the mpeg4_unpack_bframes patch i sent
> last night.

I'll apply this later today or tomorrow unless someone objects.