A memory leak in ffplay

[elite_jwp]

In ffplay.c:637, there is a code block as below:

       do {
            if (d->queue->nb_packets == 0)
                SDL_CondSignal(d->empty_queue_cond);
            if (d->packet_pending) {
                av_packet_move_ref(&pkt, &d->pkt);
                d->packet_pending = 0;
            } else {
                if (packet_queue_get(d->queue, &pkt, 1, &d->pkt_serial) < 0)
                    return -1;
            }
        } while (d->queue->serial != d->pkt_serial);

It seems that av_packet_unref is not done with pkt before doing next loop.

[Carl Eugen Hoyos]

Can you reproduce the issue with current FFmpeg git head?

[Carl Eugen Hoyos]

Should be fixed by Marton Balint in af7ec793d49e260aa4c5595d4947322fc91aa72d

[elite_jwp]

The fixed code still exists a issue:
When using "av_packet_unref" to release memeory, it doesn't take the "flush_pkt" as a exception.

Based on the new fixed code, here gives a patch:

         do {
            if (d->queue->nb_packets == 0)
                 SDL_CondSignal(d->empty_queue_cond);
            if (d->packet_pending) {
                 av_packet_move_ref(&pkt, &d->pkt);
                 d->packet_pending = 0;
             } else {
                 if (packet_queue_get(d->queue, &pkt, 1, &d->pkt_serial) < 0)
                    return -1;
             }
             if (d->queue->serial == d->pkt_serial)
                break;
             if(pkt->data != flush_pkt.data)
                av_packet_unref(&pkt);
         } while (1);

Is it ok?

[mkver]

av_packet_unref(&pkt) does not directly free pkt.data (pkt is not a pointer here, hence pkt->data would not even compile); instead it just unreferences the AVBufferRef pkt.buf (and the packet's side-data) and only if the reference counter of the underlying AVBuffer is zero will the actual resource owned by the AVBuffer be freed. But flush_pkt.buf is NULL, i.e. there is no underlying AVBuffer. The data field is used here only to say "I am a flush packet". Nothing will be freed in case pkt.data == flush_pkt.data.

[elite_jwp]

I got it! Thanks!