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

Michael Niedermayer michaelni
Fri Nov 21 21:03:17 CET 2008


On Sat, Nov 15, 2008 at 03:47:10PM +0100, Anton Khirnov wrote:
> On Mon, Oct 27, 2008 at 4:33 PM, Aurelien Jacobs <aurel at gnuage.org> wrote:
> > On Fri, 17 Oct 2008 22:57:54 +0200
> > "Anton Khirnov" <wyskas at gmail.com> wrote:
> >
> >> ping - a reworked version.
> >
> > Only a quick review for now.
> >
> >> [...]
> >> +static int av_load_external_streams(AVFormatContext *s)
> >> +{
> >> [...]
> >> +        if(!(dir = opendir(dirname)))
> >> +            av_log(s, AV_LOG_ERROR, "Error opening directory %s\n.",
> >> dirname);
> >> +        else {
> >> +            av_log(s, AV_LOG_DEBUG, "Looking for linked files in
> >> %s.\n", dirname);
> >> +            while((file = readdir(dir))) {
> >
> > Using opendir() / readdir() directly here is ugly and won't ever
> > work for non-local files.
> > Another idea would be to add some kind of opendir/readdir operations
> > in avio, working with URLContext. Those operations would only need
> > to be implemented for local files for now, but could also be
> > implemented for http or any other kind of protocol in the future.
> >
> 
> done
> 
> >
> >> +static int
> >> +matroska_check_linked_tracks(MatroskaTrack *track, MatroskaTrack *track2)
> >> +{
> >> +    if(strcmp(track->codec_id, track2->codec_id))
> >> +        return -1;
> >> +    if(track->type == MATROSKA_TRACK_TYPE_VIDEO) {
> >> +        if(track->video.pixel_width  != track2->video.pixel_width  ||
> >> +           track->video.pixel_height != track2->video.pixel_height ||
> >> +           track->codec_priv.size    != track2->codec_priv.size)
> >> +            return -1;
> >> +    }
> >> +    else if(track->type == MATROSKA_TRACK_TYPE_AUDIO) {
> >> +        if(track->audio.channels       != track2->audio.channels ||
> >> +           track->audio.out_samplerate != track2->audio.out_samplerate)
> >> +            return -1;
> >> +        if(strstr(track->codec_id, "A_AAC")    ||
> >> +           strcmp(track->codec_id, "A_AC3")    ||
> >> +           strcmp(track->codec_id, "A_VORBIS") ||
> >> +           strcmp(track->codec_id, "A_DTS")    ||
> >> +           strstr(track->codec_id, "A_MPEG"))
> >> +            if(track->audio.bitdepth != track2->audio.bitdepth)
> >> +                return -1;
> >> +        if(strstr(track->codec_id, "A_AAC"))
> >> +            if(matroska_aac_profile(track->codec_id) != matroska_aac_profile(track2->codec_id))
> >> +                return -1;
> >> +        if(!strcmp(track->codec_id, "A_FLAC") &&
> >> +           memcmp(track->codec_priv.data, track2->codec_priv.data, track->codec_priv.size))
> >> +            return -1;
> >> +        if(!strcmp(track->codec_id, "A_VORBIS")) {
> >> +            uint8_t *header_start[3], *header_start2[3];
> >> +            int header_len[3], header_len2[3];
> >> +
> >> +            if(ff_split_xiph_headers(track->codec_priv.data, track->codec_priv.size,
> >> +                                     30, header_start, header_len) < 0 ||
> >> +               ff_split_xiph_headers(track2->codec_priv.data, track2->codec_priv.size,
> >> +                                     30, header_start2, header_len2) < 0)
> >> +                return -1;
> >> +            if(header_len[0] != header_len2[0] || header_len[2] != header_len2[2])
> >> +                return -1;
> >> +            if(memcmp(header_start[0], header_start2[0], header_len[0]) ||
> >> +               memcmp(header_start[2], header_start2[2], header_len[2]))
> >> +                return -1;
> >> +        }
> >> +    }
> >> +    return 0;
> >> +}
> >
> > I guess this stream compatibility check could be done in a non-matroska
> > specific way, and be moved as a generic function in utils.c.
> >
> 
> I'm pretty sure it could be done, but unfortunately my knowledge about
> codecs is close to none, so i wouldn't know how to do it :) this check
> is essentially a rewrite of what mkvmerge does and i suspect it's
> greatly simplified, so it wouldn't pass as a generic function.
> 
> >
> >> +AVInputFormat matroska_demuxer_ordered = {
> >> +    "matroska",
> >> +    NULL_IF_CONFIG_SMALL("Matroska file format"),
> >> +    sizeof(MatroskaDemuxContext),
> >> +    matroska_probe,
> >> +    matroska_read_header,
> >> +    matroska_read_packet_ordered,
> >> +    matroska_read_close,
> >> +    matroska_read_seek_ordered,
> >> +};
> >
> > I think you could avoid this new AVInputFormat entirely.
> > You could have only one set of read_packet/read_seek functions
> > which would work in both linked/non-linked cases.
> >
> done
> 
> Anton

[...]
> +#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?


[...]
> 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

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

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- 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/20081121/f1fdd693/attachment.pgp>



More information about the ffmpeg-devel mailing list