Patch: dxr3plugin OSD don't turn pink
Commit Message
Luca Olivetti wrote:
> 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.
Here's the patch, to apply with neither yours or Martin's one applied.
Since things are starting to get confusing, I'm also attaching the
complete files I'm using in a tgz.
Basically there were 2 problems:
1) colors are added multiple times but removed only once[*] when the osd
is closed, so the usage count would never get to 0 and the colors would
never actually be freed. The fix is to set m_colors to one instead of
incrementing it each time.
[*]that's not exactly true but it doesn't matter since the osd is being
closed anyway.
2) RemoveColor expected a color, but the colors in m_window are actually
indexes coupled to transparency values. The fix is to change RemoveColor
to accept an index instead.
There's also a futile attempt to use the nearest color when the palette
is full, but I doubt it's really useful.
Bye
Comments
Hi, Luca works great !
I had I another idea last night. Why not use cPalette from VDR (from
osd.h) itself and completly remove cDxr3PaletteManager ?
I tried it and it works for me(no pink OSD after Mplayer, femon-skins
work, haven't tried yaepg though), but try for yourself, please.
I substitued cDxr3PaletteManager in dxr3interface_spu_encoder.h with
cPalette (member m_palManager), and made calls in
dxr3interface_spu_encoder.c fit the interface of cPalette from VDR
(changes in dxr3interface_spu_encoder.c mostly around line 315-341).
cPalette lacks something like RemoveColor() AFAIS, but I think it was
only used to clear the whole palette structure, which is now done
through cPalette::Reset().
Before adding the color to the palette, it gets converted from RGB to
YCrCb thru Tools::Rgb2YCrCb().
BTW, I'm just sending you the two files which needed change
(dxr3interface_spu_encoder.[h|c]) attached to this mail
(you can remove Dxr3PaletteManager.o from the Makefile to see that there
are no references anymore) .
Comments welcome and highly appreciated.
I'm in a rush kinda, so no long text here, sorry :). If you have
question, don't hesitate..
Regards,
Martin Cap...
Luca Olivetti wrote:
>>>>
>
>
> Here's the patch, to apply with neither yours or Martin's one applied.
> Since things are starting to get confusing, I'm also attaching the
> complete files I'm using in a tgz.
> Basically there were 2 problems:
> 1) colors are added multiple times but removed only once[*] when the osd
> is closed, so the usage count would never get to 0 and the colors would
> never actually be freed. The fix is to set m_colors to one instead of
> incrementing it each time.
> [*]that's not exactly true but it doesn't matter since the osd is being
> closed anyway.
>
> 2) RemoveColor expected a color, but the colors in m_window are actually
> indexes coupled to transparency values. The fix is to change RemoveColor
> to accept an index instead.
> There's also a futile attempt to use the nearest color when the palette
> is full, but I doubt it's really useful.
>
> Bye
>
Martin Cap wrote:
> Hi, Luca works great !
>
> I had I another idea last night. Why not use cPalette from VDR (from
> osd.h) itself and completly remove cDxr3PaletteManager ?
> I tried it and it works for me(no pink OSD after Mplayer, femon-skins
> work, haven't tried yaepg though), but try for yourself, please.
It works here too, even with yaepg. If you remove the logo display it's
also possible to use the elchi skin with text2skin 1.0 (both with
cDxr3PaletteManager or cPalette). Unfortunately the other skins
available for text2skin 1.0 need more than the 16 colours the dxr3 is
capable of showing.
Bye
Hi !
The OSD still sucks, but I guess we made it suck a little less :)
Bye,
Martin Cap
Luca Olivetti wrote:
> Martin Cap wrote:
>
>> Hi, Luca works great !
>
>
> It works here too, even with yaepg. If you remove the logo display it's
> also possible to use the elchi skin with text2skin 1.0 (both with
> cDxr3PaletteManager or cPalette). Unfortunately the other skins
> available for text2skin 1.0 need more than the 16 colours the dxr3 is
> capable of showing.
>
> Bye
>
>
Luca Olivetti wrote:
>
> It works here too, even with yaepg. If you remove the logo display it's
> also possible to use the elchi skin with text2skin 1.0 (both with
> cDxr3PaletteManager or cPalette). Unfortunately the other skins
> available for text2skin 1.0 need more than the 16 colours the dxr3 is
> capable of showing.
Hu, I tried enElchi yesterday (with and without logos) and had no such luck.
I still have areas with wrong colors (especially with channel info,
teletext)),
menues are fine with enElchi though.
Well, I will investigate on this :).
Martin Cap wrote:
> Luca Olivetti wrote:
>
>>
>> It works here too, even with yaepg. If you remove the logo display
>> it's also possible to use the elchi skin with text2skin 1.0 (both with
>> cDxr3PaletteManager or cPalette). Unfortunately the other skins
>> available for text2skin 1.0 need more than the 16 colours the dxr3 is
>> capable of showing.
>
>
> Hu, I tried enElchi yesterday (with and without logos) and had no such
> luck.
with "without logo" I mean commenting out the whole section "channel
logo" in enElchi.skin (then adjusting the surrounding areas, channel
number and channel name, to fill the void).
Bye
Luca Olivetti wrote:
>
> with "without logo" I mean commenting out the whole section "channel
> logo" in enElchi.skin (then adjusting the surrounding areas, channel
> number and channel name, to fill the void).
>
> Bye
>
>
Indeed, looks good.
All other skins from text2skin with max. 16 colors work pretty well (as
long as one removes the directories
"logos" & "symbols", or edits .skin-files, like you did).
@@ -287,6 +287,7 @@
switch (cmd)
{
case OSD_SetWindow:
+ //printf("OSD_SetWindow %d\n",x0);
// (x0) set window with number 0<x0<8 as current
if (x0 < 8 && x0 > 0)
{
@@ -298,6 +299,7 @@
break;
case OSD_Open:
+ //printf("OSD_Open\n");
// (x0,y0,x1,y1,BitPerPixel[2/4/8](color&0x0F),mix[0..15](color&0xF0))
// Opens OSD with this size and bit depth
// returns 0 on success, -1 on DRAM allocation error, -2 on "already open"
@@ -310,6 +312,7 @@
break;
case OSD_SetPalette:
+ //printf("OSD_SetPalette\n");
// Spu->Cmd(OSD_SetPalette, 0, NumColors - 1, 0, 0, 0, Colors);
// (firstcolor{color},lastcolor{x0},data)
// Set a number of entries in the palette
@@ -328,9 +331,7 @@
for (int x = color, i = 0; x <= x0; x++,i++)
{
- m_palManager.AddColor((int)*col & 0xFFFFFF);
-
- idx = m_palManager.GetIndex((int)*col & 0xFFFFFF);
+ idx = m_palManager.AddColor((int)*col & 0xFFFFFF);
if (m_palManager.HasChanged())
{
cDxr3Interface::Instance().SetPalette(m_palManager.GetPalette());
@@ -388,27 +389,19 @@
break;
case OSD_Close:
- // clear colors from plattemanager
+ // printf("OSD_Close\n");
- #if VDRVERSNUM >= 10307
- if ((col = (tColor*)m_windows[m_lastwindow].colors) != NULL)
- #else
- if ((col = (eDvbColor*)m_windows[m_lastwindow].colors) != NULL)
- #endif
- {
- for (int i = 0; i < m_windows[m_lastwindow].NumColors; ++i)
- {
- m_palManager.RemoveColor((int)(col[i]) & 0xFFFFFF);
- }
- }
+ // clear colors from palettemanager
+ for (unsigned int i = 0; i < m_windows[m_lastwindow].NumColors; ++i)
+ m_palManager.RemoveColor(m_windows[m_lastwindow].colors[i] & 0xF);
// clear osd
- for (int i = m_windows[m_lastwindow].y0; i <= m_windows[m_lastwindow].y1; ++i)
+ for (unsigned int i = m_windows[m_lastwindow].y0; i <= m_windows[m_lastwindow].y1; ++i)
{
cp = &OSD_Screen[i*OSDWIDTH + m_windows[m_lastwindow].x0];
if ((cp+m_windows[m_lastwindow].x1-m_windows[m_lastwindow].x0+1) < &OSD_Screen[OSDWIDTH * OSDHEIGHT-1])
{
- for (int xx=0; xx <= (m_windows[m_lastwindow].x1-m_windows[m_lastwindow].x0); xx++)
+ for (unsigned int xx=0; xx <= (m_windows[m_lastwindow].x1-m_windows[m_lastwindow].x0); xx++)
{
*(cp+xx) = 0x00;
}
@@ -444,6 +437,7 @@
break;
case OSD_Clear:
+ //printf("OSD_Clear\n");
// Sets all pixel to color 0
// returns 0 on success
@@ -21,64 +21,69 @@
memset(m_colors, 0, sizeof(int) * MAX_COLORS);
memset(m_users, 0, sizeof(int) * MAX_COLORS);
memset(m_pal, 0, sizeof(int) * MAX_COLORS);
- m_changed = false;
+ m_changed = true;
};
+int ColorDistance (int color1, int color2)
+{
+ int d1=(color1>>24)-(color2>>24);
+ int d2=((color1>>16) & 0xFF)-((color2>>16) & 0xFF);
+ int d3=(color1 & 0xFF)-(color2 & 0xFF);
+ return d1*d1+d2*d2+d3*d3;
+}
+
// ==================================
-void cDxr3PaletteManager::AddColor(int color)
+int cDxr3PaletteManager::AddColor(int color)
{
int freeIndex = MAX_COLORS;
- bool found = false;
-
- for (int i = 0; i < MAX_COLORS && !found; ++i)
+ int curDistance = ColorDistance(color, m_colors[0]);
+ int nearestIndex = 0;
+ //printf("AddColor %x ",color);
+ for (int i = 0; i < MAX_COLORS ; ++i)
{
if (color == m_colors[i])
{
if (m_users[i] == 0) m_changed = true;
- ++m_users[i];
- found = true;
+ m_users[i]=1;
+ //printf("existing index %d\n",i);
+ return i;
}
if (m_users[i] == 0 && freeIndex >= MAX_COLORS)
{
freeIndex = i;
}
+ if (m_users[i] > 0) {
+ int newDistance=ColorDistance(color, m_colors[i]);
+ if (newDistance<curDistance) nearestIndex=i;
+ }
}
- if (!found && freeIndex < MAX_COLORS)
+ if (freeIndex < MAX_COLORS)
{
m_colors[freeIndex] = color;
m_users[freeIndex] = 1;
m_changed = true;
+ //printf("new index %d\n",freeIndex);
+ return freeIndex;
}
+ m_users[nearestIndex]=1; //FIXME nearest color it's not probably the best bet
+ //printf("palette full, using color %x\n", m_colors[nearestIndex]);
+ return nearestIndex;
}
// ==================================
-void cDxr3PaletteManager::RemoveColor(int color)
+void cDxr3PaletteManager::RemoveColor(int idx)
{
- bool found = false;
- for (int i = 0; i < MAX_COLORS && !found; ++i)
- {
- if (color == m_colors[i])
- {
- if (m_users[i] > 0) --m_users[i];
- found = true;
- }
- }
-}
-// ==================================
-int cDxr3PaletteManager::GetIndex(int color)
-{
- bool found = false;
- int index = 0;
- for (int i = 0; i < MAX_COLORS && !found; ++i)
- {
- if (color == m_colors[i])
- {
- index = i;
- found = true;
- }
- }
- return index;
+ //printf("RemoveColor %d %x ",idx,m_colors[idx]);
+ if(idx>=0 && idx<MAX_COLORS) {
+ if (m_users[idx] > 0) {
+ //printf("in use ");
+ m_users[idx]=0;
+ m_colors[idx]=0;
+ m_changed=true;
+ }
+ }
+ //printf("\n");
}
// ==================================
@@ -90,7 +95,7 @@
// ==================================
int cDxr3PaletteManager::operator[](int index)
{
- assert(index < MAX_COLORS && index > 0);
+ assert(index < MAX_COLORS && index >= 0);
return m_colors[index];
}
@@ -20,11 +20,10 @@
cDxr3PaletteManager();
~cDxr3PaletteManager() {};
- void AddColor(int color);
- void RemoveColor(int color);
+ int AddColor(int color);
+ void RemoveColor(int idx);
int GetCount();
int operator[](int index);
- int GetIndex(int color);
bool HasChanged();
uint32_t* GetPalette();