Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Plugins
  • Assigned To No-one
  • Operating System Sansa e200
  • Severity Low
  • Priority Very Low
  • Reported Version Version 3.3
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by megamike - 2009-08-17
Last edited by teru - 2010-02-18

FS#10535 - Simple BMP Viewer

This plugin is standart simple BMP viewer.
This patch enable view BMP-files on Rockbox as know file type
You have put bmpview.c and bmpview.h in apps/plugins/ directory

Closed by  teru
2010-02-18 15:27
Reason for closing:  Accepted
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

in r24754.

Oops, there is the files of my patch

I had a feeling this was going to happen after the png viewer, good thing though. I’m glad I don’t have to open them with rockpaint.

There’s really no need for the plugin_name buffer, just replace ‘plugin_load(plugin_name,buf)’ with ‘plugin_load(PLUGIN_DIR “/viewers/bmpview.rock”, buf)’ and it should work.

Ok Maurus, here is edited version :

Here is new version with fixed some bugs…

Could you please include bmpview.{c,h} in your patch?

If you don’t know how to add a file to svn: svn add <file>

Dose this work for the iPod 5G? because it claims I need bmpview.o

Of course, Maurus, I included bmpview.c and bmpview.h in patch file.
Here it is:

Gman, I’m not sure about iPod 5G, but suppose it must.
Try last version of my patch - v.1.1.

Gman, I’m not sure about iPod 5G, but suppose it must.
Try last version of my patch - v.1.1.

I see you only added credits to bmpview.h, but you didn’t use your full name.

Rockbox policy requires you to put your full name there though, see http://svn.rockbox.org/viewvc.cgi/trunk/docs/CONTRIBUTING?view=raw .

Other than that, I don’t see anything wrong (haven’t actually tested your patch though).

edit:
small nitpick: you used // comments instead of /* */ comments in some places

nls commented on 2009-08-20 10:45

Hi, the plugin looks nice but i have a few comments
Please move the contents of the .h file into the .c file to be more in line with the other single file plugins.
There are some very long lines i your code, our guidelines state a maximum of ~80 chars.
It doesn’t compile for ipod video for example as it has no BUTTON_POWER, you can look at other plugins to see how they have managed keymaps for different targets.
The hunk in filetree.c should not be needed, instead please add a line for bmpview to viewers.config (I see that as it is now this will not work but i think dropping bmp from the builtin filetypes is better than this special case.
Quite a few images triggered a " Bitmap too large for buffer” … Please do not include commented out lines of code unless they have some special meaning or are otherwise motivated.

Some suggestions too, while i’m at it.
Clearing the backdrop and using a plain coloured background would look much better i think.
Centering small images would also look nicer on large displays.

Hi, Nils. Thank you for your attention. Your suggestions are very useful, and I will embody them in the next version necessarily.

Hi everybody!
Here is new version of my bmp_viewer:

In this version :
1. Centering of small images
2. Full LCD-size of resized big images

I see you do a rb→plugin_get_audio_buffer() at plugin_start(), would it be possible to first try with a rb→plugin_get_buffer() call and if buf_size is too small, then try rb→plugin_get_audio_buffer()?
That way audio doesn’t get paused when you view (small) BMP images.

Also I spotted some typos like “, Could not load %s” → “Could not load %s”, “Error: No parametrs.”→“Error: No parameters.”, Impementation → Implementation and cb_progess → cb_progress.

Glad this works again, I test it out on my 5G. It works alright but it still needs a lot of work. It also needs more of a jpeg viewer like surface.

Hi everybody!
I have tried to consider all your remarks as much as possible and even have added a 2x zoom.
It is the following version of my plugin:

teru commented on 2009-12-18 16:08

Rewirte plugin, most part of code is from jpeg. would have almost same UI as jpeg viewer.
I haven’t tested much.

It works fine for me. But the menu shouldn’t be called Jpeg Menu and I noticed that I can’t zoom in big images more than one time.

I would like to call your attention to the task  FS#6321 , which has got the purpose to merge the image viewers.
The advantage would be that the user can make a slideshow about different image formats and we avoid dublicate code.

teru commented on 2009-12-19 04:14

I forgat to change title of menu, thanks. fixed.

For the second issue, it is intended behaviour that you can’t zoom to actual size of image for too big image.
Such image is resized to the size to fit memory while loading image. other wise you can’t view the image at all.
If you stop audio before use bmpviewer, you will be able to veiw the image in larger size.
If you are sure that the image can be opened without resizing, please report detail, the size of image (width, height, and maybe depth) and target/simulator you use.

you know that merging these plugins to one plugin cannot be done easily because available size of memory is limited.

teru commented on 2009-12-20 12:58

Update patch.
Now this would also work on mono target, at least on simulator. not tested on a real device. but i don’t actualy know how it works…

For the second issue, it is intended behaviour that you can’t zoom to actual size of image for too big image.
>Such image is resized to the size to fit memory while loading image. other wise you can’t view the image at all.
Ok thanks for the information. I’m quite impressed with bmpviewer, it displays an image which has a size of 2592×1944 and 14.4MB. (Fuze, Simulator)
you know that merging these plugins to one plugin cannot be done easily because available size of memory is limited.
I saw that you also wrote patches for the other image viewers, so I just wanted to draw your attention to it.
teru commented on 2010-01-18 15:01

resync patch.
* use imageviewer.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing