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

[Ulion]

2008/1/31, Uoti Urpala <uoti.urpala at pp1.inet.fi>:
> 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?

In some frontend, such as MPlayer OSX, when adjust volume, seek in
window mode, it did not want any osd msg or progress bar get
displayed, or when it batch run some commands to apply some
preference, it also not want the osd. Current implement of it use 'osd
0' prefix the command(s) and 'osd 1' after it(them), this will break
the original osd level. Indeed it only want to run some command(s)
without osd, so I write those patches for fixing these code after days
ago Diego reported the osd_level loss problem.

-- 
Ulion