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

[Anton Khirnov]

On Mon, Dec 1, 2008 at 5:46 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> 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
>

Matroska specs don't say a thing about where should linked files be
located (yes I know it's another proof that matroska sucks ;), my
patch is searching for them in the same dir because that's what
Haali's splitter does. But alternatively we could scan $PWD, or use a
user-supplied list of files/dirs or whatever weird way that's needed
for other formats. So we have a av_load_external_streams function in
utils.c that tries to open all candidate files based on hints from the
demuxer and then passes their AVFormatContexts to a demuxer-specific
check. Now if you decided to add file linking to nut for example, it
would require less work.

Anton