Created attachment 3051 [details]
Picture of SBC doing a bad job displaying the artwork

Created attachment 3052 [details]
Source image for the artwork file (SqueezeCenter 7.0 GA)

Created attachment 3053 [details]
Another image that doesn't display well on the SBC

The conversion of 8888 to 565 is done in the SDL library blitting function and just drops the least significant bits.

It looks to me that it would benefit from using some dithering algorithm - perhaps this should be done for large artwork images (not the thumbnails which need to be handled as fast as possible to allow scrolling down lists)

Thanks for the pictures.  I'm playing with it in photoshop and put your image up on the Controller screen.

I see a couple of things:

1.  The color banding effects are strong with this particular image.  I was able to reproduce the color pattern closely in photoshop with 32 levels of posterization (approximately 555 encoding).  The quality of the image is definitely improved with a 565 dither.

2.  The photo you took appears to be taken from slightly above perpendicular to the screen.  The viewing angle on the Controller LCD is better from the natural angle when holding the device in your hand, slightly _below_ perpendicular.  I was able to verify that the image looks a much better when viewed normally in the hand.

So the takeaway is that we can do better by doing a proper dither on the device from 888 to 565.  Also, the viewing angle matters.

Yes, dithering should solve the problem. If possible, please use error correction dithering (e.g. Floyd-Steinberg) as it produces a more pleasing result than ordered dithering.

There's a FS dither implementation in the jpeg library, but looks a bit too tied into the rest of the jpeg code to use.

I may get chance to look at this - assuming you are not already doing so.

triode, feel free to look at this.

