[FFmpeg-devel] [PATCH] avformat/ttaenc: Don't unnecessarily copy packets

[James Almer]

On 12/3/2019 8:34 AM, Andreas Rheinhardt wrote:
> When the tta muxer writes the actual packet data at the end of the
> muxing process, it repeatedly copies the packets contained in the
> linked list of stored packets to an AVPacket located on the stack via
> ff_packet_list_get(), writes the data and unrefs the packet.
> 
> But when one is willing to traverse and free the linked list oneself,
> one can get rid of the copying.

Why even add a packet list API if we're just going to reimplement it
manually in a module to save the equivalent of one call to
av_packet_move_ref() of negligible performance?
For that matter, the subject makes it sound you're preventing copying
the packet payload rather than the AVPacket struct.

If at some point this API is made public, AVPacketList may become
opaque. I don't think this micro-optimization is worth the trouble.

> This also means that the tta muxer does
> not rely on sizeof(AVPacket) any more.

This is a good idea, but I'd rather instead alloc the buffer packet at
init() and keep the clean loop calling ff_packet_list_get().

> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
>  libavformat/ttaenc.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/libavformat/ttaenc.c b/libavformat/ttaenc.c
> index 73c29ae936..84e3937141 100644
> --- a/libavformat/ttaenc.c
> +++ b/libavformat/ttaenc.c
> @@ -122,13 +122,16 @@ static int tta_write_packet(AVFormatContext *s, AVPacket *pkt)
>  static void tta_queue_flush(AVFormatContext *s)
>  {
>      TTAMuxContext *tta = s->priv_data;
> -    AVPacket pkt;
> -
> -    while (tta->queue) {
> -        ff_packet_list_get(&tta->queue, &tta->queue_end, &pkt);
> -        avio_write(s->pb, pkt.data, pkt.size);
> -        av_packet_unref(&pkt);
> +    AVPacketList *entry = tta->queue;
> +
> +    while (entry) {
> +        AVPacketList *next = entry->next;
> +        avio_write(s->pb, entry->pkt.data, entry->pkt.size);
> +        av_packet_unref(&entry->pkt);
> +        av_free(entry);
> +        entry = next;
>      }
> +    tta->queue = tta->queue_end = NULL;
>  }
>  
>  static int tta_write_trailer(AVFormatContext *s)
>