| Message ID | 1112217417.31596.17.camel@bobcat.mine.nu |
|---|---|
| State | New |
| Headers |
Received: from smtp2.pp.htv.fi ([213.243.153.35]) by www.linuxtv.org with esmtp (Exim 4.34) id 1DGkYP-0002hg-0a for vdr@linuxtv.org; Wed, 30 Mar 2005 23:16:45 +0200 Received: from cs78130200.pp.htv.fi (cs78130200.pp.htv.fi [62.78.130.200]) by smtp2.pp.htv.fi (Postfix) with ESMTP id 630C7296B47 for <vdr@linuxtv.org>; Thu, 31 Mar 2005 00:16:58 +0300 (EEST) Subject: Re: [vdr] Patch: dxr3plugin OSD don't turn pink From: Ville =?ISO-8859-1?Q?Skytt=E4?= <ville.skytta@iki.fi> To: vdr@linuxtv.org In-Reply-To: <424A9224.4000608@compuserve.de> References: <424A9224.4000608@compuserve.de> Content-Type: multipart/mixed; boundary="=-AHYjuNhJL3TgigYA5DC5" Date: Thu, 31 Mar 2005 00:16:57 +0300 Message-Id: <1112217417.31596.17.camel@bobcat.mine.nu> Mime-Version: 1.0 X-Mailer: Evolution 2.0.4 (2.0.4-2) X-BeenThere: vdr@linuxtv.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Klaus Schmidinger's VDR <vdr@linuxtv.org> List-Id: Klaus Schmidinger's VDR <vdr.linuxtv.org> List-Unsubscribe: <http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr>, <mailto:vdr-request@linuxtv.org?subject=unsubscribe> List-Archive: <http://www.linuxtv.org/pipermail/vdr> List-Post: <mailto:vdr@linuxtv.org> List-Help: <mailto:vdr-request@linuxtv.org?subject=help> List-Subscribe: <http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr>, <mailto:vdr-request@linuxtv.org?subject=subscribe> X-List-Received-Date: Wed, 30 Mar 2005 21:16:45 -0000 Status: O X-Status: X-Keywords: X-UID: 1282 |
Commit Message
Ville Skyttä
March 30, 2005, 9:16 p.m. UTC
On Wed, 2005-03-30 at 13:48 +0200, Martin Cap wrote: > Attached to this posting is a patch which fixes the odd problem making > the osd > turn pink after returning from mplayer or after hardware-reset of the > dxr3-card. > I posted this fix in a german-vdr-forum a couple of months ago and it > works for other users there, too. I haven't found the time to post it > here (other users hadn't either:) )... so, > here it is. Yep, works for me too, thanks. However, the patch gave me the creeps, so I dug a little deeper. I found that the only thing needed to fix the pink OSD here was to comment out the "m_users[i] == 0" test in AddColor() and unconditionally setting m_changed to true there, or to set it to true in GetIndex() when the color was found. Both of those were still ugly. Then I noticed that the counters in m_users were not being reset to zero when they IMO should have been, and came up with the attached patch which I think would be more appropriate. (Also applied to the vdr-dxr3-0-2 branch in CVS.) With this one applied, I no longer get pink OSD after returning from the MPlayer plugin. But after "Reset DXR3", the OSD is still momentarily pink immediately after; things return to normal when the OSD is hidden/shown again. Reviews/testing welcome...
Comments
Hi Ville, Well, even better. Works. Ville Skyttä wrote: > Reviews/testing welcome... >
Ville Skyttä wrote: > With this one applied, I no longer get pink OSD after returning from the > MPlayer plugin. But after "Reset DXR3", the OSD is still momentarily > pink immediately after; things return to normal when the OSD is > hidden/shown again. > > Reviews/testing welcome... Try to use the yaepg plugin: after that the normal osd (only with the st:tng skin) starts doing strange things here (reproducible), like discontinuous transparent background, a small rectangle black in the lower right corner of the channel name in the channel info display and thinner than normal selected lines. Without the patch it doesn't happen. Bye
Hi, how about the "original" patch I posted ? Does it happen there ? Regards,
Martin Cap wrote: > Hi, > > how about the "original" patch I posted ? Does it happen there ? No, it doesn't, but the femon plugin doesn't work anymore with the elchi, deepblue or moronimo skins (only the "classic" one works with this patch). Bye
Luca Olivetti wrote: > Martin Cap wrote: > >> Hi, >> >> how about the "original" patch I posted ? Does it happen there ? > > > No, it doesn't, but the femon plugin doesn't work anymore with the > elchi, deepblue or moronimo skins (only the "classic" one works with > this patch). Wait, it doesn't work even without your patch, it works only with Ville's patch. Bye
Luca Olivetti wrote: > Luca Olivetti wrote: > >> Martin Cap wrote: >> >>> Hi, >>> >>> how about the "original" patch I posted ? Does it happen there ? >> >> >> >> No, it doesn't, but the femon plugin doesn't work anymore with the >> elchi, deepblue or moronimo skins (only the "classic" one works with >> this patch). > > > Wait, it doesn't work even without your patch, it works only with > Ville's patch. Ville's patch also fixes some colours in some pages with the osd teletext plugin, your doesn't (i.e. it seems it can manage more colours on larger areas at once). Pity for the problem after using the yaepg plugin. Bye
Luca Olivetti wrote: > Luca Olivetti wrote: > >> Luca Olivetti wrote: >> >>> Martin Cap wrote: >>> >>>> Hi, >>>> >>>> how about the "original" patch I posted ? Does it happen there ? >>> >>> >>> >>> >>> No, it doesn't, but the femon plugin doesn't work anymore with the >>> elchi, deepblue or moronimo skins (only the "classic" one works with >>> this patch). >> >> >> >> Wait, it doesn't work even without your patch, it works only with >> Ville's patch. > > > Ville's patch also fixes some colours in some pages with the osd > teletext plugin, your doesn't (i.e. it seems it can manage more colours > on larger areas at once). Pity for the problem after using the yaepg > plugin. I'm lost now. I'm testing your patch and the femon plugin shows correctly with the elchi skin if it's the first one shown, afterwards the yaepg plugin doesn't work (all black). Viceversa, if I use the yaepg plugin first after a restart it's the one working fine but the femon plugin show its texts black with the elchi skin. Bye
Luca Olivetti wrote: >> Ville's patch also fixes some colours in some pages with the osd >> teletext plugin, your doesn't (i.e. it seems it can manage more >> colours on larger areas at once). Pity for the problem after using the >> yaepg plugin. > > > I'm lost now. I'm testing your patch and the femon plugin shows > correctly with the elchi skin if it's the first one shown, afterwards > the yaepg plugin doesn't work (all black). Viceversa, if I use the yaepg > plugin first after a restart it's the one working fine but the femon > plugin show its texts black with the elchi skin. But then, it's the same even without your patch, so it's not making things worse. Bye
On Sat, 2005-04-02 at 12:51 +0200, Luca Olivetti wrote: > Luca Olivetti wrote: > > >> Ville's patch also fixes some colours in some pages with the osd > >> teletext plugin, your doesn't (i.e. it seems it can manage more > >> colours on larger areas at once). Pity for the problem after using the > >> yaepg plugin. Yep, that's expected. Before the patch, colors weren't being "freed" in OSD_Close, but were left reserved even though nothing in the OSD was using them any more. That obviously resulted in the whole whopping limit of 16 colors reaching the top sooner than supposed. > > I'm lost now. I'm testing your patch and the femon plugin shows > > correctly with the elchi skin if it's the first one shown, afterwards > > the yaepg plugin doesn't work (all black). Viceversa, if I use the yaepg > > plugin first after a restart it's the one working fine but the femon > > plugin show its texts black with the elchi skin. > > But then, it's the same even without your patch, so it's not making > things worse. Since you're tinkering with the palette stuff, here's something I noticed while looking at Martin's patch: cDxr3PaletteManager::GetIndex() is supposed to return the index of a given color in the current palette. If the given color is not found, it returns 0, which doesn't make sense to me. It should probably return -1 or something that isn't a valid index. The return value of GetIndex() is used in cSPUEncoder::Cmd(), the OSD_SetPalette case in the big switch statement. Now, what should be done there if an invalid index is returned (ie. the color is not found in the current palette) is beyond me. Blindly using the returned 0 which is either the first color in the palette or a nonexistent color seems clearly wrong and will almost certainly cause some "effects"... ideas and especially patches welcome :)
Ville Skyttä wrote: > On Sat, 2005-04-02 at 12:51 +0200, Luca Olivetti wrote: > >>Luca Olivetti wrote: >> >> >>>>Ville's patch also fixes some colours in some pages with the osd >>>>teletext plugin, your doesn't (i.e. it seems it can manage more >>>>colours on larger areas at once). Pity for the problem after using the >>>>yaepg plugin. > > > Yep, that's expected. Before the patch, colors weren't being "freed" in > OSD_Close, but were left reserved even though nothing in the OSD was > using them any more. That obviously resulted in the whole whopping > limit of 16 colors reaching the top sooner than supposed. Since freeing colours should be better, then why the strange problem after using the yaepg plugin? Bye
On Sat, 2005-04-02 at 18:47 +0200, Luca Olivetti wrote: > Ville Skyttä wrote: > > > Yep, that's expected. Before the patch, colors weren't being "freed" in > > OSD_Close, but were left reserved even though nothing in the OSD was > > using them any more. That obviously resulted in the whole whopping > > limit of 16 colors reaching the top sooner than supposed. > > Since freeing colours should be better, then why the strange problem > after using the yaepg plugin? Beats me, I've never looked at yaepg... perhaps it's not closing the OSD properly or something.
Ville Skyttä wrote: > On Sat, 2005-04-02 at 18:47 +0200, Luca Olivetti wrote: > >>Ville Skyttä wrote: >> >> >>>Yep, that's expected. Before the patch, colors weren't being "freed" in >>>OSD_Close, but were left reserved even though nothing in the OSD was >>>using them any more. That obviously resulted in the whole whopping >>>limit of 16 colors reaching the top sooner than supposed. >> >>Since freeing colours should be better, then why the strange problem >>after using the yaepg plugin? > > > Beats me, I've never looked at yaepg... perhaps it's not closing the OSD > properly or something. Ok, your fix is wrong, I have (what seems to be) the correct one. I will try it a bit more, clean it up a bit and send it later. Bye
Index: dxr3interface_spu_encoder.c =================================================================== RCS file: /cvsroot/dxr3plugin/dxr3/Attic/dxr3interface_spu_encoder.c,v retrieving revision 1.1.2.5 diff -u -U2 -r1.1.2.5 dxr3interface_spu_encoder.c --- dxr3interface_spu_encoder.c 30 Mar 2005 20:18:13 -0000 1.1.2.5 +++ dxr3interface_spu_encoder.c 30 Mar 2005 21:05:40 -0000 @@ -450,4 +450,5 @@ // This should be done in cSPUEncoder::cSPUEncoder + m_palManager.Clear(); return 0; break; Index: dxr3palettemanager.c =================================================================== RCS file: /cvsroot/dxr3plugin/dxr3/dxr3palettemanager.c,v retrieving revision 1.1.2.2 diff -u -U2 -r1.1.2.2 dxr3palettemanager.c --- dxr3palettemanager.c 30 Mar 2005 20:34:26 -0000 1.1.2.2 +++ dxr3palettemanager.c 30 Mar 2005 21:05:41 -0000 @@ -114,2 +114,8 @@ return m_pal; } + +// ================================== +void cDxr3PaletteManager::Clear() +{ + memset(m_users, 0, sizeof(int) * MAX_COLORS); +} Index: dxr3palettemanager.h =================================================================== RCS file: /cvsroot/dxr3plugin/dxr3/dxr3palettemanager.h,v retrieving revision 1.1 diff -u -U2 -r1.1 dxr3palettemanager.h --- dxr3palettemanager.h 5 Aug 2004 23:05:21 -0000 1.1 +++ dxr3palettemanager.h 30 Mar 2005 21:05:41 -0000 @@ -23,4 +23,5 @@ void AddColor(int color); void RemoveColor(int color); + void Clear(); int GetCount(); int operator[](int index);