[FFmpeg-devel,01/13] avformat/mux: Move packet references

In the common case that the input packet was already refcounted,
ff_interleave_add_packet would allocate a new AVPacketList, use
av_packet_ref to create a new reference to the buffer for the
AVPacketList's packet, interleave the packet and finally unreference
the original input packet.
This commit changes this: It uses av_packet_move_ref to transfer
the packet to its destination. In case the input packet is refcounted,
this saves an allocation and a free (of an AVBufferRef); if not, the
packet is made refcounted before moving it. When the input packet has
side data, one saves even more than one allocation+free.

Furthermore, when the packet is in reality an uncoded frame, a hacky
ad-hoc variant of av_packet_move_ref has been employed. Not any more.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/mux.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)
  

Andreas Rheinhardt:
> In the common case that the input packet was already refcounted,
> ff_interleave_add_packet would allocate a new AVPacketList, use
> av_packet_ref to create a new reference to the buffer for the
> AVPacketList's packet, interleave the packet and finally unreference
> the original input packet.
> This commit changes this: It uses av_packet_move_ref to transfer
> the packet to its destination. In case the input packet is refcounted,
> this saves an allocation and a free (of an AVBufferRef); if not, the
> packet is made refcounted before moving it. When the input packet has
> side data, one saves even more than one allocation+free.
> 
> Furthermore, when the packet is in reality an uncoded frame, a hacky
> ad-hoc variant of av_packet_move_ref has been employed. Not any more.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavformat/mux.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/libavformat/mux.c b/libavformat/mux.c
> index 8ab5ea8c2b..ac370fb24d 100644
> --- a/libavformat/mux.c
> +++ b/libavformat/mux.c
> @@ -930,17 +930,16 @@ int ff_interleave_add_packet(AVFormatContext *s, AVPacket *pkt,
>      if ((pkt->flags & AV_PKT_FLAG_UNCODED_FRAME)) {
>          av_assert0(pkt->size == UNCODED_FRAME_PACKET_SIZE);
>          av_assert0(((AVFrame *)pkt->data)->buf);
> -        this_pktl->pkt = *pkt;
> -        pkt->buf = NULL;
> -        pkt->side_data = NULL;
> -        pkt->side_data_elems = 0;
>      } else {
> -        if ((ret = av_packet_ref(&this_pktl->pkt, pkt)) < 0) {
> +        if ((ret = av_packet_make_refcounted(pkt)) < 0) {
>              av_free(this_pktl);
>              return ret;
>          }
>      }
>  
> +    av_packet_move_ref(&this_pktl->pkt, pkt);
> +    pkt = &this_pktl->pkt;
> +
>      if (s->streams[pkt->stream_index]->last_in_packet_buffer) {
>          next_point = &(st->last_in_packet_buffer->next);
>      } else {
> @@ -989,8 +988,6 @@ next_non_null:
>      s->streams[pkt->stream_index]->last_in_packet_buffer =
>          *next_point                                      = this_pktl;
>  
> -    av_packet_unref(pkt);
> -
>      return 0;
>  }
>  
> 
Ping for this patch and the remaining unmerged patches in this
patchset (patches 1, 2, 4, 5, 7-13).

- Andreas
  

Andreas Rheinhardt:
> Andreas Rheinhardt:
>> In the common case that the input packet was already refcounted,
>> ff_interleave_add_packet would allocate a new AVPacketList, use
>> av_packet_ref to create a new reference to the buffer for the
>> AVPacketList's packet, interleave the packet and finally unreference
>> the original input packet.
>> This commit changes this: It uses av_packet_move_ref to transfer
>> the packet to its destination. In case the input packet is refcounted,
>> this saves an allocation and a free (of an AVBufferRef); if not, the
>> packet is made refcounted before moving it. When the input packet has
>> side data, one saves even more than one allocation+free.
>>
>> Furthermore, when the packet is in reality an uncoded frame, a hacky
>> ad-hoc variant of av_packet_move_ref has been employed. Not any more.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
>> ---
>>  libavformat/mux.c | 11 ++++-------
>>  1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavformat/mux.c b/libavformat/mux.c
>> index 8ab5ea8c2b..ac370fb24d 100644
>> --- a/libavformat/mux.c
>> +++ b/libavformat/mux.c
>> @@ -930,17 +930,16 @@ int ff_interleave_add_packet(AVFormatContext *s, AVPacket *pkt,
>>      if ((pkt->flags & AV_PKT_FLAG_UNCODED_FRAME)) {
>>          av_assert0(pkt->size == UNCODED_FRAME_PACKET_SIZE);
>>          av_assert0(((AVFrame *)pkt->data)->buf);
>> -        this_pktl->pkt = *pkt;
>> -        pkt->buf = NULL;
>> -        pkt->side_data = NULL;
>> -        pkt->side_data_elems = 0;
>>      } else {
>> -        if ((ret = av_packet_ref(&this_pktl->pkt, pkt)) < 0) {
>> +        if ((ret = av_packet_make_refcounted(pkt)) < 0) {
>>              av_free(this_pktl);
>>              return ret;
>>          }
>>      }
>>  
>> +    av_packet_move_ref(&this_pktl->pkt, pkt);
>> +    pkt = &this_pktl->pkt;
>> +
>>      if (s->streams[pkt->stream_index]->last_in_packet_buffer) {
>>          next_point = &(st->last_in_packet_buffer->next);
>>      } else {
>> @@ -989,8 +988,6 @@ next_non_null:
>>      s->streams[pkt->stream_index]->last_in_packet_buffer =
>>          *next_point                                      = this_pktl;
>>  
>> -    av_packet_unref(pkt);
>> -
>>      return 0;
>>  }
>>  
>>
> Ping for this patch and the remaining unmerged patches in this
> patchset (patches 1, 2, 4, 5, 7-13).
> 
> - Andreas
> 
Another ping.

- Andreas
  

Andreas Rheinhardt:
> Andreas Rheinhardt:
>> In the common case that the input packet was already refcounted,
>> ff_interleave_add_packet would allocate a new AVPacketList, use
>> av_packet_ref to create a new reference to the buffer for the
>> AVPacketList's packet, interleave the packet and finally unreference
>> the original input packet.
>> This commit changes this: It uses av_packet_move_ref to transfer
>> the packet to its destination. In case the input packet is refcounted,
>> this saves an allocation and a free (of an AVBufferRef); if not, the
>> packet is made refcounted before moving it. When the input packet has
>> side data, one saves even more than one allocation+free.
>>
>> Furthermore, when the packet is in reality an uncoded frame, a hacky
>> ad-hoc variant of av_packet_move_ref has been employed. Not any more.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
>> ---
>>  libavformat/mux.c | 11 ++++-------
>>  1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavformat/mux.c b/libavformat/mux.c
>> index 8ab5ea8c2b..ac370fb24d 100644
>> --- a/libavformat/mux.c
>> +++ b/libavformat/mux.c
>> @@ -930,17 +930,16 @@ int ff_interleave_add_packet(AVFormatContext *s, AVPacket *pkt,
>>      if ((pkt->flags & AV_PKT_FLAG_UNCODED_FRAME)) {
>>          av_assert0(pkt->size == UNCODED_FRAME_PACKET_SIZE);
>>          av_assert0(((AVFrame *)pkt->data)->buf);
>> -        this_pktl->pkt = *pkt;
>> -        pkt->buf = NULL;
>> -        pkt->side_data = NULL;
>> -        pkt->side_data_elems = 0;
>>      } else {
>> -        if ((ret = av_packet_ref(&this_pktl->pkt, pkt)) < 0) {
>> +        if ((ret = av_packet_make_refcounted(pkt)) < 0) {
>>              av_free(this_pktl);
>>              return ret;
>>          }
>>      }
>>  
>> +    av_packet_move_ref(&this_pktl->pkt, pkt);
>> +    pkt = &this_pktl->pkt;
>> +
>>      if (s->streams[pkt->stream_index]->last_in_packet_buffer) {
>>          next_point = &(st->last_in_packet_buffer->next);
>>      } else {
>> @@ -989,8 +988,6 @@ next_non_null:
>>      s->streams[pkt->stream_index]->last_in_packet_buffer =
>>          *next_point                                      = this_pktl;
>>  
>> -    av_packet_unref(pkt);
>> -
>>      return 0;
>>  }
>>  
>>
> Ping for this patch and the remaining unmerged patches in this
> patchset (patches 1, 2, 4, 5, 7-13).
> 
> - Andreas
> 
Another ping.

- Andreas
  

On Tue, Oct 8, 2019 at 12:42 PM Andreas Rheinhardt <
andreas.rheinhardt@gmail.com> wrote:

> Andreas Rheinhardt:
> > Andreas Rheinhardt:
> >> In the common case that the input packet was already refcounted,
> >> ff_interleave_add_packet would allocate a new AVPacketList, use
> >> av_packet_ref to create a new reference to the buffer for the
> >> AVPacketList's packet, interleave the packet and finally unreference
> >> the original input packet.
> >> This commit changes this: It uses av_packet_move_ref to transfer
> >> the packet to its destination. In case the input packet is refcounted,
> >> this saves an allocation and a free (of an AVBufferRef); if not, the
> >> packet is made refcounted before moving it. When the input packet has
> >> side data, one saves even more than one allocation+free.
> >>
> >> Furthermore, when the packet is in reality an uncoded frame, a hacky
> >> ad-hoc variant of av_packet_move_ref has been employed. Not any more.
> >>
> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> >> ---
> >>  libavformat/mux.c | 11 ++++-------
> >>  1 file changed, 4 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/libavformat/mux.c b/libavformat/mux.c
> >> index 8ab5ea8c2b..ac370fb24d 100644
> >> --- a/libavformat/mux.c
> >> +++ b/libavformat/mux.c
> >> @@ -930,17 +930,16 @@ int ff_interleave_add_packet(AVFormatContext *s,
> AVPacket *pkt,
> >>      if ((pkt->flags & AV_PKT_FLAG_UNCODED_FRAME)) {
> >>          av_assert0(pkt->size == UNCODED_FRAME_PACKET_SIZE);
> >>          av_assert0(((AVFrame *)pkt->data)->buf);
> >> -        this_pktl->pkt = *pkt;
> >> -        pkt->buf = NULL;
> >> -        pkt->side_data = NULL;
> >> -        pkt->side_data_elems = 0;
> >>      } else {
> >> -        if ((ret = av_packet_ref(&this_pktl->pkt, pkt)) < 0) {
> >> +        if ((ret = av_packet_make_refcounted(pkt)) < 0) {
> >>              av_free(this_pktl);
> >>              return ret;
> >>          }
> >>      }
> >>
> >> +    av_packet_move_ref(&this_pktl->pkt, pkt);
> >> +    pkt = &this_pktl->pkt;
> >> +
> >>      if (s->streams[pkt->stream_index]->last_in_packet_buffer) {
> >>          next_point = &(st->last_in_packet_buffer->next);
> >>      } else {
> >> @@ -989,8 +988,6 @@ next_non_null:
> >>      s->streams[pkt->stream_index]->last_in_packet_buffer =
> >>          *next_point                                      = this_pktl;
> >>
> >> -    av_packet_unref(pkt);
> >> -
> >>      return 0;
> >>  }
> >>
> >>
> > Ping for this patch and the remaining unmerged patches in this
> > patchset (patches 1, 2, 4, 5, 7-13).
> >
> > - Andreas
> >
> Another ping.
>
> - Andreas
>

Another ping.

- Andreas
  

On Wed, Jan 15, 2020 at 10:41 PM Andreas Rheinhardt <
andreas.rheinhardt@gmail.com> wrote:

> On Tue, Oct 8, 2019 at 12:42 PM Andreas Rheinhardt <
> andreas.rheinhardt@gmail.com> wrote:
>
>> Andreas Rheinhardt:
>> > Andreas Rheinhardt:
>> >> In the common case that the input packet was already refcounted,
>> >> ff_interleave_add_packet would allocate a new AVPacketList, use
>> >> av_packet_ref to create a new reference to the buffer for the
>> >> AVPacketList's packet, interleave the packet and finally unreference
>> >> the original input packet.
>> >> This commit changes this: It uses av_packet_move_ref to transfer
>> >> the packet to its destination. In case the input packet is refcounted,
>> >> this saves an allocation and a free (of an AVBufferRef); if not, the
>> >> packet is made refcounted before moving it. When the input packet has
>> >> side data, one saves even more than one allocation+free.
>> >>
>> >> Furthermore, when the packet is in reality an uncoded frame, a hacky
>> >> ad-hoc variant of av_packet_move_ref has been employed. Not any more.
>> >>
>> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
>> >> ---
>> >>  libavformat/mux.c | 11 ++++-------
>> >>  1 file changed, 4 insertions(+), 7 deletions(-)
>> >>
>> >> diff --git a/libavformat/mux.c b/libavformat/mux.c
>> >> index 8ab5ea8c2b..ac370fb24d 100644
>> >> --- a/libavformat/mux.c
>> >> +++ b/libavformat/mux.c
>> >> @@ -930,17 +930,16 @@ int ff_interleave_add_packet(AVFormatContext *s,
>> AVPacket *pkt,
>> >>      if ((pkt->flags & AV_PKT_FLAG_UNCODED_FRAME)) {
>> >>          av_assert0(pkt->size == UNCODED_FRAME_PACKET_SIZE);
>> >>          av_assert0(((AVFrame *)pkt->data)->buf);
>> >> -        this_pktl->pkt = *pkt;
>> >> -        pkt->buf = NULL;
>> >> -        pkt->side_data = NULL;
>> >> -        pkt->side_data_elems = 0;
>> >>      } else {
>> >> -        if ((ret = av_packet_ref(&this_pktl->pkt, pkt)) < 0) {
>> >> +        if ((ret = av_packet_make_refcounted(pkt)) < 0) {
>> >>              av_free(this_pktl);
>> >>              return ret;
>> >>          }
>> >>      }
>> >>
>> >> +    av_packet_move_ref(&this_pktl->pkt, pkt);
>> >> +    pkt = &this_pktl->pkt;
>> >> +
>> >>      if (s->streams[pkt->stream_index]->last_in_packet_buffer) {
>> >>          next_point = &(st->last_in_packet_buffer->next);
>> >>      } else {
>> >> @@ -989,8 +988,6 @@ next_non_null:
>> >>      s->streams[pkt->stream_index]->last_in_packet_buffer =
>> >>          *next_point                                      = this_pktl;
>> >>
>> >> -    av_packet_unref(pkt);
>> >> -
>> >>      return 0;
>> >>  }
>> >>
>> >>
>> > Ping for this patch and the remaining unmerged patches in this
>> > patchset (patches 1, 2, 4, 5, 7-13).
>> >
>> > - Andreas
>> >
>> Another ping.
>>
>> - Andreas
>>
>
> Another ping.
>
> - Andreas
>
> Ping.

- Andreas
  

On Tue, Aug 13, 2019 at 04:47:14AM +0200, Andreas Rheinhardt wrote:
> In the common case that the input packet was already refcounted,
> ff_interleave_add_packet would allocate a new AVPacketList, use
> av_packet_ref to create a new reference to the buffer for the
> AVPacketList's packet, interleave the packet and finally unreference
> the original input packet.
> This commit changes this: It uses av_packet_move_ref to transfer
> the packet to its destination. In case the input packet is refcounted,
> this saves an allocation and a free (of an AVBufferRef); if not, the
> packet is made refcounted before moving it. When the input packet has
> side data, one saves even more than one allocation+free.
> 
> Furthermore, when the packet is in reality an uncoded frame, a hacky
> ad-hoc variant of av_packet_move_ref has been employed. Not any more.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavformat/mux.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)

will apply

[...]