dxr3 better (hopefully) color management

Message ID 4259679A.2030605@ventoso.org
State New
Headers

Commit Message

Luca Olivetti April 10, 2005, 5:51 p.m. UTC
  Luca Olivetti wrote:

> I still couldn't find the cause of color bleeding: both the color 
> manager (apart from the error fixed by the patch) and the spu encoder 
> seems ok to me.

I found it and now it's gone :-)
Apply the attached patch, additional to the previous one. Apart from the 
name change of a couple of variables, it only removes the bogus 
optimization that avoided calculating the index if the color hadn't 
changed (but since it might be in a different region/section it could 
well have a different index even if it's the same color).

Bye
  

Comments

Martin Cap April 10, 2005, 6:33 p.m. UTC | #1
Luca Olivetti wrote:
> 
>> I still couldn't find the cause of color bleeding: both the color 
>> manager (apart from the error fixed by the patch) and the spu encoder 
>> seems ok to me.
> 
> 
> I found it and now it's gone :-)
> Apply the attached patch, additional to the previous one. Apart from the 
> name change of a couple of variables, it only removes the bogus 
> optimization that avoided calculating the index if the color hadn't 
> changed (but since it might be in a different region/section it could 
> well have a different index even if it's the same color).
> 
> Bye
> 

Hi !

Could you please resend the patch as I'm not able to apply it neither to 
branch HEAD nor branch vdr-dxr3-0-2 of current cvs  (Something around 
line 212 in dxr3colormanager.c goes wrong, I messed around trying to 
solve it... but lost my nerves after 30min :), yeah I suck) ! BTW. I 
also applied the patch you sent in earlier.


Thank you !
  
Luca Olivetti April 10, 2005, 7:13 p.m. UTC | #2
Martin Cap wrote:
> Luca Olivetti wrote:
> 
>>
>>> I still couldn't find the cause of color bleeding: both the color 
>>> manager (apart from the error fixed by the patch) and the spu encoder 
>>> seems ok to me.
>>
>>
>>
>> I found it and now it's gone :-)
>> Apply the attached patch, additional to the previous one. Apart from 
>> the name change of a couple of variables, it only removes the bogus 
>> optimization that avoided calculating the index if the color hadn't 
>> changed (but since it might be in a different region/section it could 
>> well have a different index even if it's the same color).
>>
>> Bye
>>
> 
> Hi !
> 
> Could you please resend the patch as I'm not able to apply it neither to 
> branch HEAD nor branch vdr-dxr3-0-2 of current cvs  (Something around 
> line 212 in dxr3colormanager.c goes wrong, I messed around trying to 
> solve it... but lost my nerves after 30min :), yeah I suck) ! BTW. I 
> also applied the patch you sent in earlier.

Maybe I made some modifications in between :-/
You can try the revised patch attached or the complete 
dxr3colormanager.[ch] also attached. Sorry for the problems.

Bye
  
Martin Cap April 10, 2005, 7:29 p.m. UTC | #3
Luca Olivetti wrote:
> 
> Maybe I made some modifications in between :-/
> You can try the revised patch attached or the complete 
> dxr3colormanager.[ch] also attached. 

Great job.
Two thumbs up.

I have no tearing-colors anymore, although the whole OSD-thing feels a 
little slower now (?), but - from what I did
so far - there's still a couple of things that could be done different 
regarding OSD in the plugin (maybe faster then).

Most text2skin-skins also work (some make VDR crash the way they always 
did, I must remove logos there).

> Sorry for the problems.
> 
> Bye

No prob. As long as the results are good as this ;) .
  
Luca Olivetti April 10, 2005, 7:52 p.m. UTC | #4
Martin Cap wrote:

> Great job.
> Two thumbs up.

Thanks :-)

> 
> I have no tearing-colors anymore, although the whole OSD-thing feels a 
> little slower now (?)

Yes, removing the "optimization" makes performance suffer, but I think a 
working osd is better than a fast one.
Next time I'll try to optimize it a bit.

Bye
  
Ville Skyttä April 11, 2005, 4:18 p.m. UTC | #5
On Sun, 2005-04-10 at 21:29 +0200, Martin Cap wrote:

> Great job.

Seconded, and applied (a version without the unrelated changes) to the
vdr-dxr3-0-2 branch in CVS.
  