I will write the code for you! (I can do this very quickly as I have ready made code for this that is an optimized version of IJG and easily can be adapted and does not have dependencies). What format is your data in? (Let me know, and I will make the code ready-to-use for you (C++, I can convert to C if needed).

I'd be happy to contribute this code to your project!

Thanks,
Peter

I will write ready-to-use example code for you! (I can do this very quickly as I have ready made code for this that is an optimized version of IJG and easily can be adapted and does not have dependencies). What format is your data in? (Let me know, and I will make the code ready-to-use for you (C++, I can convert to C if needed).

I'd be happy to contribute this code to your project!

Thanks,
Peter

Created attachment 3061 [details]
example dither code

Peter - the attached is a niave implementation of FS dithering that I've been playing with.  

The problem is that I am not convinced it is better.  It definately adds dither but not sure it makes the display better on the actual remote.  Also takes some time to run, but I've not considered optimisation yet.

As you can see its C which is processing image data in an SDL surface.  I'm interested in any suggestions on either how I've messed up the algorithm or known working alternative ones....  

Its called from lua (the scripting language which drives the remote) with params specifying bitmasks to quantize to.

Created attachment 3070 [details]
FS dithering code with test routines and test images (ZIP)

Created attachment 3071 [details]
5-6-5 Floyd Steinberg dithering routine (C-code)

triode: It is very easy to mess up the dithering quality if proper rounding is not used, or if there are any errors in the code. The result may be non-optimal dithering.

I've attached sampe code. The ZIP archive contains complete test routines that take a 24 bit PPM image (input888.ppm) and creates two output files dither565 and trunc565. Both files are limited to 565 data. As the name implies, the first one uses FS dithering. The second one is a simple bit truncation.

The question is if the display actually uses 5-6-5 internally. You might want to experiment with setting C1_BITS to 5 to see if that gives better quality. Also try 4-4-4. (With 4-4-4 the trunc.ppm file shows the severe banding that I'm seeing).

Thanks,
Peter

I noticed that I have a small mistake in the code I uploaded: Remove the C_ADD definition and the three rows that are using it!!!

The C_ADD code is in the wrong place and detoriates the result. It is not needed and can be left out. If there is CPU time to spare, the correct way to use C_ADD is to remove the three rows that use it and change the rows with C_MASK to:

            c0 -= (pOut[C0_INDEX] = clampTab[(c0 & C_MASK(C0_BITS)) + C_ADD(C0_BITS) + 256]);
            c1 -= (pOut[C1_INDEX] = clampTab[(c1 & C_MASK(C1_BITS)) + C_ADD(C1_BITS) + 256]);
            c2 -= (pOut[C2_INDEX] = clampTab[(c2 & C_MASK(C2_BITS)) + C_ADD(C2_BITS) + 256]);

So, remove C_ADD or make the change above.

Thanks,
Peter

Created attachment 3072 [details]
5-6-5 Floyd Steinberg dithering routine (C-code), corrected

Here's the corrected version.

Created attachment 3075 [details]
dither implementation

Peter thanks - please find attached a compiled version of your dither code which will run on the remote.  This produces slightly different results from mine (but quite a bit faster!) - I still see the dither pattern when viewed from above though as before.

To try - get ssh access to your remote and:
- copy dither.so into /usr/share/jive/jive/slim
- copy SlimServer.lua into same directory (it replaces an existing file)

Restart the remote and it should be using the dither algorithm for images.

Could you check the code - I needed to change one C_WIDTH to 3 as it was running off the calloc'd size of the array.  [I think I've done the right thing re the #define settings?]

Created attachment 3076 [details]
Slightly optimized version of jive code

triode: The code looks correct. I haven't got a chance to try it yet.

I made a few optimizations (since the I/O pointer is the same). Also, no need to call Truncate. The display will do that.

Did you try to experiment with other values for C0_BITS,C1_BITS, and C2_BITS? For instance, try setting them to 4-4-4 and see how that affects my test image...

Created attachment 3077 [details]
updated jive binaries

Peter thanks.  Here's an update based on your source.  The remote requires dither.so to be in /usr/share/jive/jive/slim along with the updated SlimServer.lua file included.  I've included compiles of 444, 555 and 565 dithers - rename the appropriate file to dither.so and restart.

Actually I think your image looks best with 444 dither but it would be useful to get views of people.

Oh, really! That confirms my guess that the display interally is less than 5-6-5.

Are the display specifications available in a PDF? I'd like to read the docs to find out what bitcount is used internally..

I don't know any details about the hardware - hopefully Dean or Richard can answer.

Created attachment 3078 [details]
LCD spec

Spec for the LCD module

Created attachment 3079 [details]
Updated dither.c

Removed unnecessary code in DATA_ROW_BYTE_WIDTH macro, added missing parenthesis in C_MASK macro.

Functionally this is no different from the previous version, but it may compile a few bytes smaller <g>

Created attachment 3080 [details]
LCD driver app note

Full datasheet for the LCD driver IC is here:  http://www.densitron.com/uploadedFiles/Displays/Support/ILI9320.pdf

Created attachment 3081 [details]
Updated dither.c

Dean, thanks for the specs. I'll give them a good read tomorrow morning!

I've tried the various dithers, and 444 does look better. The display is definitely 565, and I have quickly reviewed the kernel and that appears to be setting the display driver to use 565.

I agree 4-4-4 looks better. This is very strange, since the display should have 262K color according to the specifications.

I found more complete specs for the ILI9320 here: http://www.densitron.com/uploadedFiles/Displays/Support/ILI9320.pdf 

According to the specs, it should be possible to feed the display with 24-bit RGB and let the display do the dithering to 256K (page 71, section 8.2.23).

The fact that a 4-4-4 dither looks better implies that the LCD is not processing the lower bits. Possibly something is wrong in the implementation of the image transfer between the CPU and the ILI9320 controller. (For instance, maybe only the 4 most significant bits are sent correctly to the LCD).

Someting is not right here, for sure...

triode: Try a 4-5-4 dither just to see how that comes out.

Thanks,
Peter

Could someone familiar with the code direct me to the files relevant to transfer of image data to the display (or attach them to this bug). I'd like to have a look at those functions.

FWIW, section 7.5, page 39 shows the transfer formats.

Thanks,
Peter

There are a lot of files to check:

First we have the SDL interface to the kernel, the relevant files are:
http://svn.slimdevices.com/trunk/squeezeplay/src/SDL-1.2.11/src/video/fbcon/?root=Jive

Then we have the kernel S3C2412 driver:
http://svn.slimdevices.com/trunk/squeezeos/src/s3c2412/linux-2.6.22/drivers/video/s3c2410fb.c?rev=2031&root=Jive&view=auto

The ili9320 driver:
https://private-svn.slimdevices.com/trunk/squeezeos/src/s3c2412/linux-2.6.22/drivers/video/s3c2410fb.c?rev=2031&root=jive&view=auto

The platform configuration for jive (see s3c2410fb_display jive_vgg2432a4_display and ili9320_platdata jive_lcm_config):
http://svn.slimdevices.com/trunk/squeezeos/src/s3c2412/linux-2.6.22/arch/arm/mach-s3c2412/mach-jive.c?rev=2031&root=Jive&view=auto

You may also find the cpu datasheet useful. This used to be available on the Samsung site but it appears they have recently made it available by request only, sigh. Let me know if you would like a copy.

If you have questions it may be quickest to discuss in the developers campfire, see the post in the developers forum for access info.

Thanks,
Richard

A quick look at the mach-jive.c it looks like the ili9320 is being set to use RGB18. I'm in the middle of something else now, so not sure what the impact of that is.

Created attachment 3082 [details]
Updated dither.c to take into account ILI9320 bit duplication in 16-bit RGB mode

Since ILI9320 duplicates the red and blue LSB, this also needs to be taken into account when dithering. Here's an updated version of dither.c that does that.

Comment on attachment 3082 [details]
Updated dither.c to take into account ILI9320 bit duplication in 16-bit RGB mode

This code is not relevant if ILI9320 is in the 18-bit RGB mode.

I'm not sure why the lcd driver is set to 18 bit mode. I need to review the datasheets and schematics to see what's right.

Doesn't that depend on how the hardware is implemented? If a 16-bit data bus is used (with DB12 and DB0 unused), then 16-bit mode must be used. If the data bus is 18-bits wide, then using 18-bit RGB is best...

Right, as I said I need to revisit the schematics and datasheets for the lcd and cpu. We didn't implement the kernel driver for this panel, it was supplied by a third party and 'just worked'. I'm now wondering if the RGB is being sent correctly. I'll try to look at this more this evening.

So the cpu <-> lcd interface is wired for 18 bits. But the cpu lcd controller supports 16 or 24 bits. The best option now (also considering system performance) is 16 bit 565. At the moment the kernel incorrectly sets the lcd driver to use an 18 bit interface, so R0 and B0 are NC in the cpu. Correctly this is a simple fix that I'll try tomorrow, not sure it will be making a significant difference?

The lcd controller is now correctly set to 16 bit 565 rgb in jive 7.0.1 r2115. As expected this does not fix the dithering, and 444 still looks better for the artwork.

triode are you sure the image has not already been converted to 565 before you call the dither function? The loadImageData call in jive_surface.c will convert any images to the screen format if possible (e.g. if they don't include an alpha layer). the dither routine probably should be linked into this function?

Created attachment 3089 [details]
Updated dither.c to take into account ILI9320 bit duplication in 16-bit RGB mode

Since the display is using 16-bits, this version should be used. It takes into account that the display driver performs R0=R5 and B0=B5.

Richard, how difficult would it be to switch the display driver to 24-bit mode for testing purposes? If it's not too hard, it might be worth a try just to see if the problem is somehow related to the 16-bit driver.

Richard - I'm reasonably sure the internal format for all images is ARGB8888 and it only converts to 565 through the expensive blit which we see on the profiler when the screen is drawn - I'll add some debugs to prove though...

So I think the issue here is that the server is always adding an alpha layer for artwork images (added by Ben to make non square images look correct and supported by the libgd image code I added).  This means that loadImageData calls SDL_DisplayFormatAlpha which sets the internal format to 8888.  

So a dump of the sdl->format at the top of my jiveL_dither565 gets:
SDL surface info: Bits/pixel 32 Rmask: 00ff0000 Gmask: 0000ff00 Bmask: 000000ff

So it is fair to be assuming 8888 here and the dither routine is doing the right thing.  The image doesn't get converted to 565 until it is blitted to the hardware surface via BlitARGBto565PixelAlpha, this will do the following most of the time as the alpha is 0 or 255:

*dstp = (Uint16)((s >> 8 & 0xf800) + (s >> 5 & 0x7e0) + (s >> 3  & 0x1f));

There is scope for optimisation here with color keys and storing the internal image in 16 bit format to avoid the final blit needing to convert to 16 bits, but I don't believe this is relavent to the current problem.

Triode: That's right. BTW, the CPU (and image data) is big-endian, right? (At least that's the way ARGB currently is configured in dither.c, i.e. memory byte offset 0=A, 1=R, 2=G, and 3=B).

Created attachment 3095 [details]
display text image to check pixel depth

Attached is a simple applet which displays a test pattern on the screen.  How many unique colours do you see?

Peter to install scp the zip to jive and uncompress so it creates a directory DisplayTest in /usr/share/jive/applets

When you go into the menu from Extras you should see RGB + White columns where each pixel represents an increment in 888 - i.e. the image is 256 pixels tall with the first one being 0x000000 and the last being 0xffffff for the white band on the right.  Theres also some white/black guides which are 4 pixels high.

I think I see 32 unique Red & Blue bands and 64 unique Green bands - what do you see.

C code included so you can check what its doing...  (should be able do the same from lua but I don't trust the bindings at present so this writes direct to the image bytes in C)

triode: Very good test! Did you notice that it reveals a problem? Look at the 2nd last segment on the grayscale. It is PINK (instead of a very light gray)! That's probably the source of the problem. Why is it pink? (4-4-4 dithering would hide this problem since it would not use the gray level that is pink).

Well, the gray bar I see is possibly because I'm using r2097 and not the new 7.0.1 r2115 that correctly sets the display to 16-bits. Where can I get 7.0.1 r2115?

Latest trunk/7.0.1 build is here:

http://www.slimdevices.com/downloads/nightly/latest/trunk/

But as of the build time, it was r2114

Yes it does look pink to me if viewed from square on, but gray from below.  This is with the 16bit modification in 7.0.1.  

Is this highlighting that the viewing angle is critical, as pointed out by Dean?  What is the intended viewing angle of the lcd itself?

triode: could you please compile the latest version with 4-4-4, 4-5-4, 5-5-5, and 5-6-5. Thanks!

Well the "latest version" was a change in the kernel options - so you really need a new jive.bin which is over 10M, the previous dither.so should work with it though - which ones do you not have?

I can send you a jive.bin image to your email address or you can wait to download tomorrow's nightly.

No, I'm referring to the version of dither.c that compensates for the bit duplication done by ILI9320.

I got the v2114 installed...

Created attachment 3096 [details]
dither.so with correction

Sorry.  Here's the dither.so files with the correction for bit duplication.

Created attachment 3097 [details]
dither.c that uses 5-6-5 bits except near full level 

triode: Please give this verision a try. It's avoiding the use of the level that results in obvious pink on the gray scale.

Created attachment 3098 [details]
compile for attachment 3097 [details]

Created attachment 3099 [details]
Version of dither that lets you set the max level used

Possibly the answer to why 4-4-4 dither looks best (so far) is that the maximum level sent to the display is 240 (11110000 binary). Consequently the levels that result in pinkish tones are mostly avoided.

I've attached a version of 5-6-5 dithering that scales the image to use a maximum level. Triode, could you please build a version with MAX_OUTPUT_LEVEL set to 224 and one set to 240. Thanks!

Created attachment 3100 [details]
compile for attachment 3099 [details]

triode, thanks for compiling it! FWIW, the original 4-4-4 still remains the best option in my opinion.

It really troubles me that the 4-4-4 is the best looking solution, given that we'd be throwing away a 1/4th of our bits!

Could this be explained by miswiring of some of the lower order bits?

This is a bit of a puzzle, I've checked the schematics and datasheets it looks to me like 565 should be working. Peter how did you work out that the maximum value sent to the display is 240? Are we ok with values < 240, or are some bits getting lost?

Thanks.

Richard, I was referring to the 4-4-4 dither that looks best. Dithering to 4-4-4 means that only 4 bits are sent to the display, thus the maximum value is 11110000 binary (240 decimal).

When you look at triode's test program, you can see that something strange is going on with level 63 ([1..64]) in the gray scale. Level 64 (brightest) appears white, level 62 and below look gray and fade towards black (correct), but level 63 looks pink when viewed from certain angles. (Level 62 is 11111000b or 248 decimal).

This is probably related to the LCD hardware and it is questionable if there is anything that can be done to it.

4-4-4 dithering makes the thumbnails and artwork look much better than non-dithered 5-6-5.

I think we should produce some more test images to understand the colours seen in the region of 0xff for each colour.

I agree the previous one looks pink at one point, but it also shows that we do see 565 resolution apart from this.  I note we only see R = G = B with my current test - I would like to see other values around the pink case.

What is the intended viewing angle of the display?  Do we have any way of varying the contrast to alter this?

One observation: The color wheel displayed in the display self test does NOT exhibit any obvious pink tones.

Hm, that implies a bug in the rendering code.

I decided to give this some time... Unfortunately my Controller now is unable to see my SB/Transporter (haven't tried to use the Receiver). The software upgrade fails with a "There was a problem upgrading this remote..." message.

I tried to downgrade to SC 7.0 and reset the remote, but that did not help...

Frustrating... Is the trash can the place to put the Controller, or...

Thanks,
Peter

The controller is one that I bought (preorder)

I have done a factory reset (with the latest nightly build installed). This did not help. I then tried to install v7.0 and do a factory reset on the SBC, but that did not help either...

Thanks,
Peter

I don't have time to complete this for 7.1.

Punting to 7.3

Created attachment 4073 [details]
cover with bad quality (color gradient, dithering)

the attached cover does really look bad on my controller (combined with different viewing angles it's almost like one of those special effects postcards ;)

i hope this could be improved in the (near) future.

thanks!
Markus

Moving to 7.3.1

we have a theory that this is actually a problem with the LCD initialization...

Changing target to next release

We are now planning to make a 7.3.3 release.  Please review your bugs (all marked open against 7.3.3) to see if they can be fixed in the next few weeks, or if they should be retargeted for 7.4 or future.

Thanks!

Since there's now a planned 7.3.3 release, bugs which won't make the cut-off are being moved to the next target out.  If you feel that this bug needs to be addressed more (or less) urgently than the 7.4 release, please cc chris@slimdevices.com and leave a comment in the bug to that effect so we can review it.

Thanks.

bump. lately i've played with an iThing (not mine) running iPeng and was impressed by the image quality. Though we cannot change the size of the SBC's display nor aspects like the limited viewing angle, i think we should improve the quality to be at least as good as possible - and dithering can improved. thanks!

Targeting based on engineering discussion

Still there in the latest 7.4, it does not look good

this is so old, I don't know why it always get moved to the "next" release!

Triode: do you think this would be possible to do on the server-side with GD?

The method trueColorToPalette might be something to look at.

I think we need to understand what's going on with the display first.  I believe the code we were testing was doing dithering ok, the question was why did the 4-4-4 dither look best and does it look best on all displays?

Adrian: The conclusion thus far is that dithering should not be needed. The best guess is that something is done wrong in the initialization of the display hardware.

One of the first things I recognized after using the controller for the first time was the bad looking cover art because of error dithering.
In list menu it is ok, but not in "now playing" as large.
Please give this bug a much higher priority, 18 voters are a lot and what do I have from a great color display with a bad looking cover art?!

Look close at the Radio's display it shows some off this too.

But the display apparently works better in the other direction or something.

The blocky dithering and the showing of wrong colors is till there.

Check for example the Quartet wallpaper it is brown on my computer but purple on Radio and Controller.

Somwone should assign this bug to Radio to

I've added my vote.
Current cover art display quality is a blocking point for me on the Squeezebox Duet. (i'm considering sending it back to the seller because of this).

I'll add some attachment to show it appears on 7.4.1 release, it's just - in my own opinion - awful.
Maybe there's a way to prepare cover art differently (some others JPEG options?) for them to be displayed correctly?

Created attachment 6715 [details]
Example Cover as it is displayed on the remote.

Created attachment 6716 [details]
Example Cover original image.

I have the same problems as everyone who has posted screenshots of their controller screen.

Thing is that if I hold my controller at an angle of approx. 50-60° (turning it down away from me as if pointing it to the ground), the picture quality gets much better.

So is it hardware-related in the end?

(In reply to comment #89)
> I have the same problems as everyone who has posted screenshots of their
> controller screen.
> 
> Thing is that if I hold my controller at an angle of approx. 50-60° (turning
> it down away from me as if pointing it to the ground), the picture quality gets
> much better.
> 
> So is it hardware-related in the end?

Sorry, it's more like 100-120° instead.

Richard is no longer available to us.

Any news regarding this ? Album art still looks like lego if much red or white is used ?

Unassigned bugs cannot have a priority.