[MPlayer-dev-eng] window title patch

[Guillaume POIRIER]

Hi,

On 3/19/06, Danny Milosavljevic <danny.milo at gmx.net> wrote:
> Hi,
>
> Am Dienstag, den 14.03.2006, 08:30 +0100 schrieb Guillaume POIRIER:
> > Hi,
> >
> > On 2/12/06, Danny Milosavljevic <danny.milo at gmx.net> wrote:
> > > Hi,
> > >
> > > attached is a (messy) patch that puts the name of the current video into
> > > the window title ... perhaps you find it useful.
> > >
> > > That probably should be solved at a higher layer, but it _does_ work
> > > with that, at least :)
> >
> > The patch doesn't currently work. Here's what I get at the end of build:
> >
> > libvo/libvo.a(x11_common.o): In function
> > `vo_x11_create_smooth_window':x11_common.c:(.text+0x21be): undefined
> > reference to `XpmCreatePixmapFromData'
> > collect2: ld returned 1 exit status
> >
> >
> > I'll try to look into it later on, but if you could fix this, and
> > resend a patch, that would be greatly appreciated... I've tried to
> > append '-lxpm' to the build line, but it didn't change anything...
>
> Append '-lXpm' (big "X").
>
> I feel uneasy patching around in Makefiles I didnt write (and thus dont
> understand the implications completely) :)

The only thing that should be patched is the configure script, in
order to detect the presence of xpm lib. If it's present, code that
depend on it can be built, and if it's not, it should not be built.
In the C code, that means that you should put some #ifdef CONFIG_XPM
around the code that uses XPM. On the configure script, you should
come up with a simple program that uses XPM, which the configure
script will attempt to build (and depending on the success of that
build, the symbol CONFIG_XPM will be set or not).

BTW, I suggest you split your patch in 2 parts: one that adds the
icon, using XPM, and the other that adds the title. Those are 2
unrelated features, therefore they should be split to ease reviewing
process.


> Also, I'm not sure if static libraries (libvo) can list dynamic
> libraries (Xpm) as dependencies, and how. I just added it to the main
> mplayer link command here...
> try adding "-lXpm" to the toplevel Makefile on "VO_LIBS" ....

Nope, not that I know of. The key is to add support in ./configure so
it can test the presence of xpm lib.


> > Regarding the way your patch is done, and whether it should be done at
> > a higher level or not, I frankly don't know. It would certainly be
> > _better_ if it could be done at a higher level, but then how can this
> > be done? Don't know!
>
> Well, by introducing a function at the vo level that is called when the
> file currently played changes (switched to another name, like in a
> playlist when one of the files is done playing), and calling it, giving
> it the file path as parameter.

That seems fair. Do you think you could implement this?

I haven't tested you current patch yes, but I have it built on my tree
now. I'll see how it goes...

Thanks!

Guillaume

--
I am disillusioned enough to know that no man's opinion on any subject
is worth a damn unless backed up with enough genuine information to
make him really know what he's talking about.

-- H. P. Lovecraft (about the flamewars on FFmpeg and MPlayer-dev mailing lists)
http://www.brainyquote.com/quotes/quotes/h/hplovecr278144.html