[MPlayer-dev-eng] [PATCH] DVD subpicture/audio stream mapping

Reimar Döffinger Reimar.Doeffinger at stud.uni-karlsruhe.de
Wed Sep 28 10:41:02 CEST 2005


Hi,
On Wed, Sep 28, 2005 at 12:31:36AM +0200, Lehel Bernadt wrote:
> > cosmetics (I think) you only removed the space after subp_attr_t *,
> > right?
> 
> Yes, to fix the style which is "type *pointer" throughout the whole file, 
> except here.

True, but changing it is against our commit rules. I will change it when
applying.

> > Use the old code in the else part (without changing indentation).
> 
> Done.

I'll change it so that this line will not appear in the patch. Makes it
easier to see how things were changed exactly.

> > That doesn't have to be a global variable, does it?
> 
> Not really :) It just ended up here because of the other similar variables.
> I've moved it to the innermost code block.

Well, that maybe be the innermost block common to all of them, but
declaring it each time it is used wouldn't hurt IMHO *g*. I guess I
will change that a bit when applying...

> > actually d_dvdsub->id =
> > ((dvd_priv_t*)stream->priv)->subtitles[dvdsub_id].id;
> > should work as well, although it is ugly.
[...]
> I think we should keep it this way for readability reasons. I already 

Which is more readable depends on what you're used to I guess. So I'll
accept your decision.

Now did somebody test this patch already? If not, please do so.

Greetings,
Reimar Döffinger




More information about the MPlayer-dev-eng mailing list