[FFmpeg-devel] [PATCH v2] avformat/movenc: properly handle cover image codecs

[Timo Teras]

On Sat, 2 Jun 2018 00:46:30 +0200
Michael Niedermayer <michael at niedermayer.cc> wrote:

> On Fri, Jun 01, 2018 at 02:24:39AM +0300, Timo Teräs wrote:
> > Find codec tag for attached images using appropriate list of
> > supported image formats.
> > 
> > This fixes writing the cover image to m4v/m4a and other container
> > formats that do not allow these codecs as a track.
> > 
> > Signed-off-by: Timo Teräs <timo.teras at iki.fi>
> > ---
> > This replaces the previous patch:
> >  PATCH] [RFC] avformat/movenc: support covert images for ipod muxer
> > 
> > It appears that query_codec() is used only to figure out which
> > stream to select as the default copy stream. Fixing it properly for
> > movenc attached pictures is difficult, as the stream disposition
> > would need to be passed through the call stack. Additionally, it
> > would make sense to rewrite the selection logic open_output_file()
> > to not use APIC special tag and be based on the input/output
> > dispositions.
> > 
> > So this just adds proper handling of the codecs in mov_init() phase
> > which fixes most of the usability issues.
> > 
> > Please backport to 4.0 stable branch too.
> > 
> > Changes in v2:
> >  - map the codec to the actual covr image tag value and
> >    use that when writing the cover image instead of doing
> >    a new switch over the codec
> > 
> >  libavformat/movenc.c | 30 ++++++++++++------------------
> >  1 file changed, 12 insertions(+), 18 deletions(-)  
> 
> this seems to break:
> ./ffmpeg -i GF9720Repeal20the20Eighth20with20Helen20Linehan.m4a -t 1
> test.mov
> 
> [mov @ 0x2f45140] Could not find tag for codec h264 in stream #0,
> codec not currently supported in container Could not write header for
> output file #0 (incorrect codec parameters ?): Invalid argument Error
> initializing output stream 0:1 -- 
> 
> input can be found here:
> https://guiltyfeminist.libsyn.com/97-repeal-the-eighth-with-helen-linehan

This has been broken all time, I think. It'll work correct with the
patch if you add "-c copy" to preserve the cover image codecs.

Before the cover image patch (before 4.0 release), it would just
incorrectly convert the first cover image to a video track in the .mov.

Without this patch (the current 4.0 and git head) will silently drop
the cover image which got converted to h264 (default codec of the
container).

I wonder what the right thing to do would be?
a) not auto-map any attached_pic video streams
b) make ffmpeg by default just copy (not transcode) attached_pic
c) allow AVOutputFormat to communicate default codec based on
   disposition
d) have movenc ignore/warn about cover images in incorrect format
e) something else?

Ideas?

Timo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180602/3e946f18/attachment.sig>