[MPlayer-dev-eng] [PATCH] support run command with specific osd_level

[Uoti Urpala]

On Thu, 2008-01-31 at 22:53 +0800, Ulion wrote:
> 2008/1/31, Uoti Urpala <uoti.urpala at pp1.inet.fi>:
> > The new status variables should be mpctx fields, not globals.
> >
> > -            if (osd_level == 3)
> > +            if (osd_level == 3 ||
> > +                    (mpctx->osd_show_percentage && force_seek_osd_level >= 3))
> >
> > Code like this assumes a particular implementation of other OSD
> > functionality (that nothing but seeks sets osd_show_percentage) and
> > makes it harder to change. In general this makes OSD handling more
> > complex. I think it at least needs a good justification. What kind of
> > use cases do you have for this functionality?
> 
> osd_level is global, so I make force_osd_level and
> force_seek_osd_level be global. then should all of them be made as
> member of mpctx?

The reason I didn't move osd_level to mpctx already is that the GUI code
references it. I think fixing such issues in the GUI would not be a good
use of time (unless someone turns up to do a big rewrite of the GUI code
- which doesn't seem to be happening - it'll probably have to be dropped
at some point in favor of external frontends). On the other hand it
wasn't worth it to let the GUI break over a minor improvement like this
alone so I've left it for later.

> mpctx->osd_show_percentage is only used by SEEK command in command.c,
> this code is only used for display percentage for 1 second after a
> seek. If some code want to use mpctx->osd_show_percentage in future
> for non-seek function, it should settle problems such as
> justification. From where code you referred, can not judge who at
> where set the mpctx->osd_show_percentage, so any better idea?

Maybe you misunderstood what I meant. I didn't mean justifying that
specific line. Rather I meant that because the patch has such changes
which make the code more complex and tricky (a negative thing) the
usefulness of the functionality added by the patch needs to be
justified. What kind of use cases do you have in mind for the patch?