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

[Timo Teras]

Hi

On Sat, 31 Mar 2018 02:13:14 +0200
Michael Niedermayer <michael at niedermayer.cc> wrote:

> On Thu, Mar 29, 2018 at 09:45:13AM +0300, Timo Teräs wrote:
> > Fixes https://trac.ffmpeg.org/ticket/2798
> > 
> > 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.
> [...]
> 
> > @@ -5480,6 +5529,24 @@ static int mov_write_packet(AVFormatContext
> > *s, AVPacket *pkt) if (!pkt) {
> >          mov_flush_fragment(s, 1);
> >          return 1;
> > +    } if (s->streams[pkt->stream_index]->disposition &
> > AV_DISPOSITION_ATTACHED_PIC) {  
> 
> The if() should be in the next line or it should be a else if
> it looks like someone forgot a "else" even though it makes no
> difference here.

Thanks. Will fix this, and have few other minor cleanups too. Will
resend, but I have additional questions/notes too:

I think I will audit all for (...; ... < mov->nb_streams; ...) loops
to see if they need to skip these attachments. In most cases it's
implicitly done because they don't get track->entry incremented now.
However, there might be few off especially with empty_moov flag.

Wondering also if all those "track->entry <= 0" checks should be made
to static inline function mov_track_has_data() or similar?

Any comments if the patch should be split e.g. adding the attached_pic
flag on it's own?

Or if the code in mov_write_packet() to stuff the received data to the
buffers should be in generic code, or made a helper function?

And finally, should the codec tag/type negotiation code somehow be
updated to recognize attachment_pic and accept only the valid three
image formats supported for now? If yes, how to do it?

Thanks,
Timo