[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