diff mbox

[FFmpeg-devel,1/2] lavf/flacenc: support writing attached pictures

Message ID 20171123220843.804-1-jamrial@gmail.com
State New
Headers show

Commit Message

James Almer Nov. 23, 2017, 10:08 p.m. UTC 
From: Rodger Combs <rodger.combs@gmail.com>

Signed-off-by: James Almer <jamrial@gmail.com>
---
Should be good to commit now.

 libavformat/flacenc.c | 286 +++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 250 insertions(+), 36 deletions(-)

Comments

Carl Eugen Hoyos Nov. 23, 2017, 11:43 p.m. UTC | #1 
2017-11-23 23:08 GMT+01:00 James Almer <jamrial@gmail.com>:

> Should be good to commit now.

Please mention ticket #4442.

Thank you, Carl Eugen
Michael Niedermayer Nov. 24, 2017, 11:27 p.m. UTC | #2 
On Thu, Nov 23, 2017 at 07:08:42PM -0300, James Almer wrote:
> From: Rodger Combs <rodger.combs@gmail.com>
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> Should be good to commit now.
> 
>  libavformat/flacenc.c | 286 +++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 250 insertions(+), 36 deletions(-)
> 
> diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c
> index b894f9ef61..84da54a1df 100644
> --- a/libavformat/flacenc.c
> +++ b/libavformat/flacenc.c
> @@ -21,10 +21,13 @@
>  
>  #include "libavutil/channel_layout.h"
>  #include "libavutil/opt.h"
> +#include "libavutil/pixdesc.h"
>  #include "libavcodec/flac.h"
>  #include "avformat.h"
>  #include "avio_internal.h"
>  #include "flacenc.h"
> +#include "id3v2.h"
> +#include "internal.h"
>  #include "vorbiscomment.h"
>  #include "libavcodec/bytestream.h"
>  
> @@ -33,8 +36,16 @@ typedef struct FlacMuxerContext {
>      const AVClass *class;
>      int write_header;
>  
> +    int audio_stream_idx;
> +    AVPacket *pics;
> +    int nb_pics, waiting_pics;
> +    /* audio packets are queued here until we get all the attached pictures */
> +    AVPacketList *queue, *queue_end;
> +
>      /* updated streaminfo sent by the encoder at the end */
>      uint8_t *streaminfo;
> +
> +    unsigned attached_types;
>  } FlacMuxerContext;
>  
>  static int flac_write_block_padding(AVIOContext *pb, unsigned int n_padding_bytes,
> @@ -74,31 +85,157 @@ static int flac_write_block_comment(AVIOContext *pb, AVDictionary **m,
>      return 0;
>  }
>  
> -static int flac_write_header(struct AVFormatContext *s)
> +static int flac_write_picture(struct AVFormatContext *s, AVPacket *pkt)
>  {
> -    int ret;
> -    int padding = s->metadata_header_padding;
> -    AVCodecParameters *par = s->streams[0]->codecpar;
> -    FlacMuxerContext *c   = s->priv_data;
> -
> -    if (!c->write_header)
> +    FlacMuxerContext *c = s->priv_data;
> +    AVIOContext *pb = s->pb;
> +    const AVPixFmtDescriptor *pixdesc;
> +    const CodecMime *mime = ff_id3v2_mime_tags;
> +    AVDictionaryEntry *e;
> +    const char *mimetype = NULL, *desc = "";
> +    const AVStream *st = s->streams[pkt->stream_index];
> +    int i, mimelen, desclen, type = 0;
> +
> +    if (!pkt->data)
>          return 0;
>  
> -    if (s->nb_streams > 1) {
> -        av_log(s, AV_LOG_ERROR, "only one stream is supported\n");
> +    while (mime->id != AV_CODEC_ID_NONE) {
> +        if (mime->id == st->codecpar->codec_id) {
> +            mimetype = mime->str;
> +            break;
> +        }
> +        mime++;
> +    }

> +    if (!mimetype) {
> +        av_log(s, AV_LOG_ERROR, "No mimetype is known for stream %d, cannot "
> +               "write an attached picture.\n", st->index);
> +        return AVERROR(EINVAL);
> +    }

This should print the name of the codec thats not supported, so the
user knows what is unsupported


[...]
> @@ -166,23 +347,56 @@ 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;
> +    int ret;
>  
> -    /* 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);
> +    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));

This kind of packet buffering code should not be duplicated per
muxer.
Also we already have buffering code that ensures packet interleaving.
can this not be done there ?
or am i missing something ?



[...]
James Almer Nov. 24, 2017, 11:53 p.m. UTC | #3 
On 11/24/2017 8:27 PM, Michael Niedermayer wrote:
> On Thu, Nov 23, 2017 at 07:08:42PM -0300, James Almer wrote:
>> From: Rodger Combs <rodger.combs@gmail.com>
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> Should be good to commit now.
>>
>>  libavformat/flacenc.c | 286 +++++++++++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 250 insertions(+), 36 deletions(-)
>>
>> diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c
>> index b894f9ef61..84da54a1df 100644
>> --- a/libavformat/flacenc.c
>> +++ b/libavformat/flacenc.c
>> @@ -21,10 +21,13 @@
>>  
>>  #include "libavutil/channel_layout.h"
>>  #include "libavutil/opt.h"
>> +#include "libavutil/pixdesc.h"
>>  #include "libavcodec/flac.h"
>>  #include "avformat.h"
>>  #include "avio_internal.h"
>>  #include "flacenc.h"
>> +#include "id3v2.h"
>> +#include "internal.h"
>>  #include "vorbiscomment.h"
>>  #include "libavcodec/bytestream.h"
>>  
>> @@ -33,8 +36,16 @@ typedef struct FlacMuxerContext {
>>      const AVClass *class;
>>      int write_header;
>>  
>> +    int audio_stream_idx;
>> +    AVPacket *pics;
>> +    int nb_pics, waiting_pics;
>> +    /* audio packets are queued here until we get all the attached pictures */
>> +    AVPacketList *queue, *queue_end;
>> +
>>      /* updated streaminfo sent by the encoder at the end */
>>      uint8_t *streaminfo;
>> +
>> +    unsigned attached_types;
>>  } FlacMuxerContext;
>>  
>>  static int flac_write_block_padding(AVIOContext *pb, unsigned int n_padding_bytes,
>> @@ -74,31 +85,157 @@ static int flac_write_block_comment(AVIOContext *pb, AVDictionary **m,
>>      return 0;
>>  }
>>  
>> -static int flac_write_header(struct AVFormatContext *s)
>> +static int flac_write_picture(struct AVFormatContext *s, AVPacket *pkt)
>>  {
>> -    int ret;
>> -    int padding = s->metadata_header_padding;
>> -    AVCodecParameters *par = s->streams[0]->codecpar;
>> -    FlacMuxerContext *c   = s->priv_data;
>> -
>> -    if (!c->write_header)
>> +    FlacMuxerContext *c = s->priv_data;
>> +    AVIOContext *pb = s->pb;
>> +    const AVPixFmtDescriptor *pixdesc;
>> +    const CodecMime *mime = ff_id3v2_mime_tags;
>> +    AVDictionaryEntry *e;
>> +    const char *mimetype = NULL, *desc = "";
>> +    const AVStream *st = s->streams[pkt->stream_index];
>> +    int i, mimelen, desclen, type = 0;
>> +
>> +    if (!pkt->data)
>>          return 0;
>>  
>> -    if (s->nb_streams > 1) {
>> -        av_log(s, AV_LOG_ERROR, "only one stream is supported\n");
>> +    while (mime->id != AV_CODEC_ID_NONE) {
>> +        if (mime->id == st->codecpar->codec_id) {
>> +            mimetype = mime->str;
>> +            break;
>> +        }
>> +        mime++;
>> +    }
> 
>> +    if (!mimetype) {
>> +        av_log(s, AV_LOG_ERROR, "No mimetype is known for stream %d, cannot "
>> +               "write an attached picture.\n", st->index);
>> +        return AVERROR(EINVAL);
>> +    }
> 
> This should print the name of the codec thats not supported, so the
> user knows what is unsupported
> 
> 
> [...]
>> @@ -166,23 +347,56 @@ 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;
>> +    int ret;
>>  
>> -    /* 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);
>> +    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));
> 
> This kind of packet buffering code should not be duplicated per
> muxer.

Having this code in three muxers isn't much of an issue. Also, I've been
waiting on some promised AVPacketList API but it never showed up.

> Also we already have buffering code that ensures packet interleaving.
> can this not be done there ?

What code would this be?

> or am i missing something ?

It's just buffering audio until the first video packet (AKA, cover art)
shows up. It then writes it, dumps all the buffered audio, then keeps
muxing audio normally (no more images are expected after that, and any
that shows up will be ignored).

> 
> 
> 
> [...]
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Michael Niedermayer Nov. 25, 2017, 3:22 a.m. UTC | #4 
On Fri, Nov 24, 2017 at 08:53:55PM -0300, James Almer wrote:
> On 11/24/2017 8:27 PM, Michael Niedermayer wrote:
> > On Thu, Nov 23, 2017 at 07:08:42PM -0300, James Almer wrote:
> >> From: Rodger Combs <rodger.combs@gmail.com>
> >>
> >> Signed-off-by: James Almer <jamrial@gmail.com>
> >> ---
> >> Should be good to commit now.
> >>
> >>  libavformat/flacenc.c | 286 +++++++++++++++++++++++++++++++++++++++++++-------
> >>  1 file changed, 250 insertions(+), 36 deletions(-)
> >>
> >> diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c
> >> index b894f9ef61..84da54a1df 100644
> >> --- a/libavformat/flacenc.c
> >> +++ b/libavformat/flacenc.c
> >> @@ -21,10 +21,13 @@
> >>  
> >>  #include "libavutil/channel_layout.h"
> >>  #include "libavutil/opt.h"
> >> +#include "libavutil/pixdesc.h"
> >>  #include "libavcodec/flac.h"
> >>  #include "avformat.h"
> >>  #include "avio_internal.h"
> >>  #include "flacenc.h"
> >> +#include "id3v2.h"
> >> +#include "internal.h"
> >>  #include "vorbiscomment.h"
> >>  #include "libavcodec/bytestream.h"
> >>  
> >> @@ -33,8 +36,16 @@ typedef struct FlacMuxerContext {
> >>      const AVClass *class;
> >>      int write_header;
> >>  
> >> +    int audio_stream_idx;
> >> +    AVPacket *pics;
> >> +    int nb_pics, waiting_pics;
> >> +    /* audio packets are queued here until we get all the attached pictures */
> >> +    AVPacketList *queue, *queue_end;
> >> +
> >>      /* updated streaminfo sent by the encoder at the end */
> >>      uint8_t *streaminfo;
> >> +
> >> +    unsigned attached_types;
> >>  } FlacMuxerContext;
> >>  
> >>  static int flac_write_block_padding(AVIOContext *pb, unsigned int n_padding_bytes,
> >> @@ -74,31 +85,157 @@ static int flac_write_block_comment(AVIOContext *pb, AVDictionary **m,
> >>      return 0;
> >>  }
> >>  
> >> -static int flac_write_header(struct AVFormatContext *s)
> >> +static int flac_write_picture(struct AVFormatContext *s, AVPacket *pkt)
> >>  {
> >> -    int ret;
> >> -    int padding = s->metadata_header_padding;
> >> -    AVCodecParameters *par = s->streams[0]->codecpar;
> >> -    FlacMuxerContext *c   = s->priv_data;
> >> -
> >> -    if (!c->write_header)
> >> +    FlacMuxerContext *c = s->priv_data;
> >> +    AVIOContext *pb = s->pb;
> >> +    const AVPixFmtDescriptor *pixdesc;
> >> +    const CodecMime *mime = ff_id3v2_mime_tags;
> >> +    AVDictionaryEntry *e;
> >> +    const char *mimetype = NULL, *desc = "";
> >> +    const AVStream *st = s->streams[pkt->stream_index];
> >> +    int i, mimelen, desclen, type = 0;
> >> +
> >> +    if (!pkt->data)
> >>          return 0;
> >>  
> >> -    if (s->nb_streams > 1) {
> >> -        av_log(s, AV_LOG_ERROR, "only one stream is supported\n");
> >> +    while (mime->id != AV_CODEC_ID_NONE) {
> >> +        if (mime->id == st->codecpar->codec_id) {
> >> +            mimetype = mime->str;
> >> +            break;
> >> +        }
> >> +        mime++;
> >> +    }
> > 
> >> +    if (!mimetype) {
> >> +        av_log(s, AV_LOG_ERROR, "No mimetype is known for stream %d, cannot "
> >> +               "write an attached picture.\n", st->index);
> >> +        return AVERROR(EINVAL);
> >> +    }
> > 
> > This should print the name of the codec thats not supported, so the
> > user knows what is unsupported
> > 
> > 
> > [...]
> >> @@ -166,23 +347,56 @@ 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;
> >> +    int ret;
> >>  
> >> -    /* 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);
> >> +    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));
> > 
> > This kind of packet buffering code should not be duplicated per
> > muxer.
> 
> Having this code in three muxers isn't much of an issue. Also, I've been
> waiting on some promised AVPacketList API but it never showed up.
> 
> > Also we already have buffering code that ensures packet interleaving.
> > can this not be done there ?
> 
> What code would this be?

see
interleave_packet()
ff_interleave_packet_per_dts()

I think waiting for the first video packet should be much simpler in
there than in a muxer, its not very different from waiting for a
packet with fitting dts


thanks

[...]
diff mbox

Patch

diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c
index b894f9ef61..84da54a1df 100644
--- a/libavformat/flacenc.c
+++ b/libavformat/flacenc.c
@@ -21,10 +21,13 @@ 
 
 #include "libavutil/channel_layout.h"
 #include "libavutil/opt.h"
+#include "libavutil/pixdesc.h"
 #include "libavcodec/flac.h"
 #include "avformat.h"
 #include "avio_internal.h"
 #include "flacenc.h"
+#include "id3v2.h"
+#include "internal.h"
 #include "vorbiscomment.h"
 #include "libavcodec/bytestream.h"
 
@@ -33,8 +36,16 @@  typedef struct FlacMuxerContext {
     const AVClass *class;
     int write_header;
 
+    int audio_stream_idx;
+    AVPacket *pics;
+    int nb_pics, waiting_pics;
+    /* audio packets are queued here until we get all the attached pictures */
+    AVPacketList *queue, *queue_end;
+
     /* updated streaminfo sent by the encoder at the end */
     uint8_t *streaminfo;
+
+    unsigned attached_types;
 } FlacMuxerContext;
 
 static int flac_write_block_padding(AVIOContext *pb, unsigned int n_padding_bytes,
@@ -74,31 +85,157 @@  static int flac_write_block_comment(AVIOContext *pb, AVDictionary **m,
     return 0;
 }
 
-static int flac_write_header(struct AVFormatContext *s)
+static int flac_write_picture(struct AVFormatContext *s, AVPacket *pkt)
 {
-    int ret;
-    int padding = s->metadata_header_padding;
-    AVCodecParameters *par = s->streams[0]->codecpar;
-    FlacMuxerContext *c   = s->priv_data;
-
-    if (!c->write_header)
+    FlacMuxerContext *c = s->priv_data;
+    AVIOContext *pb = s->pb;
+    const AVPixFmtDescriptor *pixdesc;
+    const CodecMime *mime = ff_id3v2_mime_tags;
+    AVDictionaryEntry *e;
+    const char *mimetype = NULL, *desc = "";
+    const AVStream *st = s->streams[pkt->stream_index];
+    int i, mimelen, desclen, type = 0;
+
+    if (!pkt->data)
         return 0;
 
-    if (s->nb_streams > 1) {
-        av_log(s, AV_LOG_ERROR, "only one stream is supported\n");
+    while (mime->id != AV_CODEC_ID_NONE) {
+        if (mime->id == st->codecpar->codec_id) {
+            mimetype = mime->str;
+            break;
+        }
+        mime++;
+    }
+    if (!mimetype) {
+        av_log(s, AV_LOG_ERROR, "No mimetype is known for stream %d, cannot "
+               "write an attached picture.\n", st->index);
+        return AVERROR(EINVAL);
+    }
+    mimelen = strlen(mimetype);
+
+    /* get the picture type */
+    e = av_dict_get(st->metadata, "comment", NULL, 0);
+    for (i = 0; e && i < FF_ARRAY_ELEMS(ff_id3v2_picture_types); i++) {
+        if (!av_strcasecmp(e->value, ff_id3v2_picture_types[i])) {
+            type = i;
+            break;
+        }
+    }
+
+    if ((c->attached_types & (1 << type)) & 0x6) {
+        av_log(s, AV_LOG_ERROR, "Duplicate attachment for type '%s'\n", ff_id3v2_picture_types[type]);
         return AVERROR(EINVAL);
     }
-    if (par->codec_id != AV_CODEC_ID_FLAC) {
-        av_log(s, AV_LOG_ERROR, "unsupported codec\n");
+
+    if (type == 1 && (st->codecpar->codec_id != AV_CODEC_ID_PNG ||
+                      st->codecpar->width != 32 ||
+                      st->codecpar->height != 32)) {
+        av_log(s, AV_LOG_ERROR, "File icon attachment must be a 32x32 PNG");
         return AVERROR(EINVAL);
     }
 
+    c->attached_types |= (1 << type);
+
+    /* get the description */
+    if ((e = av_dict_get(st->metadata, "title", NULL, 0)))
+        desc = e->value;
+    desclen = strlen(desc);
+
+    avio_w8(pb, 0x06);
+    avio_wb24(pb, 4 + 4 + mimelen + 4 + desclen + 4 + 4 + 4 + 4 + 4 + pkt->size);
+
+    avio_wb32(pb, type);
+
+    avio_wb32(pb, mimelen);
+    avio_write(pb, mimetype, mimelen);
+
+    avio_wb32(pb, desclen);
+    avio_write(pb, desc, desclen);
+
+    avio_wb32(pb, st->codecpar->width);
+    avio_wb32(pb, st->codecpar->height);
+    if ((pixdesc = av_pix_fmt_desc_get(st->codecpar->format)))
+        avio_wb32(pb, av_get_bits_per_pixel(pixdesc));
+    else
+        avio_wb32(pb, 0);
+    avio_wb32(pb, 0);
+
+    avio_wb32(pb, pkt->size);
+    avio_write(pb, pkt->data, pkt->size);
+    return 0;
+}
+
+static int flac_finish_header(struct AVFormatContext *s)
+{
+    FlacMuxerContext *c = s->priv_data;
+    int i, ret, padding = s->metadata_header_padding;
     if (padding < 0)
         padding = 8192;
     /* The FLAC specification states that 24 bits are used to represent the
      * size of a metadata block so we must clip this value to 2^24-1. */
     padding = av_clip_uintp2(padding, 24);
 
+    for (i = 0; i < c->nb_pics; i++) {
+        ret = flac_write_picture(s, &c->pics[i]);
+        av_packet_unref(&c->pics[i]);
+        if (ret)
+            return ret;
+    }
+
+    ret = flac_write_block_comment(s->pb, &s->metadata, !padding,
+                                   s->flags & AVFMT_FLAG_BITEXACT);
+    if (ret)
+        return ret;
+
+    /* The command line flac encoder defaults to placing a seekpoint
+     * every 10s.  So one might add padding to allow that later
+     * but there seems to be no simple way to get the duration here.
+     * So just add the amount requested by the user. */
+    if (padding)
+        flac_write_block_padding(s->pb, padding, 1);
+
+    return 0;
+}
+
+static int flac_write_header(struct AVFormatContext *s)
+{
+    AVCodecParameters *par;
+    FlacMuxerContext *c = s->priv_data;
+    int ret, i;
+
+    c->audio_stream_idx = -1;
+    for (i = 0; i < s->nb_streams; i++) {
+        AVStream *st = s->streams[i];
+        if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {
+            if (c->audio_stream_idx >= 0 || st->codecpar->codec_id != AV_CODEC_ID_FLAC) {
+                av_log(s, AV_LOG_ERROR, "Invalid audio stream. Exactly one FLAC "
+                       "audio stream is required.\n");
+                return AVERROR(EINVAL);
+            }
+            par = s->streams[i]->codecpar;
+            c->audio_stream_idx = i;
+        } else if (st->codecpar->codec_type != AVMEDIA_TYPE_VIDEO) {
+            av_log(s, AV_LOG_ERROR, "Only audio streams and pictures are allowed in FLAC.\n");
+            return AVERROR(EINVAL);
+        } else if (st->codecpar->codec_id == AV_CODEC_ID_GIF) {
+            av_log(s, AV_LOG_ERROR, "GIF image support is not implemented.\n");
+            return AVERROR_PATCHWELCOME;
+        } else if (!c->write_header) {
+            av_log(s, AV_LOG_ERROR, "Can't write attached pictures without a header.\n");
+            return AVERROR(EINVAL);
+        }
+    }
+    if (c->audio_stream_idx < 0) {
+        av_log(s, AV_LOG_ERROR, "No audio stream present.\n");
+        return AVERROR(EINVAL);
+    }
+    c->waiting_pics = c->nb_pics = s->nb_streams - 1;
+    if (c->nb_pics && !(c->pics = av_calloc(c->nb_pics, sizeof(AVPacket))))
+        return AVERROR(ENOMEM);
+
+    if (!c->write_header)
+        return 0;
+
     ret = ff_flac_write_header(s->pb, par->extradata,
                                par->extradata_size, 0);
     if (ret)
@@ -121,18 +258,51 @@  static int flac_write_header(struct AVFormatContext *s)
         }
     }
 
-    ret = flac_write_block_comment(s->pb, &s->metadata, !padding,
-                                   s->flags & AVFMT_FLAG_BITEXACT);
-    if (ret)
-        return ret;
+    if (!c->waiting_pics)
+        ret = flac_finish_header(s);
 
-    /* The command line flac encoder defaults to placing a seekpoint
-     * every 10s.  So one might add padding to allow that later
-     * but there seems to be no simple way to get the duration here.
-     * So just add the amount requested by the user. */
-    if (padding)
-        flac_write_block_padding(s->pb, padding, 1);
+    return ret;
+}
+
+static int flac_write_audio_packet(struct AVFormatContext *s, AVPacket *pkt)
+{
+    FlacMuxerContext *c = s->priv_data;
+    uint8_t *streaminfo;
+    int streaminfo_size;
 
+    /* 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);
+
+        c->streaminfo = av_malloc(FLAC_STREAMINFO_SIZE);
+        if (!c->streaminfo)
+            return AVERROR(ENOMEM);
+        memcpy(c->streaminfo, streaminfo, FLAC_STREAMINFO_SIZE);
+    }
+
+    if (pkt->size)
+        avio_write(s->pb, pkt->data, pkt->size);
+    return 0;
+}
+
+static int flac_queue_flush(AVFormatContext *s)
+{
+    FlacMuxerContext *c = s->priv_data;
+    AVPacketList *pktl;
+    int ret = 0, write = 1;
+
+    flac_finish_header(s);
+
+    while ((pktl = c->queue)) {
+        if (write && (ret = flac_write_audio_packet(s, &pktl->pkt)) < 0)
+            write = 0;
+        av_packet_unref(&pktl->pkt);
+        c->queue = pktl->next;
+        av_freep(&pktl);
+    }
+    c->queue_end = NULL;
     return ret;
 }
 
@@ -142,7 +312,18 @@  static int flac_write_trailer(struct AVFormatContext *s)
     int64_t file_size;
     FlacMuxerContext *c = s->priv_data;
     uint8_t *streaminfo = c->streaminfo ? c->streaminfo :
-                                          s->streams[0]->codecpar->extradata;
+                                          s->streams[c->audio_stream_idx]->codecpar->extradata;
+    int i;
+
+    if (c->waiting_pics) {
+        av_log(s, AV_LOG_WARNING, "No packets were sent for some of the "
+               "attached pictures.\n");
+        flac_queue_flush(s);
+    }
+
+    for (i = 0; i < c->nb_pics; i++)
+        av_packet_unref(&c->pics[i]);
+    av_freep(&c->pics);
 
     if (!c->write_header || !streaminfo)
         return 0;
@@ -166,23 +347,56 @@  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;
+    int ret;
 
-    /* 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);
+    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));
+
+            if (!pktl || av_packet_ref(&pktl->pkt, pkt) < 0) {
+                av_free(pktl);
+                if (s->error_recognition & AV_EF_EXPLODE)
+                    return AVERROR(ENOMEM);
+                av_log(s, AV_LOG_ERROR, "Out of memory in packet queue; skipping attached pictures\n");
+                c->waiting_pics = 0;
+                ret = flac_queue_flush(s);
+                if (ret < 0)
+                    return ret;
+                return flac_write_audio_packet(s, pkt);
+            }
+
+            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 index = pkt->stream_index;
 
-        c->streaminfo = av_malloc(FLAC_STREAMINFO_SIZE);
-        if (!c->streaminfo)
-            return AVERROR(ENOMEM);
-        memcpy(c->streaminfo, streaminfo, FLAC_STREAMINFO_SIZE);
+        /* 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;
+
+        if (index > c->audio_stream_idx)
+            index--;
+
+        if ((ret = av_packet_ref(&c->pics[index], pkt)) < 0)
+            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 +419,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,