Luca Olivetti April 11, 2005, 7:31 p.m. UTC | #6
Ville Skyttä wrote:
> On Sun, 2005-04-10 at 21:29 +0200, Martin Cap wrote:
> 
> 
>>Great job.
> 
> 
> Seconded, and applied (a version without the unrelated changes) to the
> vdr-dxr3-0-2 branch in CVS.

Ok, so that you don't get bored ;-) , here's an "optimized" version (to 
apply over the previous one). The style isn't exactly what I like but it 
takes about half the time to do the color encoding than the previous 
one. Oh, and it also avoids doing it many times for the same screen ;-)

Bye
  
Martin Cap April 11, 2005, 7:49 p.m. UTC | #7
Luca Olivetti wrote:
> 
> Ok, so that you don't get bored ;-) , here's an "optimized" version (to 
> apply over the previous one). The style isn't exactly what I like but it 
> takes about half the time to do the color encoding than the previous 
> one. Oh, and it also avoids doing it many times for the same screen ;-)
> 

Brilliant :). Works like a charm.

Thank you !
  
Martin Cap April 12, 2005, 11:04 a.m. UTC | #8
Luca Olivetti wrote:
> Ville Skyttä wrote:
> 
>> On Sun, 2005-04-10 at 21:29 +0200, Martin Cap wrote:
>>
>>

> 
> Ok, so that you don't get bored ;-) , here's an "optimized" version (to 
> apply over the previous one). The style isn't exactly what I like but it 
> takes about half the time to do the color encoding than the previous 
> one. Oh, and it also avoids doing it many times for the same screen ;-)
> 

Hi,

Just a quick info what I've noticed: have you tried the recent 
"solitaire"-plugin? Bleeding colors sadly still appear there.
  
Luca Olivetti April 12, 2005, 3:36 p.m. UTC | #9
Martin Cap wrote:
> Luca Olivetti wrote:
> 
>> Ville Skyttä wrote:
>>
>>> On Sun, 2005-04-10 at 21:29 +0200, Martin Cap wrote:
>>>
>>>
> 
>>
>> Ok, so that you don't get bored ;-) , here's an "optimized" version 
>> (to apply over the previous one). The style isn't exactly what I like 
>> but it takes about half the time to do the color encoding than the 
>> previous one. Oh, and it also avoids doing it many times for the same 
>> screen ;-)
>>
> 
> Hi,
> 
> Just a quick info what I've noticed: have you tried the recent 
> "solitaire"-plugin? Bleeding colors sadly still appear there.

No, I didn't. With or without the "optimized" patch?

Bye
  
Martin Cap April 12, 2005, 5:15 p.m. UTC | #10
Luca Olivetti wrote:
> Martin Cap wrote:
> 
>> Luca Olivetti wrote:

> No, I didn't. With or without the "optimized" patch?
> 
> Bye
> 
Hi,
well, I tried it with the optmizations. Must be due to the bitmaps used 
for the cards, all in all more than 16 colors, I guess.
Thus, not the dxr3-plugins' fault ;).
  
Luca Olivetti April 12, 2005, 5:16 p.m. UTC | #11
Martin Cap wrote:
> Luca Olivetti wrote:
> 
>> Ville Skyttä wrote:
>>
>>> On Sun, 2005-04-10 at 21:29 +0200, Martin Cap wrote:
>>>
>>>
> 
>>
>> Ok, so that you don't get bored ;-) , here's an "optimized" version 
>> (to apply over the previous one). The style isn't exactly what I like 
>> but it takes about half the time to do the color encoding than the 
>> previous one. Oh, and it also avoids doing it many times for the same 
>> screen ;-)
>>
> 
> Hi,
> 
> Just a quick info what I've noticed: have you tried the recent 
> "solitaire"-plugin? Bleeding colors sadly still appear there.

Ok, I downloaded it and I'm afraid there's not much to do: it stretches 
the limit of the spu format (btw, I'd like to try some of the stuff the 
guys that designed this standard smoked, it must have been very strong 
stuff).
The cards need too many regions (too many different adjacent colors for 
the jack, queen and king). I tried with MAX_REGIONS 120 in 
dxr3colormanager.h and it seems to work now *but* it could cause the spu 
data to grow to big, hence no picture at all. And it isn't too stable.

Bye
  
Luca Olivetti April 12, 2005, 5:27 p.m. UTC | #12
Martin Cap wrote:
> Luca Olivetti wrote:
> 
>> Martin Cap wrote:
>>
>>> Luca Olivetti wrote:
> 
> 
>> No, I didn't. With or without the "optimized" patch?
>>
>> Bye
>>
> Hi,
> well, I tried it with the optmizations. Must be due to the bitmaps used 
> for the cards, all in all more than 16 colors, I guess.

