[FFmpeg-devel] [PATCH] support for ordered chapters/segment linking in Matroska

[Michael Niedermayer]

On Sat, Nov 29, 2008 at 01:08:13PM +0100, Anton Khirnov wrote:
> On Fri, Nov 28, 2008 at 9:25 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Fri, Nov 28, 2008 at 08:14:31PM +0100, Anton Khirnov wrote:
> >> On Fri, Nov 28, 2008 at 5:10 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >> > On Fri, Nov 28, 2008 at 04:08:08PM +0100, Anton Khirnov wrote:
> >> >> On Thu, Nov 27, 2008 at 11:49 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >> >> > On Fri, Nov 21, 2008 at 11:13:30PM +0100, Anton Khirnov wrote:
> >> >> >> On Fri, Nov 21, 2008 at 9:03 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
[...]
> >
> >>
> >> or am i missing something?
> >>
> >> >
> >> >>
> >> >> >
> >> >> >> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> >> >> >> index acdcec4..e85b49d 100644
> >> >> >> --- a/libavformat/avformat.h
> >> >> >> +++ b/libavformat/avformat.h
> >> >> >> @@ -22,8 +22,8 @@
> >> >> >>  #define AVFORMAT_AVFORMAT_H
> >> >> >>
> >> >> >>  #define LIBAVFORMAT_VERSION_MAJOR 52
> >> >> >> -#define LIBAVFORMAT_VERSION_MINOR 23
> >> >> >> -#define LIBAVFORMAT_VERSION_MICRO  1
> >> >> >> +#define LIBAVFORMAT_VERSION_MINOR 24
> >> >> >> +#define LIBAVFORMAT_VERSION_MICRO  0
> >> >> >>
> >> >> >>  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
> >> >> >>                                                 LIBAVFORMAT_VERSION_MINOR, \
> >> >> >> @@ -464,6 +464,29 @@ typedef struct AVChapter {
> >> >> >>
> >> >> >>  #define MAX_STREAMS 20
> >> >> >>
> >> >> >> +#define AV_LINK_FILENAME  0x0001  ///< exact paths to external streams are known
> >> >> >> +#define AV_LINK_SAMEDIR   0x0002  ///< search for linked files in the same directory
> >> >> >> +
> >> >> >> +typedef struct AVLinkContext {
> >> >> >> +    int search_flags;       /**< Where should we look for external streams. See AV_LINK_* */
> >> >> >> +    char **filenames;       /**< Paths to external streams, if known. */
> >> >> >> +    unsigned int nb_filenames;
> >> >> >> +    char *extensions;      /**< Extensions we are interested in, comma-separated. */
> >> >> >> +    struct AVFormatContext **external_ctx; /** Handles of external streams. */
> >> >> >> +    unsigned int nb_external_ctx;
> >> >> >> +    /**
> >> >> >> +     * Check if this stream is needed.
> >> >> >> +     * First parameter is the main context, second is
> >> >> >> +     * external stream.
> >> >> >> +     */
> >> >> >> +    int (*check_external_stream)(struct AVFormatContext *, struct AVFormatContext *);
> >> >> >> +    /**
> >> >> >> +     * Prepare the main context for using external
> >> >> >> +     * streams.
> >> >> >> +     */
> >> >> >> +    int (*setup_context)(struct AVFormatContext *);
> >> >> >> +} AVLinkContext;
> >> >> >> +
> >> >> >>  /**
> >> >> >>   * Format I/O context.
> >> >> >>   * New fields can be added to the end with minor version bumps.
> >> >> >
> >> >> > Why this complexity?
> >> >> >
> >> >> > I mean what is the problem with a demuxer calling a function that then
> >> >> > creates more AVFormatContext/... for the external streams and adds them
> >> >> > to some list in the main one?
> >> >> >
> >> >>
> >> >> I'm not sure what do you mean, some pingpong between demuxer and
> >> >> utils.c is unavoidable because the demuxer doesn's always know the
> >> >> filenames it'll need and utils.c can't do checking by itself.
> >> >
> >> > i cant follow your logic.
> >> > * there is a way to decide which file to open or the file could not
> >> >  be opened.
> >> > * above way can be implemented in a function be that in the demuxer or
> >> >  utils.c
> >> > * this function can be called to find the file, next it can be opened
> >> >  the contexts and streams be created ...
> >> >
> >> > at no point does a AVLinkContext with callbacks become needed thus my
> >> > question again, what is this good for?
> >> >
> >> > If matroska uses some silly system to guess which file to open, its a
> >> > matter of matroska calling a
> >> > give_me_a_list_of_file_names()
> >> > url_open(the_name_i_picked)
> >> >
> >>
> >> each matroska file contains a UID and these UIDs are used for
> >> specifying linking betweeen files. So we open all mkv files in the
> >> same directory and search for these UIDs. This can't be done in
> >> demuxer-independent way.
> >
> > true, but i cannot see how this is related to the complex code you propose
> > the demuxer can easily just get a directory listing and check files until
> > it found all it needs. That is once the URLProtocol supports listing
> > directorie contents.
> > My question is what the "pingpong" callback stuff is good for? Maybe it is
> > usefull for something but i cant see what this somehing might be...
> >
> 
> the idea was to do as much as possible in utils.c to prevent code
> duplication and allow easy adding of other ways to locate needed
> files. the demuxer should do only what's absolutely necessary, i.e.
> checking files and modifying avformatcontext.

what other ways?
what code duplication?! this is matroska specific
iam against putting matroska "search the file with ID x" code in utils.c

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- 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/20081201/a84cdb20/attachment.pgp>