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

[Michael Niedermayer]

On Thu, Nov 23, 2017 at 07:08:42PM -0300, James Almer wrote:
> From: Rodger Combs <rodger.combs at gmail.com>
> 
> Signed-off-by: James Almer <jamrial at 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 ?



[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171125/da287d80/attachment.sig>