[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