[MPlayer-dev-eng] Re: [PATCH] -identify vids, aids, sids, alangs and slangs

[Moritz Bunkus]

Hey,

> Hmm... I don't think I understand. The only place I found where the
> language is set is in demux_mkv_read_trackentry, which is called from
> demux_mkv_read_tracks. And display_tracks is called only after
> demux_mkv_open is done calling demux_mkv_read_tracks. So why is it
> too early? Did I miss something?

Ups sorry, I mixed up places :) I thought the part I was complaining
about was to be inserted in demux_mkv_read_trackentry. No,
display_tracks is OK of course.

> The only easy way to fix this mess I could think of was to change the
> accepted aids and vids to match the internal ones,

Or fix the demuxers to all use zero-based IDs (which would be MUCH more
consistent than the mess we have now...).

> but I understand you (and Richard) object to that,

Strongly. It's a matter of usability (which mplayer lacks in too many
ways already). Having to call mplayer once just to find out the correct
IDs to use for the first audio track or so is simply not acceptable.

> So the demux_aid_vid_mismatch is the workaround that tells new_sh_*
> not to print the ids it gets, in which case it is the demux module
> that prints them.

In fact this could be changed. For demux_ogg.c you've already introduced
a new member "id" which stores the -Xid ID. How about you extend the
patch so that demux_ogg.c always uses those? :)

> > -alang doesn't work for Ogg (yet), just so you know, but this would be
> > the right place to output it. So let's keep it.
> 
> I understand, but a language name is useful for displaying in a GUI.

Sure. I just wanted to point it out.

> The patch will be one line shorter, but the resulting code one line
> longer. Whatever your preference. :-)

Shorter patches. Makes reviewing easier.

> Another minor problem in demux_ogg is that demux_ogg_check_comments
> only gets called for streams that are actually going to be used, and
> so the other stream languages are not known until they are actually
> played. But I am guessing there is not easy fix for that.

Well, the "best" way IMHO would be to have demux_ogg_open parse ALL
header packets of ALL streams before returning. That way we could also
enable -alang.

> Thanks for looking at the patch.

You're welcome. I'm sorry I cannot really comment on the other
parts. demux_ogg.c and demux_mkv.c are the ones I know best, so I
preferred to talk only about those.

Mosu

-- 
If Darl McBride was in charge, he'd probably make marriage
unconstitutional too, since clearly it de-emphasizes the commercial
nature of normal human interaction, and probably is a major impediment
to the commercial growth of prostitution. - Linus Torvalds