[FFmpeg-devel] [PATCH v2] avformat/movenc: support writing iTunes cover image

[Timo Teras]

On Tue, 17 Apr 2018 00:01:42 +0100
Rostislav Pehlivanov <atomnuker at gmail.com> wrote:

> On 16 April 2018 at 15:05, Timo Teras <timo.teras at iki.fi> wrote:
> 
> > On Mon, 16 Apr 2018 10:53:43 -0300
> > James Almer <jamrial at gmail.com> wrote:
> >  
> > > On 4/14/2018 3:32 PM, Timo Teräs wrote:  
> > > > This makes movenc handle AV_DISPOSITION_ATTACHED_PIC and write
> > > > the associated pictures in iTunes cover atom. This corresponds
> > > > to how 'mov' demuxer parses and exposes the cover images when
> > > > reading.
> > >
> > > This is failing when i try to mux a jpg as cover art into m4a
> > > (Ipod muxer). It complains about missing codec tag for mjpeg. Is
> > > the covr atom valid for that format?  
> >
> > Yes, it's valid for m4a.
> >  
> > > Adding an entry to codec_ipod_tags[] may fix it, but that alone
> > > would probably then allow non cover art video tracks using that
> > > codec, which i assume is undesirable.  
> >
> > For audio formats, e.g. mp3, query codec returns MKTAG('A', 'P',
> > 'I', 'C') to indicate that the image works only as attached picture.
> >
> > But even for video formats, it would be nice if query codec can
> > properly tell which image formats it does based on disposition. The
> > codec list for cover images is different from generic video streams.
> >
> > Any suggestions how to do that properly?
> 
> What's the problem with doing what mp3 does?

Because mp3 files take only audio. It can give universal reply that the
video codecs are valid for cover images only. Mp4/mov in general takes
also video. avformat_query_codec() or AVOutputFormat.query_codec() (if
we'd define it) does not have stream config info or it's disposition.

Ultimately the problem is that currently codec negotiation assumes the
acceptable codecs are same regardless of disposition. When mp3 started
to support this, a hack was added to query_codec() to return special
code to mean "only as cover art". But in mov/mp4 we cannot do that
because the same video codec can be used for a main stream too.

To me it sounds like we need to introduce new version of
avformat_query_codec() that gets the whole AVStream config, or at least
the stream's disposition information so we can return different codec
matches based on disposition. Or modify AVOutputFormat's codec_tag (or
add something new) that can specify the "attached_pic" video codecs
that get's used properly.

Since the above will likely be ABI change, perhaps the existing patch
can be taken as such, and the above be left a follow up fix. The patch
implements stuff correctly for video enabled mp4/mov, so as it is
now it's a big improvement or works correctly on the mp4/mov file
subtypes it happens to support.

Timo