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

[Anton Khirnov]

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:
>> >> >
>> >> > [...]
>> >> >> +#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. */
>> >> >
>> >> > is the reader supposed to guess what values this field has or if it
>> >> > maybe takes AV_LINK flags?
>> >> >
>> >>
>> >> fixed =p
>> >>
>> >> >
>> >> > [...]
>> >> >> diff --git a/libavformat/avio.h b/libavformat/avio.h
>> >> >> index 687333e..261fb32 100644
>> >> >> --- a/libavformat/avio.h
>> >> >> +++ b/libavformat/avio.h
>> >> >> @@ -135,6 +135,9 @@ typedef struct URLProtocol {
>> >> >>      int (*url_read_pause)(URLContext *h, int pause);
>> >> >>      int64_t (*url_read_seek)(URLContext *h,
>> >> >>                           int stream_index, int64_t timestamp, int flags);
>> >> >> +    int  (*url_opendir)(URLContext **h, const char *path, int flags);
>> >> >> +    int  (*url_readdir)( URLContext *h, char *filename, int max_size);
>> >> >> +    void (*url_closedir)(URLContext *h);
>> >> >>  } URLProtocol;
>> >> >
>> >> > This stuff belongs in a seperate patch
>> >> >
>> >>
>> >> done
>> >>
>> >> Anton
>> >
>> >> diff --git a/libavformat/avio.h b/libavformat/avio.h
>> >> index 687333e..261fb32 100644
>> >> --- a/libavformat/avio.h
>> >> +++ b/libavformat/avio.h
>> >> @@ -135,6 +135,9 @@ typedef struct URLProtocol {
>> >>      int (*url_read_pause)(URLContext *h, int pause);
>> >>      int64_t (*url_read_seek)(URLContext *h,
>> >>                           int stream_index, int64_t timestamp, int flags);
>> >> +    int  (*url_opendir)(URLContext **h, const char *path, int flags);
>> >> +    int  (*url_readdir)( URLContext *h, char *filename, int max_size);
>> >> +    void (*url_closedir)(URLContext *h);
>> >>  } URLProtocol;
>> >
>> > i was wondering if this is needed, i mean
>> > url_open/url_read could be used too given a special flag during open to
>> > indicate opening a directory.
>> > But maybe this is undesireable, and your patch is better, i dont know
>> > except this i think the directory patch is fine
>> >
>>
>> I'd say that since we treat directories differently from normal files,
>> we should also use different functions.
>
> we treat them differently because the underlaying functions we (have to) use
> treat them differently
>
>
>> Besides i don't see an
>> immediate way to determine if url_read is called for directory or
>> file.
>
> the "int flags"
>
> like
> #define AV_OPEN_DIR 0x1234
> url_open(..., AV_OPEN_DIR);
>

url_*read*

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.

Anton