[FFmpeg-devel] [PATCH] attachments support in matroska demuxer
Michael Niedermayer
michaelni
Sun Jan 20 03:42:09 CET 2008
On Sun, Jan 20, 2008 at 03:16:28AM +0100, Aurelien Jacobs wrote:
> On Sun, 20 Jan 2008 02:56:20 +0100
> Michael Niedermayer <michaelni at gmx.at> wrote:
>
> > On Thu, Jan 17, 2008 at 01:00:26AM +0100, Aurelien Jacobs wrote:
> > > Evgeniy Stepanov wrote:
> > >
> > > > On Thursday 03 January 2008 04:23:09 Michael Niedermayer wrote:
> > > > > On Thu, Jan 03, 2008 at 01:38:39AM +0100, Aurelien Jacobs wrote:
> > > > > > On Sat, 29 Dec 2007 22:38:24 +0300
> > > > > > Evgeniy Stepanov <eugeni.stepanov at gmail.com> wrote:
> > > > >
> > > > > [...]
> > > > >
> > > > > > Conclusions:
> > > > > > - Attachment really looks like a stream itself. You can select
> > > > > > the one you want to remux. For example the film poster would
> > > > > > just be a jpeg stream with a single picture. The specificity
> > > > > > of those streams would be:
> > > > > >
> > > > > > * contain one single packet
> > > > > > * not pts
> > > > >
> > > > > Contain no packets, just one global extradata, this was proposed
> > > > > already. If you use normal packets there will be problems with them
> > > > > disapearing if a stream is split timewise or if you seek before
> > > > > demuxing the first packet.
> > > > > Iam not completely happy with this use of streams but we will need
> > > > > some solution and maybe its the least ugly, we will see ...
> > > > >
> > > > > at least iam pretty sure extradata to be a better choice than 1
> > > > > normal packet
> > > > >
> > > > > > * has a filename (important so that an ASS decoder can
> > > > > > select the proper ttf based on it's name)
> > > > > >
> > > > > > * always demuxed before other (normal) streams (IIRC)
> > > > >
> > > > > extradata should be available before packets ...
> > > > >
> > > > > > It seems to me that attachment would fit pretty well, each one
> > > > > > in its own AVStream. Does this sound reasonable ?
> > > > > > - Here is how remuxing to other formats (avi, mov...) could work:
> > > > > > * supported codec (jpeg poster) could be muxed as a normal
> > > > > > JPEG stream with only one picture
> > > > > > * ttf streams should be written in a separate file (that's
> > > > > > the way various players expect to find ttf fonts when
> > > > > > demuxing AVI, IIRC)
> > > > > >
> > > > > > - I think the only required API modifications to support this
> > > > > > would be:
> > > > > > * adding AVStream.filename and maybe
> > > > > > AVStream.id_of_link_stream.
> > > > > > * adding CODEC_ID_TTF ? (ugly, but shouldn't cause much
> > > > > > trouble)
> > > > > > * adding a way to mux ttf font in a separate file (maybe use
> > > > > > a separate muxer for this, ie. call 2 different muxers from
> > > > > > ffmpeg. seems ugly, but I've no better idea right now).
> > > > >
> > > > > Well, there are a few more things i think
> > > > > We need name / (mime) type to remux attachments
> > > > > (CD Cover / ? / tiff)
> > > > > (Fan art / image/x-photoshop / psd)
> > > > > There also could be other things like Author, ...
> > > > >
> > > > > Also we need a way to get the attachments to the decoders, yes we
> > > > > dont have a ASS/SSA decoder but lets assume we had.
> > > > >
> > > > > And while we can just store the streams in avi/mov, where does the
> > > > > filename/name/type go? if its dropped you could end up with a huge
> > > > > number of images and not know what each is.
> > > > >
> > > > > > What do you think about this proposal ? Does it sounds like a
> > > > > > reasonable base ?
> > > > > >
> > > > > > If it's not, the only viable alternative seems to be
> > > > > > AVFormatContext.attachments just like in the original patch,
> > > > > > but with some additionnal code to allow "remuxing", etc...
> > > > >
> > > > > yes, these seem to be the 2 options we have
> > > >
> > > > This patch implements attachments as separate streams with extradata.
> > >
> > > OK. Here are things issues in this patch:
> > > - CODEC_TYPE_NB must stay at the end of the enum.
> > > - Adding attachments to AVFormatContext is probably a leftover from
> > > your previous patch.
> > > - I don't like the fake MATROSKA_TRACK_TYPE_ATTACHMENT and everything
> > > depending on it.
> > > - I don't think CODEC_ID_ATTACHMENT is of any use if it's used for
> > > every kind of attachement. CODEC_TYPE_ATTACHMENT already gives the
> > > same information.
> > >
> > > I fixed all those issues in the attached patch.
> > >
> > > Moreover, I tried to associate a specific CODEC_ID_* to different kind
> > > of attachements. To me, this seems like a good idea, but I would like
> > > to know what others think about it ?
> > >
> > [...]
> >
> > > Index: libavformat/avformat.h
> > > ===================================================================
> > > --- libavformat/avformat.h (revision 11544)
> > > +++ libavformat/avformat.h (working copy)
> > > @@ -21,8 +21,8 @@
> > > #ifndef FFMPEG_AVFORMAT_H
> > > #define FFMPEG_AVFORMAT_H
> > >
> > > -#define LIBAVFORMAT_VERSION_INT ((52<<16)+(4<<8)+0)
> > > -#define LIBAVFORMAT_VERSION 52.4.0
> > > +#define LIBAVFORMAT_VERSION_INT ((52<<16)+(5<<8)+0)
> > > +#define LIBAVFORMAT_VERSION 52.5.0
> > > #define LIBAVFORMAT_BUILD LIBAVFORMAT_VERSION_INT
> > >
> > > #define LIBAVFORMAT_IDENT "Lavf" AV_STRINGIFY(LIBAVFORMAT_VERSION)
> > > @@ -347,6 +347,9 @@
> > >
> > > #define MAX_REORDER_DELAY 4
> > > int64_t pts_buffer[MAX_REORDER_DELAY+1];
> > > +
> > > + char *filename;
> >
> > > + char *type;
> >
> > iam against some undocumented char *type
> > please document precissely what it repressents and how it differs from
> > codec_tag, stream_codec_tag and codec_id
> > or even better get rid of it and use codec_tag or explain why the type
> > here should be special cased relative to these funny codec id strings in
> > matroska
>
> Currently, *type stores the standard mime type of the attachment.
so it idetifies what the attachment is ...
thats what codec_tag does alraedy ...
> It is only useful to stream-copy from mkv to mkv, preserving attachments
> which have a mime-type not listed in ff_mkv_mime_tags[].
> So I guess it's not really useful as long as ff_mkv_mime_tags[] is
> reasonably complete (which it is not, right now, but I will improve it).
> I will remove it for now.
>
> > filename should be documented as well, and in stream type independant way like
> > the "source filename" of the stream or so
>
> Sure. I will add doxy comments in next version of this patch (probably
> tomorrow).
>
> Aurel
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> http://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080120/8c959fa7/attachment.pgp>
More information about the ffmpeg-devel
mailing list