Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#10535 - Simple BMP Viewer

Attached to Project: Rockbox
Opened by Michael Ignatiev (megamike) - Monday, 17 August 2009, 17:13 GMT+1
Last edited by Teruaki Kawashima (teru) - Thursday, 18 February 2010, 16:27 GMT+1
Task Type Patches
Category Plugins
Status Closed
Assigned To No-one
Player Type Sansa e200
Severity Low
Priority Normal
Reported Version Version 3.3
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

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
This task depends upon

Closed by  Teruaki Kawashima (teru)
Thursday, 18 February 2010, 16:27 GMT+1
Reason for closing:  Accepted
Additional comments about closing:  in r24754.
Comment by Michael Ignatiev (megamike) - Monday, 17 August 2009, 17:32 GMT+1
Oops, there is the files of my patch
   bmpview.c (9.1 KiB)
   bmpview.h (1.8 KiB)
   bmpview.diff (1.7 KiB) 
 apps/plugins/CATEGORIES |    1 +
 apps/plugins/SOURCES    |    1 +
 apps/filetree.c         |   22 ++++++++++++++++++++++
 3 files changed, 24 insertions(+)

Comment by Gman (Thecoolgman) - Tuesday, 18 August 2009, 01:26 GMT+1
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.
Comment by Maurus Cuelenaere (mcuelenaere) - Tuesday, 18 August 2009, 01:54 GMT+1
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.
Comment by Michael Ignatiev (megamike) - Tuesday, 18 August 2009, 05:02 GMT+1
Ok Maurus, here is edited version :
   bmpview.diff (1.6 KiB) 
 apps/plugins/CATEGORIES |    1 +
 apps/plugins/SOURCES    |    2 ++
 apps/filetree.c         |   20 ++++++++++++++++++++
 3 files changed, 23 insertions(+)

Comment by Michael Ignatiev (megamike) - Tuesday, 18 August 2009, 19:07 GMT+1
Here is new version with fixed some bugs...
   bmpview.c (9.3 KiB)
Comment by Maurus Cuelenaere (mcuelenaere) - Tuesday, 18 August 2009, 19:43 GMT+1
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>
Comment by Gman (Thecoolgman) - Tuesday, 18 August 2009, 23:49 GMT+1
Dose this work for the iPod 5G? because it claims I need bmpview.o
Comment by Michael Ignatiev (megamike) - Wednesday, 19 August 2009, 09:49 GMT+1
Of course, Maurus, I included bmpview.c and bmpview.h in patch file.
Here it is:
   bmpview_v1.1.diff (13.2 KiB) 
 apps/plugins/CATEGORIES |    1 
 apps/plugins/SOURCES    |    2 
 apps/plugins/bmpview.c  |  305 ++++++++++++++++++++++++++++++++++++++++++++++++
 apps/plugins/bmpview.h  |   44 ++++++
 apps/filetree.c         |   20 +++
 5 files changed, 372 insertions(+)

Comment by Michael Ignatiev (megamike) - Wednesday, 19 August 2009, 09:57 GMT+1
Gman, I'm not sure about iPod 5G, but suppose it must.
Try last version of my patch - v.1.1.
Comment by Michael Ignatiev (megamike) - Wednesday, 19 August 2009, 10:51 GMT+1
Gman, I'm not sure about iPod 5G, but suppose it must.
Try last version of my patch - v.1.1.
Comment by Maurus Cuelenaere (mcuelenaere) - Wednesday, 19 August 2009, 13:51 GMT+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
Comment by Nils Wallménius (nls) - Thursday, 20 August 2009, 12:45 GMT+1
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.
Comment by Michael Ignatiev (megamike) - Thursday, 20 August 2009, 19:51 GMT+1
Hi, Nils. Thank you for your attention. Your suggestions are very useful, and I will embody them in the next version necessarily.
Comment by Michael Ignatiev (megamike) - Saturday, 22 August 2009, 08:16 GMT+1
Hi everybody!
Here is new version of my bmp_viewer:
   bmpview_v1.2.diff (21.5 KiB) 
 apps/plugins/CATEGORIES     |    1 
 apps/plugins/SOURCES        |    1 
 apps/plugins/viewers.config |    1 
 apps/plugins/bmpview.c      |  616 ++++++++++++++++++++++++++++++++++++++++++++
 apps/filetypes.c            |    4 
 5 files changed, 622 insertions(+), 1 deletion(-)

Comment by Michael Ignatiev (megamike) - Sunday, 23 August 2009, 19:42 GMT+1
In this version :
1. Centering of small images
2. Full LCD-size of resized big images
   bmpview_v1.3.diff (22.5 KiB) 
 apps/plugins/CATEGORIES     |    1 
 apps/plugins/SOURCES        |    1 
 apps/plugins/viewers.config |    1 
 apps/plugins/bmpview.c      |  645 ++++++++++++++++++++++++++++++++++++++++++++
 apps/filetypes.c            |    4 
 5 files changed, 651 insertions(+), 1 deletion(-)

