From: Stas S. <st...@ak...> - 2007-05-18 17:40:17
Attachments:
stdond.diff
|
Hi. Since I still don't know where's the patch tracker is, I'm going to spam this list for just a little more. :) Right now xine will redirect the stdout to /dev/null, which is highly undesireable in the stdctl mode. I actually think is is never desireable and should not be done at all (there are much better ways to handle the debug output), but the attached patch is the minimal possible change that fixes the problem. |
From: Hans-Dieter K. <hd...@t-...> - 2007-05-22 23:08:43
|
Hi Stas, Stas Sergeev wrote: > Hi. > > Since I still don't know where's the > patch tracker is, I'm going to spam this > list for just a little more. :) xine-ui has no patch tracker, so the list is the right way. Folks, please correct me if I'm telling something wrong. > > Right now xine will redirect the stdout > to /dev/null, which is highly undesireable > in the stdctl mode. I actually think is > is never desireable and should not be done > at all (there are much better ways to handle > the debug output), but the attached patch is > the minimal possible change that fixes the > problem. That's an item we probably can discuss about. But anyway, in order to not force verbose output with stdctl output, I suggest following approach: Use a separate filepointer for stdctl redirected to original standard output if not verbose; use original stdout in verbose mode. Approximately like this: if(!gGui->verbosity) { int fd = dup(STDOUT_FILENO); FILE *fp = fdopen(fd, "w" /* or "a" */); gGui->stdctl_outfp = fp; /* - this is a new struct member - */ /* Now redirect stdout to /dev/null */ } else { gGui->stdctl_outfp = stdout; } Then, all stdctl outputs should use fprintf(gGui->stdctl_outfp, ...). What do you think, would you give it a try? PS: I'll review all your other postings and apply as appropriate. I'll drop a note. Thanks for your contribution. Cheers, Hans-Dieter > > > ------------------------------------------------------------------------ > > --- xine-ui/src/xitk/main.c.old1 2007-05-17 15:42:03.000000000 +0400 > +++ xine-ui/src/xitk/main.c 2007-05-18 17:55:26.000000000 +0400 > @@ -1902,7 +1902,7 @@ > show_banner(); > > #ifndef DEBUG > - if(!gGui->verbosity) { > + if(!gGui->verbosity && !gGui->stdctl_enable) { > int error_fd; > > if ((error_fd = open("/dev/null", O_WRONLY)) < 0) |
From: Stas S. <st...@ak...> - 2007-05-23 17:49:28
Attachments:
stdmcom1.diff
|
Hi. Hans-Dieter Kosch wrote: > xine-ui has no patch tracker, so the list is the right way. OK, I mostly thought this is the case. Just wondering why so. > But anyway, in order to not force verbose output with stdctl output, I > suggest following approach: Use a separate filepointer for stdctl > redirected to original standard output if not verbose; use original > stdout in verbose mode. I like this approach more than my check. Except that I'd rather duplicate an FD unconditionally, no matter of the verbosity. My opinion is that the debugging output should never affect the code flow. Actually, I had a lot of the headache before created this patch. The reason is that xine-ui redirects stdout to /dev/null only if you build it without --enable-debug for configure. So the debug build worked perfectly for me, while the non-debugging didn't. It is very bad when that happens and I've wasted a lot of time tracking down that seemingly simple problem. So I'd rather not make anything depend on a verbosity level or --enable-debug or the like. And IIRC xine-ui forgets to use -g flag when you specify --enable-debug. But IMHO yet the cleaner approach would be to change xine-ui to print the debug info into stderr, not stdout. This is more of a changes, but do you think it is better than the above? > What do you think, would you give it a try? Sure, but first I'd like the following patches: http://sourceforge.net/mailarchive/forum.php?thread_name=464C9B4E.8020408%40aknet.ru&forum_name=xine-devel http://sourceforge.net/mailarchive/forum.php?thread_name=464C9C2E.4000902%40aknet.ru&forum_name=xine-devel to be dealt with, otherwise there would be a clash. > PS: I'll review all your other postings and apply as appropriate. I'll > drop a note. Thanks, thats a good news. Btw, I updated one of them to not involve an extra buffer. Without the patch tracker, I can only attach it to this mail and point an URL to the patch which it obsoletes. But I could really do with the tracker here! :) Attached patch obsoletes this one: http://sourceforge.net/mailarchive/forum.php?thread_name=464C9C2E.4000902%40aknet.ru&forum_name=xine-devel |
From: Hans-Dieter K. <hd...@t-...> - 2007-05-24 00:11:16
|
Hi, Stas Sergeev wrote: > Hi. > > Hans-Dieter Kosch wrote: > >> xine-ui has no patch tracker, so the list is the right way. > > OK, I mostly thought this is the case. > Just wondering why so. Don't know, I'm not a project admin. However, I think the list is a more communicative way to post patches and discuss them. >> But anyway, in order to not force verbose output with stdctl output, I >> suggest following approach: Use a separate filepointer for stdctl >> redirected to original standard output if not verbose; use original >> stdout in verbose mode. > > I like this approach more than my check. > Except that I'd rather duplicate an FD > unconditionally, no matter of the verbosity. > My opinion is that the debugging output should > never affect the code flow. I suggested this conditional to prevent asynchronously mixed output because of independent buffering in the separate streams. Alternatively we could use unbuffered streams which is less efficient. > Actually, I had a lot of the headache before > created this patch. The reason is that xine-ui > redirects stdout to /dev/null only if you build > it without --enable-debug for configure. So the > debug build worked perfectly for me, while the > non-debugging didn't. It is very bad when that > happens and I've wasted a lot of time tracking > down that seemingly simple problem. > So I'd rather not make anything depend on a > verbosity level or --enable-debug or the like. > And IIRC xine-ui forgets to use -g flag when > you specify --enable-debug. This depends if we only want DEBUG defined or also have gdb info, but it seems reasonable to have the -g flag. > But IMHO yet the cleaner approach would be to > change xine-ui to print the debug info into stderr, > not stdout. This is more of a changes, but do you > think it is better than the above? Agreed. I suggest to use --enable-debug for developer debugging and to use gGui->verbosity for user debugging and to write it all to stderr. This requires to work through the whole code. Who has time for it :-( >> What do you think, would you give it a try? > > Sure, but first I'd like the following patches: > http://sourceforge.net/mailarchive/forum.php?thread_name=464C9B4E.8020408%40aknet.ru&forum_name=xine-devel > > http://sourceforge.net/mailarchive/forum.php?thread_name=464C9C2E.4000902%40aknet.ru&forum_name=xine-devel > > to be dealt with, otherwise there would be a > clash. OK. Nevertheless, we should implement your patch collection in one consistent step if possible. >> PS: I'll review all your other postings and apply as appropriate. I'll >> drop a note. > > Thanks, thats a good news. > Btw, I updated one of them to not involve an > extra buffer. Great, this was also my idea :-) Without the patch tracker, I can > only attach it to this mail and point an URL to > the patch which it obsoletes. > But I could really do with the tracker here! :) No problem, I always use the most recent patch and I don't rely just on the patch command but always review and probably correct it manually. > Attached patch obsoletes this one: > http://sourceforge.net/mailarchive/forum.php?thread_name=464C9C2E.4000902%40aknet.ru&forum_name=xine-devel Very thorough! Cheers, Hans-Dieter > > ------------------------------------------------------------------------ > > --- xine-ui/src/xitk/stdctl.c.old1 2007-05-17 16:35:43.000000000 +0400 > +++ xine-ui/src/xitk/stdctl.c 2007-05-17 19:00:04.000000000 +0400 > @@ -35,7 +35,7 @@ > > #include "common.h" > > -#undef DEBUG_STDCTL > +#define DEBUG_STDCTL 0 > > extern gGui_t *gGui; > extern _panel_t *panel; > @@ -48,7 +48,7 @@ > static _stdctl_t stdctl; > > static void *xine_stdctl_loop(void *dummy) { > - char c[255]; > + char buf[255], *c, *pos, *pos1; > int len, selrt; > kbinding_entry_t *k; > fd_set set; > @@ -71,66 +71,73 @@ > > if(selrt > 0 && FD_ISSET(stdctl.fd, &set)) { > > - len = read(stdctl.fd, &c, 255); > + len = read(stdctl.fd, buf, sizeof(buf) - 1); > > if(len > 0) { > > - c[len - 1] = '\0'; > - > -#ifdef DEBUG_STDCTL > - fprintf(stdout, "Command Received = '%s'\n", c); > + buf[len] = 0; > + > + pos1 = buf; > + while ((pos = strchr(pos1, '\n'))) { > + *pos = 0; > + c = pos1; > + pos1 = pos + 1; > + > +#if DEBUG_STDCTL > + fprintf(stdout, "Command Received = '%s'\n", c); > #endif > > - /* Handle optional parameter */ > + /* Handle optional parameter */ > > - /* alphanum: separated from the command by a '$' */ > - /* syntax: "command$parameter" */ > - /* example: "OSDWriteText$Some Information to Display" */ > + /* alphanum: separated from the command by a '$' */ > + /* syntax: "command$parameter" */ > + /* example: "OSDWriteText$Some Information to Display" */ > > - gGui->alphanum.set = 0; > - params = strchr(c, '$'); > + gGui->alphanum.set = 0; > + params = strchr(c, '$'); > > - if(params != NULL) { > + if(params != NULL) { > > - /* parameters available */ > + /* parameters available */ > > - *params = '\0'; > - params++; > + *params = '\0'; > + params++; > > - gGui->alphanum.set = 1; > - gGui->alphanum.arg = params; > + gGui->alphanum.set = 1; > + gGui->alphanum.arg = params; > > -#ifdef DEBUG_STDCTL > - fprintf(stdout, "Command: '%s'\nParameters: '%s'\n", c, params); > +#if DEBUG_STDCTL > + fprintf(stdout, "Command: '%s'\nParameters: '%s'\n", c, params); > #endif > - } > + } > > - /* numeric: separated from the command by a '#' */ > - /* syntax: "command#parameter" */ > - /* example: "SetPosition%#99" */ > + /* numeric: separated from the command by a '#' */ > + /* syntax: "command#parameter" */ > + /* example: "SetPosition%#99" */ > > - gGui->numeric.set = 0; > - params = strchr(c, '#'); > + gGui->numeric.set = 0; > + params = strchr(c, '#'); > > - if(params != NULL) { > + if(params != NULL) { > > - /* parameters available */ > + /* parameters available */ > > - *params = '\0'; > - params++; > + *params = '\0'; > + params++; > > - gGui->numeric.set = 1; > - gGui->numeric.arg = atoi(params); > + gGui->numeric.set = 1; > + gGui->numeric.arg = atoi(params); > > -#ifdef DEBUG_STDCTL > - fprintf(stdout, "Command: '%s'\nParameters: '%d'\n", c, gGui->numeric.arg); > +#if DEBUG_STDCTL > + fprintf(stdout, "Command: '%s'\nParameters: '%d'\n", c, gGui->numeric.arg); > #endif > - } > + } > > - k = kbindings_lookup_action(gGui->kbindings, c); > + k = kbindings_lookup_action(gGui->kbindings, c); > > - if(k) > - gui_execute_action_id((kbindings_get_action_id(k))); > + if(k) > + gui_execute_action_id((kbindings_get_action_id(k))); > + } > } > > if(panel_is_visible()) |
From: Stas S. <st...@ak...> - 2007-05-24 18:19:58
Attachments:
confg.diff
|
Hi. Hans-Dieter Kosch wrote: > I suggested this conditional to prevent asynchronously mixed output > because of independent buffering in the separate streams. Yes, I see. OK, in this case I'll weight the amount of work needed to get the debug output to stderr entirely. Doesn't look like too much in fact. :) For some reasons the code flow dependancy on a verbosity level, gives a rather large penalty to the proposed approach in my eyes. >> And IIRC xine-ui forgets to use -g flag when >> you specify --enable-debug. > This depends if we only want DEBUG defined or also have gdb info, but it > seems reasonable to have the -g flag. Attached is the patch that does this. I also removed NDEBUG - it seems to be unused, and even if not - the one should just use "#ifndef DEBUG" instead IMHO. |
From: Hans-Dieter K. <hd...@t-...> - 2007-05-24 22:08:44
|
Hi, Stas Sergeev wrote: > Hi. > > Hans-Dieter Kosch wrote: > >> I suggested this conditional to prevent asynchronously mixed output >> because of independent buffering in the separate streams. > > Yes, I see. OK, in this case I'll weight the amount > of work needed to get the debug output to stderr > entirely. Doesn't look like too much in fact. :) > For some reasons the code flow dependancy on a > verbosity level, gives a rather large penalty to > the proposed approach in my eyes. > How shall we proceed? Could you work it out? My time is once more quite limited at the moment... >>> And IIRC xine-ui forgets to use -g flag when >>> you specify --enable-debug. >> >> This depends if we only want DEBUG defined or also have gdb info, but >> it seems reasonable to have the -g flag. > > Attached is the patch that does this. > I also removed NDEBUG - it seems to be unused, and > even if not - the one should just use "#ifndef DEBUG" > instead IMHO. > That looks OK, besides the fact that we can then leave out the else clause as it doesn't change anything, I'll remember it for committing :-) > > ------------------------------------------------------------------------ > > --- xine-ui/configure.ac.old 2007-05-16 22:34:56.000000000 +0400 > +++ xine-ui/configure.ac 2007-05-24 12:40:09.000000000 +0400 > @@ -527,12 +527,12 @@ > AC_SUBST(CFLAGS) > > AC_ARG_ENABLE([debug], > - AS_HELP_STRING([--enable-debug], [Enable debugging code])) > + AS_HELP_STRING([--enable-debug], [Compile with debug information])) > > if test "x$enable_debug" = "xyes"; then > - CFLAGS="$CFLAGS -DDEBUG" > + CFLAGS="$CFLAGS -DDEBUG -g" > else > - CFLAGS="$CFLAGS -DNDEBUG" > + CFLAGS="$CFLAGS" > fi > > dnl gcc __attribute__ ((aligned ())) > > > ------------------------------------------------------------------------ |
From: Stas S. <st...@ak...> - 2007-05-25 04:30:38
|
Hi. Hans-Dieter Kosch wrote: > How shall we proceed? Could you work it out? Yes, I'll see about changing all the printf's to stderr this weekend. But the patches I already posted, are mostly very small - would it be possible to proceed with them for now? This will avoid clashes. > That looks OK, besides the fact that we can then leave out the else > clause as it doesn't change anything Ouch sorry. |
From: Hans-Dieter K. <hd...@t-...> - 2007-05-25 21:45:30
|
Hi, Stas Sergeev wrote: > Hans-Dieter Kosch wrote: > >>How shall we proceed? Could you work it out? > > Yes, I'll see about changing all the printf's > to stderr this weekend. > But the patches I already posted, are mostly > very small - would it be possible to proceed > with them for now? This will avoid clashes. > Fine :-) I'll apply the patches meanwhile. Cheers, Hans-Dieter |
From: Hans-Dieter K. <hd...@t-...> - 2007-05-28 00:23:16
|
Hi, Stas Sergeev wrote: > Hans-Dieter Kosch wrote: > >>How shall we proceed? Could you work it out? > > Yes, I'll see about changing all the printf's > to stderr this weekend. > But the patches I already posted, are mostly > very small - would it be possible to proceed > with them for now? This will avoid clashes. > All patches committed, except those for main.c; I'd like to wait until it's clear how we go on with debug and the --verbose option. Cheers, Hans-Dieter |
From: Stas S. <st...@ak...> - 2007-05-28 17:39:52
|
Hello. Hans-Dieter Kosch wrote: > All patches committed, You missed the rdtsc patches it seems. They are needed on x86_64, as otherwise the xine would just crash on startup. Since the SF mailarchive doesn't seem to work for me right now, I am attaching them to this message. > except those for main.c; I'd like to wait until > it's clear how we go on with debug and the --verbose option. Thanks. As promised, I tried to make the debugging output go to stderr. This is really something that can't be done in an hour... I think I located most of the places, but someone really have to double-check this stuff. Can we start from something like the attached patch? Btw, I can post all the patches in a separate mails if you prefer. |
From: Darren S. <li...@yo...> - 2007-05-28 18:17:50
|
I demand that Stas Sergeev may or may not have written... > Hans-Dieter Kosch wrote: >> All patches committed, > You missed the rdtsc patches it seems. They are needed on x86_64, as > otherwise the xine would just crash on startup. They are? It would? Works for me without these... > Since the SF mailarchive doesn't seem to work for me right now, I am > attaching them to this message. [snip] > Btw, I can post all the patches in a separate mails if you prefer. Well... that or add a one-line summary and, optionally, a description to each. The ideal situation is that we can use 'hg import' to apply the patch and automatically have the correct author and datestamp information without providing hg with anything more than the file containing the patch. This tends to work well with one patch per message or with patches which have been generated by 'hg export'. -- | Darren Salt | linux or ds at | nr. Ashington, | Toon | RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army | + Use more efficient products. Use less. BE MORE ENERGY EFFICIENT. Bad command or file name. Go and stand in the corner. |
From: Stas S. <st...@ak...> - 2007-05-28 18:56:26
|
Hello. Darren Salt wrote: > I demand that Stas Sergeev may or may not have written... Please, what does this mean? >> You missed the rdtsc patches it seems. They are needed on x86_64, as >> otherwise the xine would just crash on startup. > They are? Sure. The "A" constraint on x86_64 seems to use the rax:rdx regs, while the rdtsc works exactly the way the x86 version works, that is, uses eax:edx. On x86_64, gcc moves only rdx to the "x" var. > It would? Works for me without these... I would post an assembly code here, but now as Diego have confirmed the crash, I think this is not necessary. > Well... that or add a one-line summary and, optionally, a description to > each. If you really want this, then why not there is a patch tracker yet? :) I of course can do what you say, but I wonder what will happen if I post, say, 10 patches in 10 separate mails, then post an updated versions on 6 of them in 6 mails more, and then another few updates. OK, nevermind. I'll try right now with what's remained. > The ideal situation is that we can use 'hg import' Well, AFAICS xine-ui is not in a hg, only xine-lib is. So I don't think I can do that, or...? Btw, I am having a problem with current xine-lib code: the colors are completely messed up while playing all videos (mpeg4). They all look greenish and weird. And on the xine-0.99.5 which comes in Fedora Extras, everything is fine. Just a quick query if maybe this is a known problem, or something on my side is wrong... |
From: Darren S. <li...@yo...> - 2007-05-28 21:22:59
|
I demand that Stas Sergeev may or may not have written... > Darren Salt wrote: >> I demand that Stas Sergeev may or may not have written... > Please, what does this mean? Read "The Hitch-Hiker's Guide to the Galaxy" and you may - or may not! - be enlightened. >>> You missed the rdtsc patches it seems. They are needed on x86_64, as >>> otherwise the xine would just crash on startup. >> They are? > Sure. The "A" constraint on x86_64 seems to use the rax:rdx regs, while the > rdtsc works exactly the way the x86 version works, that is, uses eax:edx. > On x86_64, gcc moves only rdx to the "x" var. Right... >> It would? Works for me without these... > I would post an assembly code here, but now as Diego have confirmed the > crash, I think this is not necessary. Fair enough... >> Well... that or add a one-line summary and, optionally, a description to >> each. > If you really want this, then why not there is a patch tracker yet? :) You can use the bug tracker for this; the same kind of thing applies there. However, it's less convenient: it's necessary to visit the bug tracker because attachments are not mailed to the bugs list. > I of course can do what you say, but I wonder what will happen if I post, > say, 10 patches in 10 separate mails, then post an updated versions on 6 of > them in 6 mails more, and then another few updates. [...] It can become confusing, certainly :-) But if you send it as a follow-up to the replaced patch, that should help; so long as the subject describes the patch and the start of the patch description is clear, I don't see any particular problem - unless the patch which it's replacing has been committed... >> The ideal situation is that we can use 'hg import' > Well, AFAICS xine-ui is not in a hg, only xine-lib is. So I don't think I > can do that, or...? True. However, I'm only concerned with the xine-lib patches. > Btw, I am having a problem with current xine-lib code: the colors are > completely messed up while playing all videos (mpeg4). Which branch? Internal or external ffmpeg? If external, what revision no.? Also, a sample video may help; if you're using any video post-plugins, switch them off or play with their configurations to see if that helps. (For example, I see overflow errors in the h1 and v1 filters in the 'pp' post-plugin - some pixels which should be black are being shown as white. OTOH, this error is documented in the source...) > They all look greenish and weird. And on the xine-0.99.5 which comes in > Fedora Extras, everything is fine. Hmm. Presumably it's using Fedora-supplied xine-lib 1.1.6...? > Just a quick query if maybe this is a known problem, or something on my > side is wrong... Right now, I couldn't say :-) -- | Darren Salt | linux or ds at | nr. Ashington, | Toon | RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army | + Use more efficient products. Use less. BE MORE ENERGY EFFICIENT. Democracy is based on the theorem that many men are smarter than one. |
From: Stas S. <st...@ak...> - 2007-05-29 17:35:22
|
Hello. Darren Salt wrote: > However, it's less convenient: it's necessary to visit the bug tracker > because attachments are not mailed to the bugs list. You should really re-check that, SF evolves. Even if the comment was not added, the SF tracker can now generate a message about the attachment. But adding the attachment without a comment is never a good practice anyway. >> Btw, I am having a problem with current xine-lib code: the colors are >> completely messed up while playing all videos (mpeg4). > Which branch? Internal or external ffmpeg? If external, what revision no.? Internal. > Also, a sample video may help; Any will do. visuals/default.avi for example. > Hmm. Presumably it's using Fedora-supplied xine-lib 1.1.6...? No, this doesn't seem to be the case. I uninstalled everything xine-related, and doublechecked with "ldd" which libs are used. > Right now, I couldn't say :-) I wanted to make a screenshot for you to demonstrate the problem. So I pressed "t" and what??? The screenshot had the correct colours! How can this be? But then I made a screenshot with an external program, and now it is "honest". What can this be after all? Why the screenshots made with "t" do not match whatever is displayed? Here are they: ftp://nostalgie.no-ip.biz/good.png ftp://nostalgie.no-ip.biz/bad.png I attached them in the previous message, but it was held by the list because of the size limit - should be deleted by the list admin. |
From: Darren S. <li...@yo...> - 2007-05-31 15:54:19
|
I demand that Stas Sergeev may or may not have written... > Darren Salt wrote: >> However, it's less convenient: it's necessary to visit the bug tracker >> because attachments are not mailed to the bugs list. > You should really re-check that, SF evolves. Even if the comment was not > added, the SF tracker can now generate a message about the attachment. Yes. About it. Not including it. [snip] >>> Btw, I am having a problem with current xine-lib code: the colors are >>> completely messed up while playing all videos (mpeg4). [snip] >> Also, a sample video may help; > Any will do. visuals/default.avi for example. That's mpeg2... [snip] >> Right now, I couldn't say :-) > I wanted to make a screenshot for you to demonstrate the problem. So I > pressed "t" and what??? The screenshot had the correct colours! How can > this be? Because the screenshot is created by copying and processing the current frame from the stream. [snip] > Here are they: > ftp://nostalgie.no-ip.biz/good.png > ftp://nostalgie.no-ip.biz/bad.png This looks like a mismatch between whichever video output plugin you're using and what the graphics driver expects. [snip] -- | Darren Salt | linux or ds at | nr. Ashington, | Toon | RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army | + Use more efficient products. Use less. BE MORE ENERGY EFFICIENT. Why is it called tourist season if we can't shoot them? |
From: Stas S. <st...@ak...> - 2007-06-07 18:22:22
|
Hello. Darren Salt wrote: > This looks like a mismatch between whichever video output plugin you're using > and what the graphics driver expects. Could you please verify whether or not does it happen for you if you set the vo driver to "opengl", and an OpenGL Renderer to "2D_Tex_Fragprog"? And I've finally found out why all the other video drivers didn't work for me. This is because those misterious settings like vo_contrast/vo_saturation which are available only in a "Debugger" expirience level. They seem to be completely broken. Config says they have a default value of -1, but it doesn't look possible to set it via GUI. So once you just touched one of these controls, you can't bring them back to the old value (-1) other than by editing the config by hands. And since they are not available in the GUI at all in most cases ("Debugger" level is unavailable in the pre-built rpms), the one can hardly suspect them in case of a failure. Also, the "Apply" button does not seem to work for them, neither does the "Ok" button, and the warning that the xine have to be restarted, is not emitted either. So basically, once you touched these controls, your xine is non-functional, and the only way to fix it, is to edit the config by hands. Something must be done about that... |
From: Darren S. <li...@yo...> - 2007-06-08 14:47:27
|
I demand that Stas Sergeev may or may not have written... > Darren Salt wrote: >> This looks like a mismatch between whichever video output plugin you're >> using and what the graphics driver expects. > Could you please verify whether or not does it happen for you if you set > the vo driver to "opengl", and an OpenGL Renderer to "2D_Tex_Fragprog"? That works fine on my laptop (i945). For some reason the opengl driver doesn't want to work on my desktop box (not that it matters, since I don't think that that particular renderer works on R200-series cards). Unfortunately, the image is rendered above windows which are above the video window... :-| However, you may be able to hack this: just after this comment in src/video_out/video_out_opengl.c: /* TODO: Use GL_BGRA extension check for dynamically changing this */ there's a "#if 1" which you could try changing to "#if 0". (Hmm, nice. Xv output interacts *very* well with the composite extension, at least with the new Intel driver...) > And I've finally found out why all the other video drivers didn't work for > me. This is because those misterious settings like > vo_contrast/vo_saturation which are available only in a "Debugger" > experience level. Hmm? They're available as sliders... > They seem to be completely broken. Config says they have a default value of > -1, but it doesn't look possible to set it via GUI. So once you just > touched one of these controls, you can't bring them back to the old value > (-1) other than by editing the config by hands. The sliders work properly in gxine, though they have no effect with that particular renderer. With the others, only the hue setting has no effect. (i945 again.) [snip] -- | Darren Salt | linux or ds at | nr. Ashington, | Toon | RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army | + Use more efficient products. Use less. BE MORE ENERGY EFFICIENT. The perfect guest is one who makes his host feel at home. |
From: Stas S. <st...@ak...> - 2007-06-08 16:51:58
|
Hello. Darren Salt wrote: > However, you may be able to hack this: just after this comment in > src/video_out/video_out_opengl.c: > /* TODO: Use GL_BGRA extension check for dynamically changing this */ > there's a "#if 1" which you could try changing to "#if 0". That doesn't help. :( >> And I've finally found out why all the other video drivers didn't work for >> me. This is because those misterious settings like >> vo_contrast/vo_saturation which are available only in a "Debugger" >> experience level. > Hmm? They're available as sliders... On which experience_level? For me it is available only for the "Debugger" one, which is usually unavailable itself. > The sliders work properly in gxine, though they have no effect with that > particular renderer. That's why it kept working for me (with the wrong colours though), while the others did not at all. Why is it unaffected btw? SDL is also unaffected, correct? But again, their default values seem to be (-1) according to the config comments. How do you set them to -1 with the sliders? For me, once I touch the slider, I can never set them back to -1, and so the xine no longer works at all! And do you say that the Apply or the Ok button work for these settings properly? |
From: Darren S. <li...@yo...> - 2007-06-08 18:33:51
|
I demand that Stas Sergeev may or may not have written... > Darren Salt wrote: >> However, you may be able to hack this: just after this comment in >> src/video_out/video_out_opengl.c: >> /* TODO: Use GL_BGRA extension check for dynamically changing this */ >> there's a "#if 1" which you could try changing to "#if 0". > That doesn't help. :( So it wouldn't be resolved with a RGB/BGR switch. In which case, I currently have no more ideas. >>> And I've finally found out why all the other video drivers didn't work >>> for me. This is because those misterious settings like >>> vo_contrast/vo_saturation which are available only in a "Debugger" >>> experience level. >> Hmm? They're available as sliders... > On which experience_level? [...] All. You're looking in the wrong place... gxine - the icon to the left of the speaker icon, or Video->Settings. xine - centre-left icon (tooltip "Control") or Settings->Video. >> The sliders work properly in gxine, though they have no effect with that >> particular renderer. > That's why it kept working for me (with the wrong colours though), while > the others did not at all. Which is... odd. xvinfo output may be useful. > Why is it unaffected btw? SDL is also unaffected, correct? That's not working on my desktop box either - hmm, not quite true: it works with xine-ui but not gxine. xine-ui uses X11 whereas my current gxine build uses XCB... [snip] -- | Darren Salt | linux or ds at | nr. Ashington, | Toon | RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army | + Use more efficient products. Use less. BE MORE ENERGY EFFICIENT. CChheecckk yyoouurr dduupplleexx sswwiittcchh.. |
From: Stas S. <st...@ak...> - 2007-06-08 19:36:04
Attachments:
contrs.diff
|
Hello. Darren Salt wrote: > So it wouldn't be resolved with a RGB/BGR switch. In which case, I currently > have no more ideas. That's not so much of a problem now, when I found out the way to get the other VO plugins to work. > All. You're looking in the wrong place... Yes, thanks for the clarifications. However, could you (or anyone else) please at least apply the attached patch? It fixes the biggest part of the problem in that wrong place I was looking them in. I.e. if they appear at the wrong place (for what, btw?) then at least they should not be that broken there. >> That's why it kept working for me (with the wrong colours though), while >> the others did not at all. > Which is... odd. xvinfo output may be useful. But that was because I have touched the sliders "at the wrong place", nothing more. Which is hopefully what the attached patch can address. Apparently the sliders are not aware that -1 == 32768. |
From: Hans-Dieter K. <hd...@t-...> - 2007-06-09 00:52:24
|
Hi, Stas Sergeev wrote: > > However, could you (or anyone else) please at > least apply the attached patch? It fixes the > biggest part of the problem in that wrong place > I was looking them in. OK, I'll apply. > > But that was because I have touched the sliders > "at the wrong place", nothing more. Which is > hopefully what the attached patch can address. > Apparently the sliders are not aware that -1 == 32768. BTW: -1 is *never* 32768 in a 32bit integer, so your patch is reasonable as it sets the default to the center value. Maybe -1 had originally the meaning of undefined or so... Cheers, Hans-Dieter > > > ------------------------------------------------------------------------ > > --- xine-ui/src/xitk/control.c.old 2007-06-04 08:27:24.000000000 +0400 > +++ xine-ui/src/xitk/control.c 2007-06-08 23:16:22.000000000 +0400 > @@ -305,7 +305,7 @@ > if(hue_ena) { > gGui->video_settings.hue = > xine_config_register_range(gGui->xine, "gui.vo_hue", > - -1, > + 32768, > 0, 65535, > _("hue value"), > _("Hue value."), > @@ -321,7 +321,7 @@ > if(bright_ena) { > gGui->video_settings.brightness = > xine_config_register_range(gGui->xine, "gui.vo_brightness", > - -1, > + 32768, > 0, 65535, > _("brightness value"), > _("Brightness value."), > @@ -337,7 +337,7 @@ > if(sat_ena) { > gGui->video_settings.saturation = > xine_config_register_range(gGui->xine, "gui.vo_saturation", > - -1, > + 32768, > 0, 65535, > _("saturation value"), > _("Saturation value."), > @@ -353,7 +353,7 @@ > if(contr_ena) { > gGui->video_settings.contrast = > xine_config_register_range(gGui->xine, "gui.vo_contrast", > - -1, > + 32768, > 0, 65535, > _("contrast value"), > _("Contrast value."), > |
From: Hans-Dieter K. <hd...@t-...> - 2007-06-13 00:39:52
|
Hi Stas, I myself wrote: > Hi, > > Stas Sergeev wrote: > >> >> However, could you (or anyone else) please at >> least apply the attached patch? It fixes the >> biggest part of the problem in that wrong place >> I was looking them in. > > OK, I'll apply. > Not yet applied, see below. >> >> But that was because I have touched the sliders >> "at the wrong place", nothing more. Which is >> hopefully what the attached patch can address. >> Apparently the sliders are not aware that -1 == 32768. > > BTW: -1 is *never* 32768 in a 32bit integer, so your patch is reasonable > as it sets the default to the center value. Maybe -1 had originally the > meaning of undefined or so... > I've learned a bit more meanwhile. The -1 means to get a default value from xine-lib, as one of the code fragments from control.c shows, which follow immediately the settings to 32768 you proposed: if( gGui->video_settings.hue < 0 ) /* i.e. -1; no preconfigured value */ /* dbg */ { gGui->video_settings.hue = get_current_param(XINE_PARAM_VO_HUE); /* dbg */ fprintf(stderr, "***HUE: %d\n", gGui->video_settings.hue); /* dbg */ } else /* i.e. we have a preconfigured value */ set_current_param(XINE_PARAM_VO_HUE,gGui->video_settings.hue); In my tests, all gGui->video_settings.* got values around 32768 +/- some granularity after this initialization. So, the problem appears to be related to the vo drivers, not to the sliders. Could you please check the returned values like in the above example? I suspect, setting 32768 is a quick workaround but not the solution. Hint: To achieve these default conditions, comment out the respective entries in your $HOME/.xine/config file before starting xine-ui. Cheers, Hans-Dieter |
From: Stas S. <st...@ak...> - 2007-06-14 16:52:36
Attachments:
voctrfix.diff
|
Hello. Hans-Dieter Kosch wrote: > I've learned a bit more meanwhile. The -1 means to get a default value > from xine-lib, as one of the code fragments from control.c shows, which > follow immediately the settings to 32768 you proposed: Right, I could have noticed that myself too. Attached is the better patch then. It looks quite correct, but still there is a small pitfall, which is that now the options would change -1->32768 even if you have not touched the sliders at all. Whether it is good or bad I don't know. I personally think it is acceptable, but maybe you know how to avoid also that. :) |
From: Stas S. <st...@ak...> - 2007-06-20 04:43:34
|
Hi Hans. Hans-Dieter Kosch wrote: > You used the debug sliders in the setup window which are not available > to the normal user (with all their problems you mentioned, which may or > may not be solved). Yes, they are available only to the "Debuggers", but it doesn't mean they are not a problem. Any user can became a "Debugger" if he just wants to fix a couple of bugs and add a few missing features (my case). So the fact he build xine with debug info, doesn't yet mean he knows what he does. :) The result of these small bugs were that I was not able to run xine for almost a month, and only by a pure lack combined with lots of the patience, I've got it working again rather than to give up on it completely. :) > You need a way to reset > to the -1 defaults. Not quite. Its just that with my patch, the -1 will always be changed. Whether or not it is a problem, is what I don't know. But if we (you) declare it a problem, then what I need is so that the config options are not overwritten unless the sliders are actually touched by the user. > The control panel has a reset function in its menu > (right mouse click). This function currently sets the values to > (min+max)/2, i.e. 32767; would it solve the problem if we reset to the > -1 defaults in this place? I don't think so. Firstly, with my patch, the config value gets _always_ overwritten. It won't help to reset it to -1, as the next time it will be overwritten again. And secondly, it looks like the -1 is handled locally by xine-ui. Its not like it asks the xine-lib for the default value. So if you happened to already alter the value, then using -1 and quering xine-lib will probably just return that altered value. So I think resetting to (min+max)/2 is correct, unless you can query xine-lib itself for the default value. Note: I haven't verified this all yet, may be wrong. |
From: Hans-Dieter K. <hd...@t-...> - 2007-06-20 23:36:04
|
Hi Stas, Stas Sergeev wrote: > Hi Hans. > > Hans-Dieter Kosch wrote: > >>You used the debug sliders in the setup window which are not available >>to the normal user (with all their problems you mentioned, which may or >>may not be solved). > > Yes, they are available only to the "Debuggers", > but it doesn't mean they are not a problem. Any > user can became a "Debugger" if he just wants > to fix a couple of bugs and add a few missing > features (my case). So the fact he build xine > with debug info, doesn't yet mean he knows what > he does. :) The config entries with debug level have a normal user counterpart. E.g. look at the GUI window positions: they are set by moving the windows, not by some setup menu item. The reason for this level is that those entries are needed in the config file but should not appear in the setup menus via the setup menu system. Maybe we should change "debug" level to "no-direct-access" level, preventing access via the setup in any case. You should really forget about the debug video sliders in the setup, but use the control panel window instead. > The result of these small bugs were that I was > not able to run xine for almost a month, and > only by a pure lack combined with lots of the > patience, I've got it working again rather than > to give up on it completely. :) > I appreciate that :-) > >>You need a way to reset >>to the -1 defaults. > > Not quite. Its just that with my patch, the -1 > will always be changed. Whether or not it is a > problem, is what I don't know. But if we (you) > declare it a problem, then what I need is so > that the config options are not overwritten > unless the sliders are actually touched by the > user. > I don't know if I miss something, but at least in non-debug mode the config is not changed if you leave the sliders untouched. > >>The control panel has a reset function in its menu >>(right mouse click). This function currently sets the values to >>(min+max)/2, i.e. 32767; would it solve the problem if we reset to the >>-1 defaults in this place? > > I don't think so. Firstly, with my patch, the > config value gets _always_ overwritten. It won't > help to reset it to -1, as the next time it will > be overwritten again. And secondly, it looks like > the -1 is handled locally by xine-ui. Its not > like it asks the xine-lib for the default value. > So if you happened to already alter the value, > then using -1 and quering xine-lib will probably > just return that altered value. So I think resetting > to (min+max)/2 is correct, unless you can query > xine-lib itself for the default value. > Note: I haven't verified this all yet, may be wrong. > Test the attached patch. If I didn't totally misunderstand, this could be what you need. It queries the default config, remembers it, and restores it by selecting the "Reset Video Settings" control menu option (including the -1). IMHO this is better than resetting to a fixed center value anyway. Cheers, Hans-Dieter |