See my other message, but actually the limit of the spu unit is *4* 
simultaneous colors, the plugin achieves 16 by playing with highlight 
regions (each region can use 4 color different than the standard ones 
and different in each region, up to the palette limit of 16 colors).
Each vertical region can have no more than 15 horizontal sections (each 
with different colors). It seems there is no limit in the number of 
regions but there's a limit in the total size of the data you can send 
the spu.
If only there was a way to send the dxr3 the raw picture data....

> Thus, not the dxr3-plugins' fault ;).

f<censored>g sigma design :-(

Bye
  

Patch

--- dxr3colormanager.c.orig	2005-04-10 19:44:53.279555663 +0200
+++ dxr3colormanager.c	2005-04-10 19:42:13.417600365 +0200
@@ -81,19 +81,11 @@ 
 
 // ==================================
 // Opens a new highlight region
-void cColorManager::OpenRegion(int y, int NrOfSecToCopy)
+void cColorManager::OpenRegion(int y)
 {
 	hlr[NrOfRegions] = new yRegion();
 	hlr[NrOfRegions]->Y1 = y;
 	isopen = true;
-
-    if (NrOfSecToCopy > 0)
-    {
-    	for (int i = 0; i < NrOfSecToCopy; i++)
-        {
-        	hlr[NrOfRegions]->Section[i] = hlr[NrOfRegions - 1]->Section[i];
-        }
-    }
 }
 
 // ==================================
@@ -103,6 +95,7 @@ 
 
 	hlr[NrOfRegions]->Y2 = y;
 	isopen = false;
+	
 
 	if (hlr[NrOfRegions]->N != 0)		// skip this region if there is no section defined
 	{
@@ -117,60 +110,100 @@ 
 // ==================================    
 void cColorManager::EncodeColors(int width, int height, unsigned char* map, unsigned char* dmap)
 {
-    unsigned char color = 0xFF, ccol = 0xFF;
-    unsigned char ColorIndex = 0xFF;
+    unsigned char color;
+    unsigned char ColorIndex;
     unsigned char buffer[1024] = {0};
 
-    for (int y = 0; y < height; ++y) 
-	{
-        color = 0xFF;
-        for(int x = 0; x < width; ++x) 
-		{
-            ccol = map[y * width + x];
-            if (ccol != 0) MaxY = y;
-            if (ccol != color)  
-			{
-                color = ccol; // save this color
-                if (!AddColor(x,y,color, ColorIndex)) 
-				{ 
-					// add this color to highlight regions
-                    color = 0xFF;
-                    x = -1;
-                } 
-				else
-				{ 
-					// color successfully added
-                    buffer[x] = ColorIndex;
-                }
+    for (int y = 0; y < height; ++y) {
+        for(int x = 0; x < width; ++x) {
+            color = map[y * width + x];
+            if (color != 0) MaxY = y;
+            if (AddColor(x,y,color, ColorIndex)) { 
+	       // store as the highlight region index
+	       buffer[x]=ColorIndex;
+	    } else {   
+	      //retry with another region - FIXME: check limits to avoid infinite loop				
+              x = -1;
             } 
-			else
-			{
-                buffer[x] = ColorIndex;//*(dmap+(y * width + x)) = ColorIndex;
-            }
         }
         dxr3_memcpy(dmap+y*width, buffer,width);
     }
+  
+/*    
+     {
+    FILE *fp;
+    fp = fopen("OSD.dump","w+");
+    u_char *pippo=dmap;
+    u_char *pippo2=map;
+    int curregion=0;
+    int cursection=0;
+    
+    
+    for (int dumpy=0; dumpy<height; dumpy++) 
+      {
+      if(curregion<NrOfRegions) {
+        if(hlr[curregion]->Y1==dumpy) {
+          fprintf(fp,"%i",hlr[curregion]->N);
+          for(int sec=0; sec<hlr[curregion]->N; sec++) fprintf(fp,",%i",hlr[curregion]->Section[sec]->X1);
+          for(int dumpx=0; dumpx<width; dumpx++) fprintf(fp,"=");
+          fprintf(fp,"\n");
+          curregion++;
+        }
+      }  
+      
+      cursection=0;
+      for(int dumpx=0; dumpx<width; dumpx++) {
+       if(curregion<NrOfRegions) {
+         if(cursection<hlr[curregion]->N) {
+           if(hlr[curregion]->Section[cursection]->X1==dumpx) { 
+             fprintf(fp,"|"); 
+             cursection++;
+             }
+           }
+         }    
+       fprintf(fp,"%01X",*pippo2 & 0xF);
+       pippo2++;
+       }
+       fprintf(fp,"\n");
+      
+      
+      cursection=0;
+      for(int dumpx=0; dumpx<width; dumpx++) {
+       if(curregion<NrOfRegions) {
+         if(cursection<hlr[curregion]->N) {
+           if(hlr[curregion]->Section[cursection]->X1==dumpx) { 
+             fprintf(fp,"|"); 
+             cursection++;
+             }
+           }
+         }    
+       fprintf(fp,"%01X",*pippo & 0xF);
+       pippo++;
+       }
+       fprintf(fp,"\n");
+      }
+      printf("**** dumped\n");
+    }
+*/
+   
 }
 
 // ==================================
 unsigned char cColorManager::AddColor(int x, int y, unsigned char color, unsigned char &ColorIndex) {
     static int yold = -1;
-    xSection* Section = 0;
-    int SectionIndex = 0;
+    xSection *curSection;
 
     if (isopen) 
 	{
-		// there is an opened highlight-region
-        Section = GetSection(x, SectionIndex);	
-		
+	curSection=GetSection(x);
 		// checks whether we have a section defined on the formerly line on this x-position
-        if (Section != NULL) 
+        if (curSection != NULL) 
 		{ 
 			// there was a section
-            if (!Section->HasColor(color, ColorIndex)) 
+            if (!curSection->HasColor(color, ColorIndex)) 
 			{ 
 				// this color is new for this section
-                if (Section->AllColorsUsed()) 
+                if (curSection->AllColorsUsed()) 
 				{ 
 					// no more free colors
                     if (yold != y) 
@@ -178,14 +211,10 @@ 
                         CloseRegion(y-1); 
 						// terminate region
                         return(0);
-                        //yold = y;
-			//			// open new region
-                        //OpenRegion(y,SectionIndex+1);
                     }
-		
-                    Section = NewSection(x);
+                    curSection = NewSection(x);
 		}		// and add new color
-                ColorIndex = Section->AddColor(color);
+                ColorIndex = curSection->AddColor(color);
             }
         }
 		else
@@ -198,9 +227,9 @@ 
 			// open new region
             OpenRegion(y);
         	// create new section
-			Section = NewSection(x);           
+			curSection = NewSection(x);           
 			// and add new color
-			ColorIndex = Section->AddColor(color);
+			ColorIndex = curSection->AddColor(color);
         }
     } 
 	else
@@ -211,25 +240,23 @@ 
 		// open new region
         OpenRegion(y);
 		// create new section
-        Section = NewSection(x);
+        curSection = NewSection(x);
 		// and add new color
-        ColorIndex = Section->AddColor(color);
+        ColorIndex = curSection->AddColor(color);
     }
     return(1);
 }
 
 // ==================================
-xSection *cColorManager::GetSection(int x, int &n)
+xSection *cColorManager::GetSection(int x)
 {
 	int i;
-    n = 0;
 
 	// for every section in the current region
     for (i = 0; i < hlr[NrOfRegions]->N; i++)	
     {
     	if ((x <= hlr[NrOfRegions]->Section[i]->X2) &&  (x >= hlr[NrOfRegions]->Section[i]->X1)) // x-pos is in section
         {
-        	n = i;
         	return (hlr[NrOfRegions]->Section[i]);
         }
     }
--- dxr3colormanager.h.orig	2005-04-10 19:44:53.294551063 +0200
+++ dxr3colormanager.h	2005-04-10 19:39:57.125414307 +0200
@@ -109,14 +109,14 @@ 
 	unsigned char spudata[2*4096];
 	unsigned int BgCol;
 	int MaxY;
-
+	
 	/** Opens a new highlight region */
-  	void OpenRegion(int y, int NrOfSecToCopy = 0);
+  	void OpenRegion(int y);
 	/** Closes the spu-highlight region */
 	void CloseRegion(int y);
 	
     xSection* NewSection(int x);
-    xSection *GetSection(int x, int &n);
+    xSection *GetSection(int x);
 };
 
 #endif /*_DXR3COLORMANAGER_H_*/