Comment by Maurus Cuelenaere (mcuelenaere) - Sunday, 23 August 2009, 20:01 GMT+1
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.
Comment by Gman (Thecoolgman) - Wednesday, 26 August 2009, 23:59 GMT+1
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.
Comment by Michael Ignatiev (megamike) - Friday, 04 September 2009, 18:32 GMT+1
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:
   bmpview_v1.4.diff (25.9 KiB) 
 apps/plugins/CATEGORIES     |    1 
 apps/plugins/SOURCES        |    1 
 apps/plugins/viewers.config |    1 
 apps/plugins/bmpview.c      |  756 ++++++++++++++++++++++++++++++++++++++++++++
 apps/filetypes.c            |    4 
 5 files changed, 762 insertions(+), 1 deletion(-)

Comment by Teruaki Kawashima (teru) - Friday, 18 December 2009, 17:08 GMT+1
Rewirte plugin, most part of code is from jpeg. would have almost same UI as jpeg viewer.
I haven't tested much.
   bmpviewer.patch (43.5 KiB) 
 apps/recorder/bmp.c         |   46 +
 apps/plugins/CATEGORIES     |    1 
 apps/plugins/bmpviewer.c    | 1329 ++++++++++++++++++++++++++++++++++++++++++++
 apps/plugins/SOURCES        |    3 
 apps/plugins/viewers.config |    1 
 apps/filetypes.c            |    3 
 6 files changed, 1376 insertions(+), 7 deletions(-)

Comment by Johannes Schwarz (Ubuntuxer) - Friday, 18 December 2009, 19:02 GMT+1
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.
Comment by Teruaki Kawashima (teru) - Saturday, 19 December 2009, 05:14 GMT+1
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.
   bmpviewer.2.1.patch (43.6 KiB) 
 apps/recorder/bmp.c         |   46 +
 apps/plugins/CATEGORIES     |    1 
 apps/plugins/bmpviewer.c    | 1330 ++++++++++++++++++++++++++++++++++++++++++++
 apps/plugins/SOURCES        |    3 
 apps/plugins/viewers.config |    1 
 apps/filetypes.c            |    3 
 6 files changed, 1377 insertions(+), 7 deletions(-)

Comment by Teruaki Kawashima (teru) - Sunday, 20 December 2009, 13:58 GMT+1
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...
   bmpviewer.2.2.patch (46.4 KiB) 
 apps/recorder/bmp.c         |   46 +
 apps/plugins/CATEGORIES     |    1 
 apps/plugins/SOURCES        |    1 
 apps/plugins/viewers.config |    1 
 apps/filetypes.c            |    3 
 apps/plugins/bmpviewer.c    | 1441 ++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 1486 insertions(+), 7 deletions(-)

Comment by Johannes Schwarz (Ubuntuxer) - Sunday, 20 December 2009, 14:56 GMT+1
>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 2592x1944 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.
Comment by Teruaki Kawashima (teru) - Monday, 18 January 2010, 16:01 GMT+1
resync patch.
* use imageviewer.
   imageviewer_bmp.2.3.patch (15.2 KiB) 
 apps/recorder/bmp.c                   |   46 ++++
 apps/filetypes.c                      |    3 
 apps/plugins/CATEGORIES               |    1 
 apps/plugins/viewers.config           |    1 
 apps/plugins/imageviewer/SUBDIRS      |    1 
 apps/plugins/imageviewer/bmp/bmp.c    |  323 ++++++++++++++++++++++++++++++++++
 apps/plugins/imageviewer/bmp/SOURCES  |    2 
 apps/plugins/imageviewer/bmp/bmp.make |   21 ++
 apps/plugins/imageviewer/bmp/bmp_ui.c |    5 
 9 files changed, 396 insertions(+), 7 deletions(-)

Comment by Teruaki Kawashima (teru) - Saturday, 13 February 2010, 05:45 GMT+1
resync.
   imageviewer_bmp.2.4.patch (15.1 KiB) 
 apps/recorder/bmp.c                   |   46 ++++
 apps/filetypes.c                      |    3 
 apps/plugins/CATEGORIES               |    1 
 apps/plugins/viewers.config           |    1 
 apps/plugins/imageviewer/SUBDIRS      |    1 
 apps/plugins/imageviewer/bmp/bmp.c    |  323 ++++++++++++++++++++++++++++++++++
 apps/plugins/imageviewer/bmp/SOURCES  |    2 
 apps/plugins/imageviewer/bmp/bmp.make |   21 ++
 apps/plugins/imageviewer/bmp/bmp_ui.c |    5 
 9 files changed, 394 insertions(+), 9 deletions(-)

Loading...