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

[Aurelien Jacobs]

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.

> +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.

> +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.

Aurel