FS#9497 - This patch adds a rotate option to the jpeg viewer.

Attached to Project: Rockbox
Opened by Gerritt Gonzales (GRaTT) - Friday, 17 October 2008, 06:26 GMT
Task Type Patches
Category Plugins
Status Unconfirmed
Assigned To No-one
Operating System Sansa e200
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No


This patch adds a rotate option to the jpeg viewer.
Tested on sansa e200. Works on colour targets only.
Additional work required for zoom and moving around the pict.

This task depends upon

Comment by Gerritt Gonzales (GRaTT) - Friday, 17 October 2008, 08:39 GMT
Zoom works proper now.
Comment by Gerritt Gonzales (GRaTT) - Sunday, 19 October 2008, 03:36 GMT
removed some changes that were not required.
some error checking for grayscale in the simulator.
black and white picts would crash the simulator.
progress bar is landscape when in landscape mode.
I am stuck when it come to scrolling around bitmaps
and true grayscale conversion.
Landscape view is not saved in settings, should it be?
Comment by Keith Perri (perrikwp) - Monday, 20 October 2008, 19:35 GMT
I have added a new option to this patch, auto rotate.
If it is turned on, it determines if the picture would
look best in landscape or portrait mode and automatically
rotate the image upon loading it.

This patch was tested in the Gigabeat F and iPod Video simulators only.

I also added a rotate button to the Gigabeat F button keymap because
the power button wasn't currently being used for anything in the plugin.
This feature could probably be added to other targets that have unused button combinations.

Comment by Dave Chapman (linuxstb) - Tuesday, 21 October 2008, 08:45 GMT
A few comments:

1) Please see docs/CONTRIBUTING for coding guidelines - e.g. variable names (such as SCREENHEIGHT and SCREENWIDTH) should be lower case, use spaces not TABs, follow the existing style of bracket placement (and other things, like whitespace conventions) in any files you edit.

2) Global variables should be declared "static" (such as SCREENWIDTH and SCREENHEIGHT)

3) Does this patch compile for all targets? If I'm reading correctly, the "rotate" function is only implemented for colour displays.

But in general (not related to this patch), this plugin is getting far too big for a single file. and should be split into its own directory - that would make it much easier to maintain and add features to (as well as enabling the core decoding code to be factored out for reuse elsewhere).
Comment by Gerritt Gonzales (GRaTT) - Tuesday, 21 October 2008, 15:29 GMT
There is still a lot of work to do on this patch before SVN inclusion can be considered.
Either a break through in the bitmap scrolling, I am currently stuck, or it not posible
to scroll the bitmap in landscape mode. The sansa e200 converts to grayscale ok but the
simulator would crash. There were some big differences to how the simulator handled an image
and how the sansa handled it, testing on other targets is needed.
Black and white picts would crash the sim and not be displayed correcly on the device.
@ linuxstb 1 and 2: I will look at the coding guidlines and fix in my next patch.
3)currently yes only colour targets.
Some one that really understands the conversion needs to look at this patch.
I am very rusty in my C, but I think there are probably better ways to do a rotation for all targets.
Comment by Keith Perri (perrikwp) - Wednesday, 22 October 2008, 04:45 GMT
Here is a resynced patch (r18858), because the jpeg code was split up recently.
This patch works on my Gigabeat F, but I wasn't sure where to stick the
rotation function in the newly split code. I stuck it in yuv2rgb.c for now,
feel free to stick it where it belongs if that is the wrong place. Thanks!

Comment by Keith Perri (perrikwp) - Wednesday, 22 October 2008, 04:49 GMT
sorry don't use that patch as it contains unrelated changes
Let me recreate a new one with only the jpeg changes.
Comment by Keith Perri (perrikwp) - Wednesday, 22 October 2008, 04:53 GMT
Here's the correct patch:
Comment by Keith Perri (perrikwp) - Thursday, 23 October 2008, 23:17 GMT
Re-synced to r18869.
Comment by Gerritt Gonzales (GRaTT) - Friday, 20 March 2009, 05:19 GMT
This patch has the delete option (fs#7729) as well as rotate.
Comment by Gerritt Gonzales (GRaTT) - Saturday, 20 June 2009, 06:13 GMT
updated for June 19 2009 Version: r21362M-090620
menu changes
Comment by Gman (Thecoolgman) - Saturday, 20 June 2009, 06:25 GMT
Thank you.
Comment by Gman (Thecoolgman) - Monday, 22 June 2009, 04:28 GMT
Hmm I kept trying this. I thought it was another patch causing this but that's not the problem. For some reason the patch applies, but dose nothing.