[PATCH] Add support for GNOME screensaver
Hello! I've wrote a patch to support GNOME screensaver in MPlayer, i.e. to disable it when playing a movie and reenabling it after that. The code requires that dbus and dbus-glib is present, as it uses GNOME screensaver's DBUS API for the task. It's integrated into x11_common.c and so it's only compiled when X11 is present. Please comment on it, especially on the changes in configure. Maybe they can be improved. Piotr
On Wed, 2006-04-26 at 15:50 +0200, Piotr Kaczuba wrote:
I've wrote a patch to support GNOME screensaver in MPlayer, i.e. to disable it when playing a movie and reenabling it after that.
Does this have any advantages over disabling the screensaver with common X calls? Does the GNOME screensaver not respect those? Btw anyone know what happened with the X screensaver extension support patch? There were a couple of questions but it looked mostly OK, anyone going to finalize it?
Çarşamba 26 Nisan 2006 17:04 tarihinde, Uoti Urpala şunları yazmıştı:
Btw anyone know what happened with the X screensaver extension support patch? There were a couple of questions but it looked mostly OK, anyone going to finalize it?
I dropped it for now, the original author of the XScreenSaverSuspend() code told me none of the screensavers around ( KDE,Gnome, etc ) supports this! Until we can get people to fix these, XScreenSaverSuspend() is not much use. I am keeping an eye on this and will re-send the patch once the KDE/Gnome screensavers are fixed. Regards, ismail --
On Wed, Apr 26, 2006 at 05:04:37PM +0300, Uoti Urpala wrote:
On Wed, 2006-04-26 at 15:50 +0200, Piotr Kaczuba wrote:
I've wrote a patch to support GNOME screensaver in MPlayer, i.e. to disable it when playing a movie and reenabling it after that.
Does this have any advantages over disabling the screensaver with common X calls? Does the GNOME screensaver not respect those? [...]
GNOME screensaver provides an API of its own for controlling it via DBUS messages. I don't know, it can be done with common X calls, but IMHO sending a message to the screensaver via DBUS is very clear and simple.
Am Mittwoch, 26. April 2006 16:36 schrieb Piotr Kaczuba:
On Wed, Apr 26, 2006 at 05:04:37PM +0300, Uoti Urpala wrote:
On Wed, 2006-04-26 at 15:50 +0200, Piotr Kaczuba wrote:
I've wrote a patch to support GNOME screensaver in MPlayer, i.e. to disable it when playing a movie and reenabling it after that.
Does this have any advantages over disabling the screensaver with common X calls? Does the GNOME screensaver not respect those?
[...]
GNOME screensaver provides an API of its own for controlling it via DBUS messages. I don't know, it can be done with common X calls, but IMHO sending a message to the screensaver via DBUS is very clear and simple.
So you want to introduce special cases for KDE, Gnome, XFCE, Windowmaker, CDE,...?
On Thursday 27 April 2006 18:41, Matthias Wieser wrote:
So you want to introduce special cases for KDE, Gnome, XFCE, Windowmaker, CDE,...?
I hope XFCE, wmaker and CDE use xscreensaver instead of providing their own.
On Wed, 26 Apr 2006 15:50:17 +0200 Piotr Kaczuba <pepe@attika.ath.cx> wrote:
+#ifdef HAVE_DBUS_GLIB + gnome_screensaver_enable(); +#endif + } if (kdescreensaver_was_running && stop_xscreensaver) { system @@ -1738,8 +1746,12 @@ allow_exp); } // turning off screensaver - if (stop_xscreensaver) + if (stop_xscreensaver) { xscreensaver_disable(mDisplay); +#ifdef HAVE_DBUS_GLIB + gnome_screensaver_disable(); +#endif + }
Shouldn't you check whether the screensaver is actualy running before disabling it, to prevent enabling it when it wasnt enabled? And how does your patch behave, when gnome is installed, but not running ? Attila Kinali -- 心をこめて聞け心をこめて話せ
On Wed, Apr 26, 2006 at 04:08:32PM +0200, Attila Kinali wrote:
On Wed, 26 Apr 2006 15:50:17 +0200 Piotr Kaczuba <pepe@attika.ath.cx> wrote:
+#ifdef HAVE_DBUS_GLIB + gnome_screensaver_enable(); +#endif + } if (kdescreensaver_was_running && stop_xscreensaver) { system @@ -1738,8 +1746,12 @@ allow_exp); } // turning off screensaver - if (stop_xscreensaver) + if (stop_xscreensaver) { xscreensaver_disable(mDisplay); +#ifdef HAVE_DBUS_GLIB + gnome_screensaver_disable(); +#endif + }
Shouldn't you check whether the screensaver is actualy running before disabling it, to prevent enabling it when it wasnt enabled?
And how does your patch behave, when gnome is installed, but not running ?
Actually, I'm sending an InhibitActivation message to the screensaver, it's not a real disable. So, when it's not enabled, InhibitActivation doesn't do anything. Similarly, AllowActivation is sent when stopping playing: it won't enable/start the screensaver when it isn't running. See the last question at http://live.gnome.org/GnomeScreensaver/FrequentlyAskedQuestions, which gives some examples of how to do it. Attached there is the proper patch, as the previous one has two files missing.
On Wed, Apr 26, 2006 at 04:31:04PM +0200, Piotr Kaczuba wrote:
Attached there is the proper patch, as the previous one has two files missing.
--- main/configure 2006-04-26 03:59:05.000000000 +0200 +++ main.new/configure 2006-04-26 14:40:07.527783461 +0200 @@ -6815,6 +6815,23 @@ fi echores "$_gethostbyname2"
+# Check for D-BUS GLib interface
This line is superfluous.
+echocheck "D-BUS GLib interface" +if pkg-config --exists dbus-glib-1; then + _dbus_glib=yes
Only run the test if _dbus_glib is set to auto.
+echores "$_dbus_glib" + +if test "$_x11" = yes -a "$_dbus_glib" = yes; then + _vosrc="$_vosrc gnome_screensaver.c" +fi
The last 3 lines belong in the test block itself, not after it. Diego
Piotr Kaczuba wrote:
Attached there is the proper patch, as the previous one has two files missing.
Next time try: $ cvsdo add libvo/gnome_screensaver.c libvo/gnome_screensaver.h That will add the files to your CVS tree without needing write access to the CVS server and 'cvs diff' will diff the new files. -Corey
On Wed, Apr 26, 2006 at 04:31:04PM +0200, Piotr Kaczuba wrote:
On Wed, Apr 26, 2006 at 04:08:32PM +0200, Attila Kinali wrote:
On Wed, 26 Apr 2006 15:50:17 +0200 Piotr Kaczuba <pepe@attika.ath.cx> wrote:
+#ifdef HAVE_DBUS_GLIB + gnome_screensaver_enable(); +#endif + } if (kdescreensaver_was_running && stop_xscreensaver) { system @@ -1738,8 +1746,12 @@ allow_exp); } // turning off screensaver - if (stop_xscreensaver) + if (stop_xscreensaver) { xscreensaver_disable(mDisplay); +#ifdef HAVE_DBUS_GLIB + gnome_screensaver_disable(); +#endif + }
Shouldn't you check whether the screensaver is actualy running before disabling it, to prevent enabling it when it wasnt enabled?
And how does your patch behave, when gnome is installed, but not running ?
Actually, I'm sending an InhibitActivation message to the screensaver, it's not a real disable. So, when it's not enabled, InhibitActivation doesn't do anything. Similarly, AllowActivation is sent when stopping playing: it won't enable/start the screensaver when it isn't running. See the last question at http://live.gnome.org/GnomeScreensaver/FrequentlyAskedQuestions, which gives some examples of how to do it.
Attached there is the proper patch, as the previous one has two files missing.
diff -uNr main/Makefile main.new/Makefile --- main/Makefile 2006-04-25 04:01:21.000000000 +0200 +++ main.new/Makefile 2006-04-26 14:44:57.303825307 +0200 @@ -74,6 +74,7 @@ $(DIRECTFB_LIB) \ $(CACA_LIB) \ $(VESA_LIB) \ + $(DBUS_GLIB_LIB) \
ifeq ($(EXTERNAL_VIDIX),yes) VO_LIBS += $(EXTERNAL_VIDIX_LIB) @@ -307,7 +308,7 @@ $(MAKE) -C libmpeg2
libvo/libvo.a: - $(MAKE) -C libvo + $(MAKE) -C libvo all
libao2/libao2.a: $(MAKE) -C libao2 diff -uNr main/configure main.new/configure --- main/configure 2006-04-26 03:59:05.000000000 +0200 +++ main.new/configure 2006-04-26 14:40:07.527783461 +0200 @@ -6815,6 +6815,23 @@ fi echores "$_gethostbyname2"
+# Check for D-BUS GLib interface +echocheck "D-BUS GLib interface" +if pkg-config --exists dbus-glib-1; then + _dbus_glib=yes + _inc_dbus_glib=`pkg-config --cflags dbus-glib-1 2>/dev/null` + _ld_dbus_glib=`pkg-config --libs dbus-glib-1 2>/dev/null` + _def_dbus_glib='#define HAVE_DBUS_GLIB 1' +else + _dbus_glib=no + _def_dbus_glib='#undef HAVE_DBUS_GLIB' +fi +echores "$_dbus_glib" + +if test "$_x11" = yes -a "$_dbus_glib" = yes; then + _vosrc="$_vosrc gnome_screensaver.c" +fi + # --------------- GUI specific tests begin ------------------- echocheck "GUI" echo "$_gui" @@ -7290,6 +7307,8 @@ AA_LIB = $_ld_aa CACA_INC = $_inc_caca CACA_LIB = $_ld_caca +DBUS_GLIB_INC = $_inc_dbus_glib +DBUS_GLIB_LIB = $_ld_dbus_glib
# audio output ALSA_LIB = $_ld_alsa @@ -8116,6 +8135,14 @@ OPTIONAL_OBJS = $_voobj EOF
+if test "$_x11" = yes -a "$_dbus_glib" = yes; then + cat >> libvo/config.mak << EOF + +gnome_screensaver.o: gnome_screensaver.c + \$(CC) -c \$(CFLAGS) \$(DBUS_GLIB_INC) -o \$@ $< +EOF +fi + #############################################################################
echo "Creating libao2/config.mak" diff -uNr main/libvo/gnome_screensaver.c main.new/libvo/gnome_screensaver.c --- main/libvo/gnome_screensaver.c 1970-01-01 01:00:00.000000000 +0100 +++ main.new/libvo/gnome_screensaver.c 2006-04-26 13:39:18.290095378 +0200 @@ -0,0 +1,98 @@ +#include <stdlib.h> +#include <unistd.h> +#include <dbus/dbus-glib.h> + +#include "gnome_screensaver.h" + +#define GS_SERVICE "org.gnome.ScreenSaver" +#define GS_PATH "/org/gnome/ScreenSaver" +#define GS_INTERFACE "org.gnome.ScreenSaver" + +void gnome_screensaver_enable() +{ + DBusGConnection *connection; + GError *error; + DBusGProxy *proxy; + + g_type_init(); + + error = NULL; + connection = dbus_g_bus_get(DBUS_BUS_SESSION, &error); + if (connection == NULL) { + g_printerr("Failed to open connection to bus: %s\n",
see bellow
+ error->message); + g_error_free(error); + return; + } + + /* Create a proxy object */ + + proxy = dbus_g_proxy_new_for_name(connection, + GS_SERVICE, GS_PATH, GS_INTERFACE); + + /* Call method, wait for reply */ + error = NULL; + if (!dbus_g_proxy_call + (proxy, "AllowActivation", &error, G_TYPE_INVALID, + G_TYPE_INVALID)) { + /* Just do demonstrate remote exceptions versus regular GError */ + if (error->domain == DBUS_GERROR + && error->code == DBUS_GERROR_REMOTE_EXCEPTION) + g_printerr("Caught remote method exception %s: %s",
see bellow
+ dbus_g_error_get_name(error), error->message); + else + g_printerr("Error: %s\n", error->message);
see bellow
+ + g_error_free(error); + } + else { + g_print("Gnome screensaver enabled\n");
see bellow
+ } + + g_object_unref(proxy); +} + + +void gnome_screensaver_disable() +{ + DBusGConnection *connection; + GError *error; + DBusGProxy *proxy; + + g_type_init(); + + error = NULL; + connection = dbus_g_bus_get(DBUS_BUS_SESSION, &error); + if (connection == NULL) { + g_printerr("Failed to open connection to bus: %s\n",
see bellow
+ error->message); + g_error_free(error); + return; + } + + /* Create a proxy object */ + + proxy = dbus_g_proxy_new_for_name(connection, + GS_SERVICE, GS_PATH, GS_INTERFACE); + + /* Call method, wait for reply */ + error = NULL; + if (!dbus_g_proxy_call + (proxy, "InhibitActivation", &error, G_TYPE_STRING, + "Playing a movie", G_TYPE_INVALID, G_TYPE_INVALID)) { + /* Just do demonstrate remote exceptions versus regular GError */ + if (error->domain == DBUS_GERROR + && error->code == DBUS_GERROR_REMOTE_EXCEPTION) + g_printerr("Caught remote method exception %s: %s",
see bellow
+ dbus_g_error_get_name(error), error->message); + else + g_printerr("Error: %s\n", error->message);
see bellow
+ + g_error_free(error); + } + else { + g_print("Gnome screensaver disabled\n");
see bellow or you hack your way around some kind of g_set_print_handler call or you replace g_printerr/g_print ocurrences with our messaging routines, this is a big patch stoper. Best regards Reynaldo
On Wed, Apr 26, 2006 at 07:16:53PM -0400, Reynaldo H. Verdejo Pinochet wrote: [...]
see bellow
or you hack your way around some kind of g_set_print_handler call or you replace g_printerr/g_print ocurrences with our messaging routines, this is a big patch stoper.
I understand that you object to g_print/g_printerr but which place do you exactly mean with see below? Piotr
On Thu, Apr 27, 2006 at 10:30:52AM +0200, Piotr Kaczuba wrote:
On Wed, Apr 26, 2006 at 07:16:53PM -0400, Reynaldo H. Verdejo Pinochet wrote: [...]
see bellow
or you hack your way around some kind of g_set_print_handler call or you replace g_printerr/g_print ocurrences with our messaging routines, this is a big patch stoper.
I understand that you object to g_print/g_printerr but which place do you exactly mean with see below?
The multiple 'see bellow' lines where there to instruct you find the relevant comentary at the end of the message, hope you can understand my weird english. Reynaldo.
Attached is the third revision of the patch. I've considered Diego Biurrun's remarks regarding the configure script and made a cvs diff (thanks to Corey Hickey for explaining how to do it properly).
On Wed, Apr 26, 2006 at 08:43:38PM +0200, Piotr Kaczuba wrote:
Attached is the third revision of the patch. I've considered Diego Biurrun's remarks regarding the configure script and made a cvs diff (thanks to Corey Hickey for explaining how to do it properly).
--- Makefile 25 Apr 2006 02:01:21 -0000 1.367 +++ Makefile 26 Apr 2006 18:11:41 -0000 @@ -307,7 +308,7 @@ $(MAKE) -C libmpeg2
libvo/libvo.a: - $(MAKE) -C libvo + $(MAKE) -C libvo all
Why?
--- configure 26 Apr 2006 01:59:05 -0000 1.1179 +++ configure 26 Apr 2006 18:11:41 -0000 @@ -6815,6 +6819,22 @@ fi echores "$_gethostbyname2"
+echocheck "D-BUS GLib interface" +if test "$_dbus_glib" != no && pkg-config --exists dbus-glib-1; then
The check should only run when _dbus_glib is set to auto. The value yes is reserved to force the setting.
@@ -8116,6 +8138,14 @@ OPTIONAL_OBJS = $_voobj EOF
+if test "$_x11" = yes -a "$_dbus_glib" = yes; then + cat >> libvo/config.mak << EOF + +gnome_screensaver.o: gnome_screensaver.c + \$(CC) -c \$(CFLAGS) \$(DBUS_GLIB_INC) -o \$@ $< +EOF
I don't like this. These rules belong in the Makefile, not somewhere else. Diego
libvo/libvo.a: - $(MAKE) -C libvo + $(MAKE) -C libvo all
Why? Because after applying my patch the libvo.a rule isn't the first one in
On Thu, Apr 27, 2006 at 12:36:13AM +0200, Diego Biurrun wrote: [...] the Makefile anymore.
--- configure 26 Apr 2006 01:59:05 -0000 1.1179 +++ configure 26 Apr 2006 18:11:41 -0000 @@ -6815,6 +6819,22 @@ fi echores "$_gethostbyname2"
+echocheck "D-BUS GLib interface" +if test "$_dbus_glib" != no && pkg-config --exists dbus-glib-1; then
The check should only run when _dbus_glib is set to auto. The value yes is reserved to force the setting.
But what about the libs and includes? If we skip autodetection of the paths to the libs and includes when the user sets _dbus_glib to yes, the whole thing won't compile.
@@ -8116,6 +8138,14 @@ OPTIONAL_OBJS = $_voobj EOF
+if test "$_x11" = yes -a "$_dbus_glib" = yes; then + cat >> libvo/config.mak << EOF + +gnome_screensaver.o: gnome_screensaver.c + \$(CC) -c \$(CFLAGS) \$(DBUS_GLIB_INC) -o \$@ $< +EOF
I don't like this. These rules belong in the Makefile, not somewhere else. I can do that. That would also eliminate the "make all" patch above.
Piotr
On Thu, Apr 27, 2006 at 10:37:01AM +0200, Piotr Kaczuba wrote:
--- configure 26 Apr 2006 01:59:05 -0000 1.1179 +++ configure 26 Apr 2006 18:11:41 -0000 @@ -6815,6 +6819,22 @@ fi echores "$_gethostbyname2"
+echocheck "D-BUS GLib interface" +if test "$_dbus_glib" != no && pkg-config --exists dbus-glib-1; then
The check should only run when _dbus_glib is set to auto. The value yes is reserved to force the setting. But what about the libs and includes? If we skip autodetection of the
On Thu, Apr 27, 2006 at 12:36:13AM +0200, Diego Biurrun wrote: paths to the libs and includes when the user sets _dbus_glib to yes, the whole thing won't compile.
The semantics of configure are clearly defined: "yes" means no autodetection. The user then has to manually pass include paths with --with-extra*. This comes up again and again. I'll add some explanation to the top of the file in a minute.
@@ -8116,6 +8138,14 @@ OPTIONAL_OBJS = $_voobj EOF
+if test "$_x11" = yes -a "$_dbus_glib" = yes; then + cat >> libvo/config.mak << EOF + +gnome_screensaver.o: gnome_screensaver.c + \$(CC) -c \$(CFLAGS) \$(DBUS_GLIB_INC) -o \$@ $< +EOF
I don't like this. These rules belong in the Makefile, not somewhere else. I can do that. That would also eliminate the "make all" patch above.
Good. Diego
Below the forth revision of my patch. I've made the following changes: 1. Make HAVE_DBUS_GLIB actually appear in config.h 2. Move gnome_screensaver.o rule from config.mak to Makefile 3. Run autodetection in configure only if _dbus_glib = auto 4. Replace g_print/g_printerr with mp_msg in gnome_screensaver.c Piotr
Hi, On Thu, Apr 27, 2006 at 05:30:18PM +0200, Piotr Kaczuba wrote:
Below the forth revision of my patch. I've made the following changes:
1. Make HAVE_DBUS_GLIB actually appear in config.h 2. Move gnome_screensaver.o rule from config.mak to Makefile 3. Run autodetection in configure only if _dbus_glib = auto 4. Replace g_print/g_printerr with mp_msg in gnome_screensaver.c
I personally would very much like to see is either: 1) a really good explanation why this is _necessary_, and why this shouldn't be fixed by the gnome people instead or: 2) at least 50% reduction in code size. Greetings, Reimar Doeffinger
On Thu, Apr 27, 2006 at 06:20:51PM +0200, Reimar Doeffinger wrote:
On Thu, Apr 27, 2006 at 05:30:18PM +0200, Piotr Kaczuba wrote:
Below the forth revision of my patch. I've made the following changes:
1. Make HAVE_DBUS_GLIB actually appear in config.h 2. Move gnome_screensaver.o rule from config.mak to Makefile 3. Run autodetection in configure only if _dbus_glib = auto 4. Replace g_print/g_printerr with mp_msg in gnome_screensaver.c
I personally would very much like to see is either: 1) a really good explanation why this is _necessary_, and why this shouldn't be fixed by the gnome people instead or: 2) at least 50% reduction in code size.
IMHO, this is necessary, because as it is now, the screensaver *will* kick in, unless the user disables it *and* the power management each time, before running mplayer to watch a full time movie and after that reenables both. This isn't acceptable. Of course, you can wait one month, half a year or longer until the gnome people implement e.g. the X screensaver extension. But then again, why does mplayer has Xscreensaver and KDE screensaver support? Gnome_screensaver.c consists of two functions, which both basically call another three functions. How is this too much code? How would you reduce it by 50%? Piotr
Hi, On Thu, Apr 27, 2006 at 07:20:26PM +0200, Piotr Kaczuba wrote:
extension. But then again, why does mplayer has Xscreensaver and KDE screensaver support?
KDE screensaver? If there is already a special case for that, that would of course make my objection moot.
Gnome_screensaver.c consists of two functions, which both basically call another three functions. How is this too much code? How would you reduce it by 50%?
Well, I find it too much for something which I currently (without any fact-checking) I see as a problem the gnome people created. Also, I find it highly laughable that something as simple as preventing the screensaver is more than 4 line max. But that's the same with xscreensaver I must admit. I guess such vague statements are annoying, I just wanted to express something like "my gut smells crap", I won't mind anyone applying it anyway though (after reviewing it), after all I'm in a complete lack of facts. Greetings, Reimar Döffinger
On Thu, Apr 27, 2006 at 05:30:18PM +0200, Piotr Kaczuba wrote: [..]
Index: libvo/Makefile =================================================================== RCS file: /cvsroot/mplayer/main/libvo/Makefile,v retrieving revision 1.54 diff -u -r1.54 Makefile --- libvo/Makefile 28 Mar 2006 22:44:16 -0000 1.54 +++ libvo/Makefile 27 Apr 2006 14:56:51 -0000 @@ -62,6 +62,9 @@ vo_mpegpes.o: vo_mpegpes.c $(CC) -c $(CFLAGS) $(DVB_INC) -o $@ $<
+gnome_screensaver.o: gnome_screensaver.c + $(CC) -c $(CFLAGS) $(DBUS_GLIB_INC) -o $@ $< + all: $(LIBNAME)
clean:
Hmm, how would that work if there is no dbus support installed? There is no check in the next file which would prevent it to fail.
Index: libvo/gnome_screensaver.c =================================================================== RCS file: libvo/gnome_screensaver.c diff -N libvo/gnome_screensaver.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ libvo/gnome_screensaver.c 27 Apr 2006 14:56:51 -0000 @@ -0,0 +1,107 @@ +#include <stdlib.h> +#include <unistd.h> +#include <dbus/dbus-glib.h>
Here, you are including something without protecting it. Maybe you should check for HAVE_DBUS_GLIB in libvo/Makefile and protect the new lines which you just added in that Makefile. Bernd
On Thu, Apr 27, 2006 at 06:56:29PM +0200, Bernd Ernesti wrote:
On Thu, Apr 27, 2006 at 05:30:18PM +0200, Piotr Kaczuba wrote: [..]
Index: libvo/Makefile =================================================================== RCS file: /cvsroot/mplayer/main/libvo/Makefile,v retrieving revision 1.54 diff -u -r1.54 Makefile --- libvo/Makefile 28 Mar 2006 22:44:16 -0000 1.54 +++ libvo/Makefile 27 Apr 2006 14:56:51 -0000 @@ -62,6 +62,9 @@ vo_mpegpes.o: vo_mpegpes.c $(CC) -c $(CFLAGS) $(DVB_INC) -o $@ $<
+gnome_screensaver.o: gnome_screensaver.c + $(CC) -c $(CFLAGS) $(DBUS_GLIB_INC) -o $@ $< + all: $(LIBNAME)
clean:
Hmm, how would that work if there is no dbus support installed?
There is no check in the next file which would prevent it to fail.
Index: libvo/gnome_screensaver.c =================================================================== RCS file: libvo/gnome_screensaver.c diff -N libvo/gnome_screensaver.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ libvo/gnome_screensaver.c 27 Apr 2006 14:56:51 -0000 @@ -0,0 +1,107 @@ +#include <stdlib.h> +#include <unistd.h> +#include <dbus/dbus-glib.h>
Here, you are including something without protecting it.
Maybe you should check for HAVE_DBUS_GLIB in libvo/Makefile and protect the new lines which you just added in that Makefile.
It's not necessary to protect the includes because if configure detects that dbus glib is not present, gnome_screensaver.o isn't compiled, i.e. the c file isn't included in config.mak. For the same reason, the makefile rule doesn't hurt. Piotr
On Thu, Apr 27, 2006 at 05:30:18PM +0200, Piotr Kaczuba wrote:
Below the forth revision of my patch. I've made the following changes:
1. Make HAVE_DBUS_GLIB actually appear in config.h 2. Move gnome_screensaver.o rule from config.mak to Makefile 3. Run autodetection in configure only if _dbus_glib = auto 4. Replace g_print/g_printerr with mp_msg in gnome_screensaver.c
Nice, thanks for taking the time to do so. just a minor correction: if youre going to use mp_msg then you need to make sure message levels between MSGL_FATAL-MSGL_STATUS are translatable and not harcoded as english strings in your code. see help/help_mp-en.h for a glance on how its currently done. regards Reynaldo
Below is the fifth revision of the patch. I changed the mp_msg strings to macros defined in help_mp-en.h. Piotr
On 4/28/06, Piotr Kaczuba <pepe@attika.ath.cx> wrote:
Below is the fifth revision of the patch. I changed the mp_msg strings to macros defined in help_mp-en.h.
I've tried it and it does not work: gnome_screensaver_disable(): Error: Method "InhibitActivation" with signature "s" on interface "org.gnome.ScreenSaver" doesn't exist This is with gnome-screensaver 2.15.1 on Mandriva Cooker i586. -- Terminate Schwarzenegger
On Fri, Apr 28, 2006 at 03:45:05PM +0200, G?tz Waschk wrote:
On 4/28/06, Piotr Kaczuba <pepe@attika.ath.cx> wrote:
Below is the fifth revision of the patch. I changed the mp_msg strings to macros defined in help_mp-en.h.
I've tried it and it does not work: gnome_screensaver_disable(): Error: Method "InhibitActivation" with signature "s" on interface "org.gnome.ScreenSaver" doesn't exist
This is with gnome-screensaver 2.15.1 on Mandriva Cooker i586.
It seems that the API changed with version 2.15. I'm using 2.14 from Debian unstable. I'll see if I can come up with a patch that supports both versions. Piotr
On Fri, Apr 28, 2006 at 03:45:05PM +0200, Götz Waschk wrote:
On 4/28/06, Piotr Kaczuba <pepe@attika.ath.cx> wrote:
Below is the fifth revision of the patch. I changed the mp_msg strings to macros defined in help_mp-en.h.
I've tried it and it does not work: gnome_screensaver_disable(): Error: Method "InhibitActivation" with signature "s" on interface "org.gnome.ScreenSaver" doesn't exist
This is with gnome-screensaver 2.15.1 on Mandriva Cooker i586.
I've just posted the sixth revision of the patch which should handle gnome screensaver 2.15 just fine. Could you try it? Piotr
On 4/28/06, Piotr Kaczuba <pepe@attika.ath.cx> wrote:
I've just posted the sixth revision of the patch which should handle gnome screensaver 2.15 just fine. Could you try it?
Yes, it disables gnome-screensaver 2.15. Thanks, Götz -- Terminate Schwarzenegger
Attached is the sixth revision of the patch. It should now use the GNOME screensaver 2.15 API first and if this fails, revert to 2.14 API. Piotr
On Friday, 28 April 2006 at 20:42, Piotr Kaczuba wrote:
Attached is the sixth revision of the patch. It should now use the GNOME screensaver 2.15 API first and if this fails, revert to 2.14 API.
Piotr
+ + /* First call the GNOME screensaver 2.15 API method */ + error = NULL; + ret = + dbus_g_proxy_call(proxy, "UnInhibit", &error, G_TYPE_UINT, cookie, + G_TYPE_INVALID); + + /* If this fails, try the GNOME screensaver 2.14 API */ + if (!ret && error->domain == DBUS_GERROR + && error->code == DBUS_GERROR_UNKNOWN_METHOD) { + mp_msg(MSGT_VO, MSGL_V, + "%s: GNOME screensaver 2.15 API failed, trying 2.14 API\n", + funcname); + error = NULL; + ret = + dbus_g_proxy_call(proxy, "AllowActivation", &error, + G_TYPE_INVALID, G_TYPE_INVALID); + }
A static variable remembering which API should be used could be in order here. [...]
+ /* First call the GNOME screensaver 2.15 API method */ + error = NULL; + ret = + dbus_g_proxy_call(proxy, "Inhibit", &error, G_TYPE_STRING, + GS_APPLICATION_NAME, G_TYPE_STRING, + GS_REASON_FOR_INHIBIT, G_TYPE_INVALID, + G_TYPE_UINT, cookie, G_TYPE_INVALID); + + /* If this fails, try the GNOME screensaver 2.14 API */ + if (!ret && error->domain == DBUS_GERROR + && error->code == DBUS_GERROR_UNKNOWN_METHOD) { + mp_msg(MSGT_VO, MSGL_V, + "%s: GNOME screensaver 2.15 API failed, trying 2.14 API\n", + funcname); + error = NULL; + ret = + dbus_g_proxy_call(proxy, "InhibitActivation", &error, + G_TYPE_STRING, GS_REASON_FOR_INHIBIT, + G_TYPE_INVALID, G_TYPE_INVALID); + }
You could use it here then, eliminating half of the function calls. Regards, R. -- MPlayer developer and RPMs maintainer: http://rpm.greysector.net/mplayer/ There should be a science of discontent. People need hard times and oppression to develop psychic muscles. -- from "Collected Sayings of Muad'Dib" by the Princess Irulan
On Fri, Apr 28, 2006 at 10:35:17PM +0200, Dominik 'Rathann' Mierzejewski wrote:
On Friday, 28 April 2006 at 20:42, Piotr Kaczuba wrote: [...] A static variable remembering which API should be used could be in order here. [...] You could use it here then, eliminating half of the function calls.
I'll think about it, but it would eliminate just one function call per enable/disable action not half of them. Piotr
I'm attaching yet another revision of the patch. There's now only one function, which takes an argument to control if the screensaver should be enabled or disabled, which simplifies the code a little. Piotr
On Sat, Apr 29, 2006 at 03:29:51PM +0200, Piotr Kaczuba wrote:
I'm attaching yet another revision of the patch. There's now only one function, which takes an argument to control if the screensaver should be enabled or disabled, which simplifies the code a little.
Piotr
So, what will happen with this patch after all? it works, doesnt breaks a thing but, as other may had said, keep commiting this kind of fixes* may encourage fuked up devs of name-you-desktop-environment-here to keep working against any standar whatsoever, pushing his methods over the rests as in some kind of sacred war everyone have to agree with and suffer upon. my advice? commit this and wait for the hundred of lines we will have in some time to support every new way of doing simple things like this one. Piotr, dont take me wrong, i know you have done you homework updating and fixing your patch, i just wanna know what to do. Can I forget about this patch? is there anyone going to commit this? it is rejected? Best regards Reynaldo
participants (14)
-
Attila Kinali -
Bernd Ernesti -
Corey Hickey -
Diego Biurrun -
Dominik 'Rathann' Mierzejewski -
Götz Waschk -
Ismail Donmez -
Jan Knutar -
Matthias Wieser -
Piotr Kaczuba -
Reimar Doeffinger -
Reimar Döffinger
-
Reynaldo H. Verdejo Pinochet -
Uoti Urpala