[PATCH] factor common code to handle -wid, vo_gc creation etc. out into x11_common.c
Hello, attached patch gets rid of a large part of massive code duplication. The reason I am sending this as a patch first is that I can't test most -vos, though I do not think anything will break besides where the behaviour was previously inconsistent between vos. Since I am almost the only one working on libvo stuff I will only give a few days to test/object though. In a next step I will probably move the CONFIG_XF86VM stuff, too, though that will have the side effect that for vos that do not support it, when -vm is specified on the command line it will no longer be ignored but instead break things completely - IMO this is acceptable. Greetings, Reimar Döffinger
On 11/16/08, Reimar Döffinger <Reimar.Doeffinger@stud.uni-karlsruhe.de> wrote:
Hello, attached patch gets rid of a large part of massive code duplication. The reason I am sending this as a patch first is that I can't test most -vos, though I do not think anything will break besides where the behaviour was previously inconsistent between vos. Since I am almost the only one working on libvo stuff I will only give a few days to test/object though. In a next step I will probably move the CONFIG_XF86VM stuff, too, though that will have the side effect that for vos that do not support it, when -vm is specified on the command line it will no longer be ignored but instead break things completely - IMO this is acceptable.
The following code vanishes without explanation: - XGetGeometry(mDisplay, vo_window, &mRoot, - &drwX, &drwY, &vo_dwidth, &vo_dheight, - &drwBorderWidth, &drwDepth); - aspect_save_prescale(vo_dwidth, vo_dheight);
On Sun, Nov 16, 2008 at 07:58:14PM +0200, Ivan Kalvachev wrote:
On 11/16/08, Reimar Döffinger <Reimar.Doeffinger@stud.uni-karlsruhe.de> wrote:
Hello, attached patch gets rid of a large part of massive code duplication. The reason I am sending this as a patch first is that I can't test most -vos, though I do not think anything will break besides where the behaviour was previously inconsistent between vos. Since I am almost the only one working on libvo stuff I will only give a few days to test/object though. In a next step I will probably move the CONFIG_XF86VM stuff, too, though that will have the side effect that for vos that do not support it, when -vm is specified on the command line it will no longer be ignored but instead break things completely - IMO this is acceptable.
The following code vanishes without explanation:
- XGetGeometry(mDisplay, vo_window, &mRoot, - &drwX, &drwY, &vo_dwidth, &vo_dheight, - &drwBorderWidth, &drwDepth); - aspect_save_prescale(vo_dwidth, vo_dheight);
Not really, it is in x11_common.c, the line if (WinID) vo_x11_update_geometry(); (note that of the values set here only vo_dwidth and vo_dheight were actually ever used). The aspect_save_prescale indeed disappeared, some vos used it, some didn't, it seemed like nonsense to me, and without effect in addition since aspect is not used with non-rootwin -wid. Greetings, Reimar Döffinger
On Sun, Nov 16, 2008 at 07:07:43PM +0100, Reimar Döffinger wrote:
The aspect_save_prescale indeed disappeared, some vos used it, some didn't, it seemed like nonsense to me, and without effect in addition since aspect is not used with non-rootwin -wid.
Note that I do not mind splitting the patch a bit, but I'd prefer to know nobody will object the overall idea and preferably also how you/whoever would like it to be split. Greetings, Reimar Döffinger
On 11/16/08, Reimar Döffinger <Reimar.Doeffinger@stud.uni-karlsruhe.de> wrote:
On Sun, Nov 16, 2008 at 07:58:14PM +0200, Ivan Kalvachev wrote:
On 11/16/08, Reimar Döffinger <Reimar.Doeffinger@stud.uni-karlsruhe.de> wrote:
Hello, attached patch gets rid of a large part of massive code duplication. The reason I am sending this as a patch first is that I can't test most -vos, though I do not think anything will break besides where the behaviour was previously inconsistent between vos. Since I am almost the only one working on libvo stuff I will only give a few days to test/object though. In a next step I will probably move the CONFIG_XF86VM stuff, too, though that will have the side effect that for vos that do not support it, when -vm is specified on the command line it will no longer be ignored but instead break things completely - IMO this is acceptable.
The following code vanishes without explanation:
- XGetGeometry(mDisplay, vo_window, &mRoot, - &drwX, &drwY, &vo_dwidth, &vo_dheight, - &drwBorderWidth, &drwDepth); - aspect_save_prescale(vo_dwidth, vo_dheight);
Not really, it is in x11_common.c, the line if (WinID) vo_x11_update_geometry(); (note that of the values set here only vo_dwidth and vo_dheight were actually ever used). The aspect_save_prescale indeed disappeared, some vos used it, some didn't, it seemed like nonsense to me, and without effect in addition since aspect is not used with non-rootwin -wid.
OK, here is my theory, I may be wrong of course. To preserve aspect when mplayer creates its own window, it uses the window manager to keep it at requested size. So we don't have to worry about keycolor getting outside overlay region and clearing black bars, unless we are in fullscreen mode. However when we use window of another program we can't resize it, so instead we get its dimensions and draw the video image so it fits in the existing window while preserving the correct aspect. Of course, I think using same behavior would be welcome for mplayer created windows, when using "strange" window managers. Not mentioning the "know" issues that mplayer uses old values after resolution changes (both ctrl+alt+plus/minus and randr)
On Sun, Nov 16, 2008 at 09:08:41PM +0200, Ivan Kalvachev wrote:
However when we use window of another program we can't resize it, so instead we get its dimensions and draw the video image so it fits in the existing window while preserving the correct aspect.
No, that behaviour is only used for the root window (and even there inconsistently). Aspect is not preserved with -wid, the whole window is filled - also the code stored the -wid window aspect as movie aspect, which makes it make even less sense. Making this behaviour optional (re-/mis-using the -fs flag for that was my idea, though something that works for MPlayer created windows is nice, too) sounds useful, but as long as the code is duplicate in about 7 places with slight variations each time it is just not feasible.
Not mentioning the "know" issues that mplayer uses old values after resolution changes (both ctrl+alt+plus/minus and randr)
To my knowledge MPlayer is only aware of the virtual desktop resolution and does not care what resolution is actually on screen, so this not really a question of using "old" values but just generally wrong ones (luckily they are rarely wrong in practice). The "old values" problem does exist when moving between xinerama screens though, it might not be that hard to solve but since things work fine for LCDs it seems questionable if it's worth the effort of fixing it. Greetings, Reimar Döffinger
participants (2)
-
Ivan Kalvachev -
Reimar Döffinger