[FFmpeg-devel] [PATCH 3/3] lavf/movenc: support iTunes cover art

[Michael Niedermayer]

On Wed, Jul 03, 2013 at 12:08:38PM +0200, Matthieu Bouron wrote:
> On Tue, Jul 02, 2013 at 10:43:39PM +0200, Michael Niedermayer wrote:
> > On Tue, Jul 02, 2013 at 09:33:04PM +0200, Matthieu Bouron wrote:
> > > On Tue, Jul 2, 2013 at 7:46 PM, Michael Niedermayer <michaelni at gmx.at>wrote:
> > > 
> > > > On Sun, Jun 30, 2013 at 04:15:46PM +0200, Matthieu Bouron wrote:
> > > > > Cover art muxing is done by introducing the -cover_stream_index option
> > > > > which takes an output stream index as argument.
> > > > > The stream used for the cover art is not muxed as a track in the
> > > > > resulting file.
> > > > > ---
> > > > >  libavformat/movenc.c     | 157
> > > > +++++++++++++++++++++++++++++++++++++++++++----
> > > > >  libavformat/movenc.h     |   5 ++
> > > > >  libavformat/movenchint.c |   1 +
> > > > >  3 files changed, 152 insertions(+), 11 deletions(-)
> > > > >
> > > > > diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> > > > > index 5577530..f411493 100644
> > > > > --- a/libavformat/movenc.c
> > > > > +++ b/libavformat/movenc.c
> > > > > @@ -63,6 +63,7 @@ static const AVOption options[] = {
> > > > >      { "ism_lookahead", "Number of lookahead entries for ISM files",
> > > > offsetof(MOVMuxContext, ism_lookahead), AV_OPT_TYPE_INT, {.i64 = 0}, 0,
> > > > INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
> > > > >      { "use_editlist", "use edit list", offsetof(MOVMuxContext,
> > > > use_editlist), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 1,
> > > > AV_OPT_FLAG_ENCODING_PARAM},
> > > > >      { "video_track_timescale", "set timescale of all video tracks",
> > > > offsetof(MOVMuxContext, video_track_timescale), AV_OPT_TYPE_INT, {.i64 =
> > > > 0}, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
> > > > > +    { "cover_stream_index", "video stream index to use for cover art",
> > > > offsetof(MOVMuxContext, cover_stream_index), AV_OPT_TYPE_INT, {.i64 = -1},
> > > > -1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
> > > > >      { NULL },
> > > >
> > > > isnt AV_DISPOSITION_ATTACHED_PIC enough ?
> > > > why is this option needed ?
> > > >
> > > 
> > > I guess AV_DISPOSITION_ATTACHED_PIC could be enough.
> > > My idea here was to let the user choose the cover art from any output
> > > streams he likes with -cover_stream_index. If the input does not have any
> > > cover arts, can a custom stream be flagged as an attached pic ?
> > 
> > if it cannot then such feature should be added but certainly doesnt
> > belong in a single individual muxer
> 
> You're right, using AV_DISPOSITON_ATTACHED_PIC seems a better solution
> than introducing the cover_stream_index.
> 
> Here is a new patch which uses all stream with AV_DISPOSITON_ATTACHED_PIC
> as cover arts. It works for all modes which use the iTunes metadata (!3gp
> and !mov).
> 
> Actually the cover art codec is only checked at the end (when writing the
> metadata). If not BMP, MJPEG or PNG the user is warned that the cover art
> is ignored.
> 
> Do you think this check should be included in the new mov_stream_is_apic
> function and discard all apic streams which have not the right codec (and
> use them as normal stream ? (IMHO it do not feel right to me to use them
> as normal stream)).

probably not

did you test this with subtitles & chapters ?

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130703/66163222/attachment.asc>