[MPlayer-dev-eng] [PATCH] add -name and -title options for x video_outs

[Paul TT]

On Thu, 22 Oct 2009 08:37:20 +0200
Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:


> > +char *vo_winname;
> > +char *vo_wintitle;
> 
> If it's called vo_winname and thus for all vos it should be in
> video_out.c, not somewhere x11-specific.
see below :)

> > -    wmClass.res_name = name;
> > +    if (vo_winname != NULL) 
> > +        wmClass.res_name = vo_winname;
> > +    else
> > +        wmClass.res_name = name;
> 
> Use ?:
changing it could be useful for window identification.
man xter
/-name
for example
with the effort to implement title, name could be useful too.

> > +/**
> > + * \brief set the X window title
> > + * \param title title for the window
> > + *
> > + */
> > +void vo_x11_settitle(char *title)
> > +{
> > +    if(title != NULL)
> > +        vo_wintitle = title;
> > +}
> [...]
> > +    vo_x11_settitle(vo_wintitle);
> 
> That really makes no sense whatsoever.
> Also the doxy is nonsense, vo_x11_settitle only
> sets vo_wintitle, it does not set the title
> of an existing window.
mhhhhh iep.
probably the doxy is superfluous indeed and moving out from x11 code
that one function ist name and meaning indeed varies

> > ...
> > +#ifdef CONFIG_X11
> > +  if (vo_wintitle != NULL)
> > +      title = vo_wintitle;
> > +#endif
> 
> I don't see the point of making this X11-specific.
because i put it in x11_common functions and it works in x11 ;P
probably you're right and it should be implemented in other vo's too,
which i really can't do :(
anyway moving the variable and its settings to video_out.c could be a
good idea, posing the stone for other vo's implementation.
i'll do, i was alredy in doubt where to put all this code indeed ;)

> > Index: mplayer.c
> > ===================================================================
> > --- mplayer.c	(revisione 29789)
> > +++ mplayer.c	(copia locale)
> > @@ -2191,6 +2191,11 @@
> >    {
> >      char* vf_arg[] = { "_oldargs_", (char*)mpctx->video_out ,
> > NULL }; sh_video->vfilter=(void*)vf_open_filter(NULL,"vo",vf_arg);
> > +#ifdef CONFIG_X11
> > +    if(vo_wintitle == NULL)
> > +      vo_wintitle = mp_basename2 (filename);
> > +    vo_x11_settitle(vo_wintitle);
> > +#endif
> 
> Seems to me like there's no way to get the current behaviour.

didnt' really understand what you're saying.. ????

> > Index: cfg-mplayer.h
> > ===================================================================
> > --- cfg-mplayer.h	(revisione 29789)
> > +++ cfg-mplayer.h	(copia locale)
> > @@ -170,6 +170,9 @@
> >  	{"screenh", &vo_screenheight, CONF_TYPE_INT,
> > CONF_RANGE|CONF_OLD, 0, 4096, NULL}, // Geometry string
> >  	{"geometry", &vo_geometry, CONF_TYPE_STRING, 0, 0, 0,
> > NULL},
> > +	// X11 Hints strings
> > +	{"name", &vo_winname, CONF_TYPE_STRING, 0, 0, 0, NULL},
> > +	{"title", &vo_wintitle, CONF_TYPE_STRING, 0, 0, 0, NULL},
> 
> I have some doubts this really works correctly with -fixed-vo and
> different values for these for different files.

good point, my fault :(
title should be updated at every file change, right :)
(this answers a little to the one above, the condition == NULL is
nosense in that other situation, probably :( )

anyway, point to do:
1. move from x11 code to video_out generic code
2. verify fixed-vo and related stuff.

is it right?
thanx bye