[FFmpeg-devel] [PATCH 5/7] lavf/flacenc: support writing attached pictures

[James Almer]

On 8/2/2017 3:30 AM, Rodger Combs wrote:
> ---
>  libavformat/flacenc.c | 285 +++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 250 insertions(+), 35 deletions(-)
> 
> diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c
> index b894f9e..9768b6a 100644

[...]

> @@ -166,23 +341,63 @@ static int flac_write_trailer(struct AVFormatContext *s)
>  static int flac_write_packet(struct AVFormatContext *s, AVPacket *pkt)
>  {
>      FlacMuxerContext *c = s->priv_data;
> -    uint8_t *streaminfo;
> -    int streaminfo_size;
> +    if (pkt->stream_index == c->audio_stream_idx) {
> +        if (c->waiting_pics) {
> +            /* buffer audio packets until we get all the pictures */
> +            AVPacketList *pktl = av_mallocz(sizeof(*pktl));
> +            int ret;
> +            if (!pktl) {
> +                ret = AVERROR(ENOMEM);
> +oom:
> +                if (s->error_recognition & AV_EF_EXPLODE) {
> +                    av_free(pktl);
> +                    return ret;
> +                }
> +                av_log(s, AV_LOG_ERROR, "Out of memory in packet queue; skipping attached pictures\n");
> +                c->waiting_pics = 0;
> +                if ((ret = flac_queue_flush(s)) < 0)
> +                    return ret;
> +                return flac_write_audio_packet(s, pkt);
> +            }
> +
> +            ret = av_packet_ref(&pktl->pkt, pkt);
> +            if (ret < 0) {
> +                av_freep(&pktl);
> +                goto oom;
> +            }
> +
> +            if (c->queue_end)
> +                c->queue_end->next = pktl;
> +            else
> +                c->queue = pktl;
> +            c->queue_end = pktl;
> +        } else {
> +            return flac_write_audio_packet(s, pkt);
> +        }
> +    } else {
> +        int ret, index = pkt->stream_index;
>  
> -    /* check for updated streaminfo */
> -    streaminfo = av_packet_get_side_data(pkt, AV_PKT_DATA_NEW_EXTRADATA,
> -                                         &streaminfo_size);
> -    if (streaminfo && streaminfo_size == FLAC_STREAMINFO_SIZE) {
> -        av_freep(&c->streaminfo);
> +        /* warn only once for each stream */
> +        if (s->streams[pkt->stream_index]->nb_frames == 1) {
> +            av_log(s, AV_LOG_WARNING, "Got more than one picture in stream %d,"
> +                   " ignoring.\n", pkt->stream_index);
> +        }
> +        if (!c->waiting_pics || s->streams[pkt->stream_index]->nb_frames >= 1)
> +            return 0;
>  
> -        c->streaminfo = av_malloc(FLAC_STREAMINFO_SIZE);
> -        if (!c->streaminfo)
> -            return AVERROR(ENOMEM);
> -        memcpy(c->streaminfo, streaminfo, FLAC_STREAMINFO_SIZE);
> +        if (index > c->audio_stream_idx)
> +            index--;
> +
> +        if ((ret = av_copy_packet(&c->pics[index], pkt)) < 0)

Shouldn't this be av_packet_ref()?
And they should probably be unreferenced after being consumed in
flac_finish_header(), much like the audio packets are in flac_queue_flush().

Also, you don't seem to be freeing c->pics anywhere.

> +            return ret;
> +        c->waiting_pics--;
> +
> +        /* flush the buffered audio packets */
> +        if (!c->waiting_pics &&
> +            (ret = flac_queue_flush(s)) < 0)
> +            return ret;
>      }
>  
> -    if (pkt->size)
> -        avio_write(s->pb, pkt->data, pkt->size);
>      return 0;
>  }
>  
> @@ -205,7 +420,7 @@ AVOutputFormat ff_flac_muxer = {
>      .mime_type         = "audio/x-flac",
>      .extensions        = "flac",
>      .audio_codec       = AV_CODEC_ID_FLAC,
> -    .video_codec       = AV_CODEC_ID_NONE,
> +    .video_codec       = AV_CODEC_ID_PNG,
>      .write_header      = flac_write_header,
>      .write_packet      = flac_write_packet,
>      .write_trailer     = flac_write_trailer,
>