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

[Anton Khirnov]

On Sat, Aug 23, 2008 at 9:01 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Sat, Aug 23, 2008 at 05:50:23PM +0200, Anton Khirnov wrote:
>> ping?
>
>
>
>> Index: libavformat/avformat.h
>> ===================================================================
>> --- libavformat/avformat.h    (revision 14918)
>> +++ libavformat/avformat.h    (working copy)
>> @@ -22,7 +22,7 @@
>>  #define FFMPEG_AVFORMAT_H
>>
>>  #define LIBAVFORMAT_VERSION_MAJOR 52
>> -#define LIBAVFORMAT_VERSION_MINOR 20
>> +#define LIBAVFORMAT_VERSION_MINOR 21
>>  #define LIBAVFORMAT_VERSION_MICRO  0
>>
>>  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
>
>> @@ -176,6 +176,8 @@
>>      enum CodecID video_codec_id;
>>      enum CodecID audio_codec_id;
>>  #endif
>> +    /**< Will automatically load linked segments for matroska files.*/
>> +    int matroska_load_linked_segments;
>>  } AVFormatParameters;
>
> AVFormatParameters is deprecated and i do not want to have more stuff added
> to it
>
It's not marked as deprecated (or i'm missing something). Then what
would be the correct place for this?
>
>>
>>  //! demuxer will use url_fopen, no opened file should be provided by the caller
>> @@ -424,12 +426,15 @@
>>
>>  #define AVFMTCTX_NOHEADER      0x0001 /**< signal that no header is present
>>                                           (streams are added dynamically) */
>> +#define AVFMTCTX_VIRTUAL       0x0010 /** this context is an abstraction over
>> +                                          several other contexts */
>>
>
> I think this needs some explanation of
> what it means
> when its used
> why it is needed
>
> besides 0x0010 is not the next available flag
>
i imagine it will work like this:
demuxer autoloads some external streams -> creates a AVFormatContext
marked with this flag, that can be used to access all these streams in
the way they are meant to be played -> add the virtual context and all
the real contexts to external_ctx array in the main AVFormatContext ->
the client app a) select the virtual context and uses it instead of
the main one b) chooses to access these external streams directly c)
ignores it
->
>>  typedef struct AVChapter {
>>      int id;                 ///< Unique id to identify the chapter
>>      AVRational time_base;   ///< Timebase in which the start/end timestamps are specified
>>      int64_t start, end;     ///< chapter start/end time in time_base units
>>      char *title;            ///< chapter title
>> +    void *priv_data;        ///< demuxer-specific data
>>  } AVChapter;
>>
>>  #define MAX_STREAMS 20
>
> Thats a seperate change as well that has little to do with the rest.
>
ok, i'll separate it. if you're asking why do we need  private data
for chapters, it's because segment linking in matroska is done through
chapters.
> Each single API change should be seperately discussed and justified
> Iam not opposed to any of these changes at the current point, iam just
> missing a little bit of explanations why that way and not another and
> if it would be sufficient for other cases like mov, smil, ... as well
>
i'm not familiar with smil or mov way of file linking, but from what i
read about them this should be usable.

Anton