Re: 3.15 release

2019-10-28 Thread Jonathan Gordon via rockbox-dev
On Mon, Oct 28, 2019, 3:56 PM Franklin Wei via rockbox-dev <
rockbox-dev@cool.haxx.se> wrote:

> All,
>
> There has been some discussion between William and I on IRC regarding a
> 3.15 release in the near future. We are currently aiming for a November 15
> release date. I will be serving as release manager.
>
> We plan to keep HWCODEC source and builds for this release (though some
> builds may be dropped if they fail to build). A future 4.0 release will
> mark the removal of HWCODEC.
>
> Thoughts?
>
> Franklin
>

Death of hwcodec? Yeah I don't believe it :)

>


Re: Please help test gerrit#890

2014-07-06 Thread Jonathan Gordon
On 6 July 2014 05:28, Richard Quirk richard.qu...@gmail.com wrote:


  Please let me know if you find any crashes - or even better if this fixes
 any crashes with your troublesome skins!


 I didn't see any crashes, but the clock_lock2 theme I use on the WPS
 didn't work any more. This is on the sansa clip zip, here's a link to the
 theme:

 http://themes.rockbox.org/index.php?themeid=2144target=sansaclipzip

 Is there anything I can do to help out debugging it? I installed the
 patched rockbox, rebooted and it was back to the default theme. The rockbox
 fallback theme I think it's called. Applying the clock lock 2 theme from
 the settings made no difference, but it didn't crash.

 Cheers,
 Richard


Thanks! This is fixed, please retry with the updated patch.


Re: Extending the metronome plugin

2014-06-19 Thread Jonathan Gordon
Hi


On 19 June 2014 16:19, Thomas Orgis thomas-fo...@orgis.org wrote:

 1. Is there an API call to get a file selection dialog or should the
 plugin instead register as handler/codec for tempo map files to be started
 from the main files menu? Excuse me for not finding that answer
 myself, I didn't find an obvious place where API/plugin design is
 documented. Pointers welcome, of course.


I havnt done anything with plugins for ages but I'm pretty sure there isnt
an API to choose a file. Your best option is to register as the extension
handler (done in apps/plugins/viewers.config). Plugins have access to
anything in the plugin_api struct (in plugin.h) and there are some extra
stuff in apps/plugins/lib/ which you might find useful (or not!).




 2. Is using the display as flashing indicator LED feasible? I guess it
 should be for the Clip+, as it's just a little grid of OLEDs. But the
 question would be if it can be done quickly in a timely manner to serve
 as metronome (thinking of switching the whole upper or lower half for
 opposing tick/tock). Since there is no background illumination, it
 should be rather power conservative to have all black pixels most of
 the time, right? And regarding the speed/latency: I remember running
 Doom on that display, so that should technically work;-)


Sure, you can draw whatever you want to the display and control the
backlight so go nuts!



 I'm a seasoned C developer (some might recognize me for maintaining
 mpg123 over the last 8 years) and am confident to get the job done for
 giving Rockbox an awesome metronome, once I got the starting points. I
 did read the source of the existing metronome plugin and would start
 from there.


 Alrighty then,

 Thomas



Re: Release

2014-06-18 Thread Jonathan Gordon
On 18 June 2014 16:25, Thomas Martitz ku...@rockbox.org wrote:

 I think we should still have a formal freeze period with RC builds. There
 are some open bugs that we can address too, perhaps. Alex is inactive so I
 don't think we have a release manager.



Considering the state of the project (activity definitely seems low
generally -  just look at http://www.rockbox.org/since-4weeks.html) I think
a freeze period is a waste of time. If someone is interested in freezing I
would suggest just picking a commit and tagging it.

Jonathan


Re: DevCon2014

2014-04-03 Thread Jonathan Gordon
So everyone is agreed on melbourne then?


On 4 April 2014 04:35, Tomasz Moń deso...@gmail.com wrote:

 I'll definitely be there!



Re: Ladies and Gentlemen

2013-12-01 Thread Jonathan Gordon
On 2 December 2013 07:49, Thomas Martitz ku...@rockbox.org wrote:

 Hey folks,

 this is not the usual kind of Ladies and Gentlemen mail. Instead of a new
 port I have managed to get Rockbox play sound in a new environment.

 What I am working on is to to detach the playback core (including codecs,
 buffering and playlists) from the GUI and legacy OS-like (threads etc)
 environment. That means I want the playback core to work on native OS
 threads and synchronisation primitives (i.e. pthreads). I can say I have
 managed to get a few songs playing in such a environment. The first song of
 was I Could Be The One by Avicii / Nicky Romero.

 The code is a _huge_ hack right now (I really mean it) but I invite any of
 you guys to collaborate. I try to push some code as early as possible.

 Best regards.


Sounds pretty cool!
(Possibly a dumb question), Are you only targeting *as App type builds or
will this be used on (i.e) ipods with the rockbox core?


Re: Soft lock and screen/lcd activation

2013-03-11 Thread Jonathan Gordon
On 12 March 2013 02:42, Amaury Pouly amaury.po...@gmail.com wrote:

 Hi all,
 As you know, many of our targets features a soft-lock in the WPS which
 locks all the keys except the unlock key, to prevent accidental changes.
 This is a very useful feature but recent targets like the Fuze+ have shown
 that it is too limited in two ways:

 1) Only in WPS
 This feature only exists in the WPS screen, which kind of make sense if
 you listen to music but this is not the only screen where it would be
 useful. One of the most asked one is the radio screen where you currently
 cannot lock the keys.

 2) Doesn't prevent screen activation
 Another problem which is more specific to the Fuze+ is that it features a
 touchpad which you can easily touch, and thus will constantly activate the
 screen/lcd. This is bad for the battery life and this is also very
 annoying. That's precisely why most phones on the market just won't respond
 to touch events when locked and you'll need to touch a physical buttons to
 activate it again. I think it would be very sensible to have such a feature
 in Rockbox too.

 To sum up, what many *users* would like to see is:
 * lock in radio screen (at least)
 * lock which disables the screen until a physical button is hit (with a
 setting)

 Maybe it is time to discuss about these issues and try to fix them. I
 would be willing to help but I must say this is mostly related to part of
 the code I don't know about so help would be greatly appreciated.

 Please share you opinion,

 Amaury Pouly



There is nothing magical about making this work. Just OR ALLOW_SOFTLOCK to
the fm screens action context and add a keymapping for ACTION_STD_KEYLOCK
to the relevant keymap table. Now to make the backlight work how you want
you just need to notify the firmware/ code that the buttons should be
locked, add a softlock_buttons_lock(bool lock) to firmware/button.c and
hook the backlight up to that.

Jonathan


Re: RFC new line API

2013-02-20 Thread Jonathan Gordon
On 20 February 2013 22:34, Thomas Martitz ku...@rockbox.org wrote:


 I start with list.c because that's the most extensive user, but other
 screens can follow easily, e.g. various plugins. The API that I suggest can
 print the whole line (indent, spacing, icon and text) in a single call so
 it's generally attractive.

 Ignoring plugins, 99% of the UI is drawn by list.c, so yes adding API's
for its use would seem attractive, but that will almost certainly end up
being wasted effort. I'm really aiming to have the lists entirely drawn by
the skin engine by the next release which would make any effort here
redundant.

So, in that spirit I'd advocate you just doing the semi-nasty hack and
adding a bit to the style flags so the scroller can redraw the seperater
line (or just piggy-back off the gradient num_lines bits). Once the skin
engine is doing the list drawing the scroller can be returned to a dumb
text drawer (all line decorations would be drawn onto the backdrop buffer
so the scroller clearing the line would leave them intact.)




 If all you want is a line separator then why is it not a simple 1 line
 lcd_hline() call in the draw_list() loop?


 This is what the current version on gerrit does. The problem is that
 scrolling breaks it (the scroll engine redraws the line selector which
 clears the separator). And it sucks from a software-architecture POV: The
 line selector is currently drawn in firmware, the separator would be drawn
 in apps (in this scenario). So two completely different places although
 they are essentially very similar (style attached to each line). I really
 want to clean up this mess for good before moving forward with the
 separator.

 As above, in the interest of not wasting developer time I'd recommend
doing the sucky way knowing it is temporary.




 Thinking about it now, it would be pretty cool to register a callback
 (per viewport) for the scroll engine to call which would do the line
 drawing instead of the current code. (It would have to be per viewport
 because it is very likley to have a skin scrolling line and the list
 scrolling line both scrolling at the same time, though a generic one would
 be needed for default which should just work).


 I have the beginnings of a scroll engine rework which does a callback (per
 line, and each line has a viewport associated). But even the current scroll
 engine already stores a viewport pointer for each line, that is set when
 the line is redrawn.

 This I'm interested in seeing. I'm starting to wonder if the scroller
really needs the viewport pointer per line? If this was changed to store
just the pixel rectangle to draw in (so the line number is also removed,
and relative to the screen, not viewport) then it wouldn't necessarily
overdraw the decorations. The callbacks becomes an issue, but I agree that
using a callback is the way to go to remove this decoration drawing from
firmware.

Jonathan


Re: RFC new line API

2013-02-17 Thread Jonathan Gordon
On 18 February 2013 07:40, Thomas Martitz ku...@rockbox.org wrote:

 Am 16.02.2013 10:46, schrieb Jonathan Gordon:




 On 16 February 2013 08:30, Thomas Martitz ku...@rockbox.org mailto:
 ku...@rockbox.org wrote:

 Hello guys,

 I'm working on a new line print API in apps that's supposed to
 replaces most of lcd_puts_* and lcd_putsxy_*.

 The lcd_puts* became really messy and it still doesn't support
 scrolling properly (not at all for pixel based functions). The
 rework I'm working on will hopefully be simplified and fix
 scrolling, while allowing more control over the line style and
 contents.

 My motivation is to properly implement [1] (line separator in
 lists). This line separator cannot be properly implemented just in
 apps because scrolling draws over it, and scrolling is entirely in
 firmware and calls only firmware function. To not further
 complicate lcd_puts* a rework is needed.

 My idea is a having a single function:

 put_line(int x, int y, struct line_desc *desc, const char *fmt, ...).

 - x, y are the position of the line (in pixels!).
 - struct line_desc defines other properties the line: style (for
 line selectors, and the line separators mentioned above),
 scrolling, line height and multiline information.
 - fmt is a format string that controls the contents of the line.
 Similar to printf() tags can be used to put icons, text and
 margins (perhaps other stuff too in the future), the variable
 paramter list is then used to build the contents

 There are some complications, most notably scrolling, multiline
 and RTL, which I can hopefully work out soon (i think I will solve
 scrolling by setting up a callback so apps code can do the actual
 drawing). As if now there is a preview of my work in [2], see
 apps/gui/bitmap/list.c for an example of how to use the new api.

 So, what do you think? Is going forward worthwhile and welcome? Do
 you have comments/remarks/suggestions as to the proposed API function?

 Best regards.

 [1]: 
 http://gerrit.rockbox.org/r/#/**c/384/http://gerrit.rockbox.org/r/#/c/384/
 [2]: 
 https://github.com/kugel-/**rockbox/tree/newline-apihttps://github.com/kugel-/rockbox/tree/newline-api


 Please don't add any non-text drawing to the lcd_put* API. That should do
 nothing but position text. I think you're going about this the wrong way
 (or at least doing too much at the firmware level).


 Sorry, I wasn't clear enough. My intent is to put put_line() into apps/. I
 totally agree that non-text stuff shouldn't be added to the firmware level,
 which is why I even make this work (to avoid putting the line separator to
 firmware which I *really* don't want). The current patch has it in apps/.

 Being in apps is also a requirement for extending the line selector to the
 icons and indent level (right now the line selector is only drawn below the
 text portion of a line), which is something I really want and already
 implemented.

 Eventually the line selector drawing (i.e. line styles) could be removed
 from firmware once a sufficient apps API is in place.



 I like the idea of registering a callback for scrolling lines, this would
 potentially fix existing issues with the list indenting, however I don't
 think this is the way to fix this (and your line separator).


 My suggestion is:
 1) fix scrolling so it works on a pixel level and not a line level. We
 are well past the time when scrolling on a line level (and using the entire
 line) is outdated. If scrolling can be told the pixel rectangle (or sub
 viewport would be better/simpler) to scroll in then you are 90% of the way
 to what you want.



 This is on my TODO list, but


  2) modify list.c to know the difference between the text height and line
 height. I'm guessing this is the actual issue you're aving with line
 seperater (though your line height patch hsold have done this anyway?)


 list.c already knows the difference, since the line height patch. What do
 you mean by this is the actual issue? list separator and line height are
 barely related.


  3) Draw the line separator in the list UI which is the only correct place
 to put it. Sure, you could piggy-back off the gradient code and draw a line
 manually but please remember that everything in the UI that is drawn should
 be customizable (maybe not from the start, but certainly not modified to
 make it impossible later). (tangent: the gradient code is really in the
 wrong place too, the list should be drawing the gradient and then drawing
 the text over the top, not the lcd_* code).


 This is already implemented by the current version.




 So yes, the lcd_put* API is long outdated and crud, but changing it is
 orthogonal to your motivation.


 I don't want to change it. Not now at least. And I particularly dont want
 to expand it. This work makes some of lcd_put* obsolete so the API can be
 cleaned up in a follow-up change

Re: RFC new line API

2013-02-16 Thread Jonathan Gordon
On 16 February 2013 08:30, Thomas Martitz ku...@rockbox.org wrote:

 Hello guys,

 I'm working on a new line print API in apps that's supposed to replaces
 most of lcd_puts_* and lcd_putsxy_*.

 The lcd_puts* became really messy and it still doesn't support scrolling
 properly (not at all for pixel based functions). The rework I'm working on
 will hopefully be simplified and fix scrolling, while allowing more control
 over the line style and contents.

 My motivation is to properly implement [1] (line separator in lists). This
 line separator cannot be properly implemented just in apps because
 scrolling draws over it, and scrolling is entirely in firmware and calls
 only firmware function. To not further complicate lcd_puts* a rework is
 needed.

 My idea is a having a single function:

 put_line(int x, int y, struct line_desc *desc, const char *fmt, ...).

 - x, y are the position of the line (in pixels!).
 - struct line_desc defines other properties the line: style (for line
 selectors, and the line separators mentioned above), scrolling, line height
 and multiline information.
 - fmt is a format string that controls the contents of the line. Similar
 to printf() tags can be used to put icons, text and margins (perhaps other
 stuff too in the future), the variable paramter list is then used to build
 the contents

 There are some complications, most notably scrolling, multiline and RTL,
 which I can hopefully work out soon (i think I will solve scrolling by
 setting up a callback so apps code can do the actual drawing). As if now
 there is a preview of my work in [2], see apps/gui/bitmap/list.c for an
 example of how to use the new api.

 So, what do you think? Is going forward worthwhile and welcome? Do you
 have comments/remarks/suggestions as to the proposed API function?

 Best regards.

 [1]: 
 http://gerrit.rockbox.org/r/#/**c/384/http://gerrit.rockbox.org/r/#/c/384/
 [2]: 
 https://github.com/kugel-/**rockbox/tree/newline-apihttps://github.com/kugel-/rockbox/tree/newline-api


Please don't add any non-text drawing to the lcd_put* API. That should do
nothing but position text. I think you're going about this the wrong way
(or at least doing too much at the firmware level).

I like the idea of registering a callback for scrolling lines, this would
potentially fix existing issues with the list indenting, however I don't
think this is the way to fix this (and your line seperator).


My suggestion is:
1) fix scrolling so it works on a pixel level and not a line level. We are
well past the time when scrolling on a line level (and using the entire
line) is outdated. If scrolling can be told the pixel rectangle (or sub
viewport would be better/simpler) to scroll in then you are 90% of the way
to what you want.
2) modify list.c to know the difference between the text height and line
height. I'm guessing this is the actual issue you're aving with line
seperater (though your line height patch hsold have done this anyway?)
3) Draw the line separator in the list UI which is the only correct place
to put it. Sure, you could piggy-back off the gradient code and draw a line
manually but please remember that everything in the UI that is drawn should
be customizable (maybe not from the start, but certainly not modified to
make it impossible later). (tangent: the gradient code is really in the
wrong place too, the list should be drawing the gradient and then drawing
the text over the top, not the lcd_* code).


So yes, the lcd_put* API is long outdated and crud, but changing it
is orthogonal to your motivation.

Jonathan


Radio Art is broken since may unfortunately)

2013-01-03 Thread Jonathan Gordon
Hi Mike,

Can you please have a look at FS#12797 when you get a chance? It looks
like da6cebb6b0b17b4a75a2bd4f51b7cf70b5dafe40 broke radio art (found by
bisecting and testing in the fuzev2 sim).

Thanks
Jonathan


Re: Fix for default sleep timer duration as a shortcut

2012-10-06 Thread Jonathan Gordon
On 6 October 2012 05:47, Richard Quirk richard.qu...@gmail.com wrote:
 A couple of patches to do with the default sleep duration setting, the first
 adds a set_sleeptimer_duration function that will be used in the second.

http://gerrit.rockbox.org/327
http://gerrit.rockbox.org/328

 My use case is: I have restart sleep timer when switching on set to yes,
 and reset timer when touching any buttons set to yes. At night, I set the
 sleep timer to 15 minutes, and fall asleep at some point listening to music
 (maybe switching it back on if I don't fall asleep right away). Then in the
 morning I set the timer to a large value that it'll probably never have
 chance to reach during the day. That way I don't have to touch any other
 setting to activate/deactivate the sleep timer.


327 looks sensible enough.. 328 doesn't seem like the correct fix
though, need to understand the issue more to comment.


Re: Archos Recorder build fails: too big

2012-08-21 Thread Jonathan Gordon
On 22 August 2012 00:50, Bertrik Sikken bert...@sikken.nl wrote:
 The archos recorder build has tipped over the limit for size, and now the
 autobuild always fails.

 The tipping point was commit bd6e6ed but the code has been growing
 steadily so I wouldn't say this particular commit is any more responsible
 than others before it.

 Still, we need to fix this somehow. Maybe someone could take a look at
 recent features and decide if some of them might be disabled for the
 recorder?

 Other suggestions?

 As I understood from some brief discussion about this on IRC,
 it can also be fixed by creating a bootloader for the archoses.
 (I don't know all of the technical details though).

 A bootloader should still fit in the available space for a very long time,
 there will be no need to cut features for archos rockbox.

 With kind regards,
 Bertrik


There is that suggestion, or we could just drop support for HWCODEC
completely... considering so few developers even have the AJBR (and
fewer use it on a regular basis) what does keeping it around give us?


Re: Working on ATA: Less internal state, more asking

2012-08-04 Thread Jonathan Gordon
On 3 August 2012 23:51, Jonas Wielicki j.wieli...@sotecware.net wrote:
 Hi all,

 I'm (still *sigh*) hunting my SSD issues on iriver (as posted previously
 on the list[1][2]). I think it basically reduces to a “have you tried
 turning it off and on again” fix, because somehow the SSD seems to get
 stuck sometimes, a powercycle can mitigiate that in most of the cases.

 BUT, in the course I implemented some ATA parts which might come in
 handy with devices which are behaving weirdly, like going to sleep
 automatically, namely, I removed the internal rockbox state about the
 drive's power management state and added requests to get the current
 state instead (using CMD_CHECK_POWER_MODE).

 I realize that I'll probably have to keep some of my rockbox kernel
 patched to work more or less with that SSD (I really should try to
 return it, maybe it's just broken). But some of these changes may give
 rockbox advantages when dealing with buggy devices. Here is a summarized
 list of changes:

 * check the current power mode of the device before using it
 * explicitly wake it up, if needed
 * two levels of error handling:
   1. do a hard reset
   2. powercycle the IDE bus

 I wonder what, if any, of this is appreciated by rockbox so that I can
 wrap it up as one or more nice commits for review at gerrit.

 best regards,
 Jonas Wielicki

[1]:
 http://www.rockbox.org/mail/archive/rockbox-dev-archive-2012-04/0010.shtml
[2]:
 http://www.rockbox.org/mail/archive/rockbox-dev-archive-2012-05/0009.shtml

Hi,
Please put up your patches on gerrit and then we can figure out wether
or not they are wanted in git (sounds like they are!).

Thanks
Jonathan



Re: Changing build/release terminology

2012-03-28 Thread Jonathan Gordon
On 28 March 2012 20:49, Torne Wuff to...@wolfpuppy.org.uk wrote:
 Hi folks,

 I would like to propose that we change our terminology used to
 describe the builds and releases, on the website and in Rockbox
 Utility. Currently we talk about release and current build a lot,
 and this confuses users over and over who don't understand the
 distinction, and I am inclined to agree with people who think this is
 confusing.

 I propose that we do our best to switch to the following terminology
 consistently in our written materials:

 1) Stable release: e.g. stable release 3.10.
 2) Development build: e.g. development build abc123g.
 3) Archived development build: the daily snapshots Rockbox Utility
 can install.

 I think this would make the distinction much clearer, and it would
 also help to make it clear that the daily snapshots are not something
 most people should install (currently people often believe they are
 somehow more stable/reliable than the current builds, which is not the
 case).

 Please, reply with your thoughts. I'm not set on these particular
 terms, it's just an initial proposal.

 Thanks,

 --
 Torne Wuff
 to...@wolfpuppy.org.uk

Sounds good to me. Not sure if I like stable release though, what
are other projects using? perhaps official release?


re FS#12625 - Sleep timer setting is broken

2012-03-27 Thread Jonathan Gordon
Hi,

This wasnt caught when the sleep timer changes were commited, and its
not really a big deal but it should be changed.

IIUC the sleep timer is now broken into a few different settings:
1) set timer on boot
2) restart timer on keypress
3) start the timer with the chosen minutes value (arbitrarily set to
5min increments).

Both the first two settings use the 3rd to know how long to start the
timer for manually. The issue in the task is:
1) It isnt a normal setting so cant be added to the quickscreen and
shortcuts menu (minor inconvinience)
2) cancelling the setting screen enables the timer anyway. - BAD!

I don't understand why setting the sleep timer duration for boot
should be linked to the sleep timer duration i want to set
temporarily. (i.e the current system means unless I have a fixed.cfg
it is impossible for me to set a 30min timer before i go to bed and
have it turn on with a 90min timer).

The duration should be a separate setting and the start sleep timer
menu item should then be changed to start at off so pressing cancel
will not enable a timer.
In addition, extra handling for the sleep timer can be manually added
to the shortcuts screen to start/stop a timer.

Jonathan.


Re: re FS#12625 - Sleep timer setting is broken

2012-03-27 Thread Jonathan Gordon
On 28 March 2012 01:05, Nick Peskett rock...@peskett.co.uk wrote:

 Unfortunately this has been the case with the callback since 2007;

 http://svn.rockbox.org/viewvc.cgi/trunk/apps/menus/main_menu.c?view=diffpathrev=30777r1=12548r2=12549

 As you move through the menu items the sleep timer continually gets reset,
 so when you cancel the menu item the last set remains.


Yes, because it is using a callback for set_int() which really isnt
necessary. I started this email last night after realising removing
that callback and setting the initial value to off would break the
other two settings. This is an easy fix.




 This is pretty much how it was in the original patch before many opinions on
 IRC and the ML stripped it down to the bare minimum changes acceptable to
 get it committed.

 http://www.rockbox.org/tracker/task/10849#comment40259

 http://www.rockbox.org/mail/archive/rockbox-dev-archive-2011-08/0018.shtml


I dont' see discussion in either of those links about stripping the
options down, and anyway, they can probably be ignored anyway. Would
you like to split it back to how it was?

Jonathan


Re: librbcodec

2012-03-04 Thread Jonathan Gordon
On 28 February 2012 15:22, Sean Bartell wingedtachik...@gmail.com wrote:
 Hi, everyone,

 I realize I all but disappeared last summer when GSoC ended, and I'm sorry for
 leaving my work unfinished. I've spent some more time on it, and completed the
 patches that create librbcodec[0].

 I think it's a reasonable way of separating the DSP, metadata, and codec code
 so they can be used by other software, but I haven't gotten much feedback on 
 my
 overall approach. Does my code have the potential to be committed?

 Thanks,
 Sean Bartell

 [0] 
 http://gerrit.rockbox.org/r/#/q/status:open+project:rockbox+branch:master+topic:rbcodec,n,z

Hey!

As always, the most likely way of getting patches pushed is hanging out in IRC.

Jonathan


Reminder: Use Gerrit to submit patches/translation fixes, not flyspray

2012-03-01 Thread Jonathan Gordon
Hi all,

Please remember we have mostly switched to gerrit for patch tracking
and would like to only use flyspray for bugs. If your patches
(translations especially) are submitted to gerrit it is much more
likely they will be merged quickly (they can be done wih a single
click from the website instead of requiring a checkout, download and
apply the patch and push.)

Useful links:
http://gerrit.rockbox.org/
http://www.rockbox.org/wiki/UsingGit
http://www.rockbox.org/wiki/UsingGit#Setting_up_Gerrit


Finally, please add your full name to docs/CREDITS on the first patch
you submit.

Thanks
Jonathan


Re: Reminder: Use Gerrit to submit patches/translation fixes, not flyspray

2012-03-01 Thread Jonathan Gordon
On 2 March 2012 09:56, Jonathan Gordon jdgo...@gmail.com wrote:
 Hi all,

 Please remember we have mostly switched to gerrit for patch tracking
 and would like to only use flyspray for bugs. If your patches
 (translations especially) are submitted to gerrit it is much more
 likely they will be merged quickly (they can be done wih a single
 click from the website instead of requiring a checkout, download and
 apply the patch and push.)

 Useful links:
 http://gerrit.rockbox.org/
 http://www.rockbox.org/wiki/UsingGit
 http://www.rockbox.org/wiki/UsingGit#Setting_up_Gerrit


 Finally, please add your full name to docs/CREDITS on the first patch
 you submit.

 Thanks
 Jonathan

I evidently forgot how translate.rockbox.org is used. Ignore this
email (though still if you can, pushing through gerrit is
easier/faster)/

Jonathan


Re: Call for testers for gerrit#120

2012-02-26 Thread Jonathan Gordon
On 26 February 2012 20:53, Yohan LEE-TIN-YIEN
yohan.leetiny...@gmail.com wrote:
 Please download for your targets and use it for a bit to see if there
 is any issues with the LCD (redrawing, screen updates, etc).


 I've been testing on Sansa Clip+ for 2 days now, and basically what I
 experienced is :
 - everything works fine : menu, wps, rds, demos (all), games (only a few)
 - the menu, wps, rds, demos, games, etc. seems to me more responsive
 (compared to the daily build 2012-02-09 and 3.10 release)
 - While I plug the USB cable and hold the button in order to pass in
 charging battery mode,
   it is a little bit more difficult : it is more likely to ignore the button
 holding and goes in card reader mode.
   But I guess this is a side effect of a more responsive player.

 All in all, everything works fine for Sansa Clip+ with even an improvement
 of responsiveness for better and for worse (a little bit more tricky to hold
 button while pluging the usb cable).

 Yohan

 PS : Sorry if there are english mistakes, I'm french.


Thanks :)

I should have mentioned it at the start, this change is relatively
minor and it will either work or it wont. So if you get any display at
all after the bootloader it means the patch worked fine.

I'm getting pretty confident that the patch (version 9) is commitable,
but I havnt heard anyone testing the older targets without colour
screens (except the ipod g2), so if you have one please give it a
test.



Call for testers for gerrit#120

2012-02-23 Thread Jonathan Gordon
Hi all,
http://gerrit.rockbox.org/r/#/c/120/ changes the way lcd drivers work
internally and every single target needs testing (or at least a few to
make sure it probably works for all of them).

gevaerts has kindly built and hosted builds at
http://rockbox.hostname.be/lcd-test/

Please download for your targets and use it for a bit to see if there
is any issues with the LCD (redrawing, screen updates, etc).

Thanks
Jonathan


Re: having pluginlib-action handling more/all plugin.

2012-01-05 Thread Jonathan Gordon
On 6 January 2012 00:16, Jean-Louis Biasini jlbias...@gmail.com wrote:
 Hi,
 Best Regards,

 Jean-Louis Biasini



The biggest issue with PLA is that quite a few of the plugins use the
same button loop for their menus and their actual game button
handling. That makes  PLA unusable because buttons will work completly
differently in those contexts.

If you *really* want to do this then go ahead, but it is mostly boring
and error prone work. Of couse, if you want to do this why not update
individual plugins properly to updated APIs? i.e many don't use the
multi-screen API at all (they use rb.lcd_put*() directly). There there
is the whole Mr Someone's TODO list if you're looking for something
else to work on :)


Re: ipod videos help

2012-01-02 Thread Jonathan Gordon
On 3 January 2012 15:12, austin galloway even...@gmail.com wrote:
 ok i have rockbox on my ipod video , and when i turn it on it has boot menu
 , with turnoff button , rockbox , then emor console , settings, how do i
 boot the original firmware ,ur directions on manual dont help because the
 apple logo doesnt show nor does either method work , can u please help me
 out , thanks

1) If you have a boot menu you arn't using rockbox's bootloader so
obviously our instructions wont work.
2) This mailing list is for development discussion, use the user
mailing list or IRC for support


Re: Moving sleep timer menu items/ restart timer on keypress

2011-12-21 Thread Jonathan Gordon
On 22 December 2011 00:31, Nick Peskett rock...@peskett.co.uk wrote:
 The root of what I'd like to change; only clock related options having a
 stonking great clock embedded at top of the menu, all non-RTC options having
 a permanent home.


yeah, the system settings submenu :)


Re: HWCODEC

2011-12-18 Thread Jonathan Gordon
On 19 December 2011 04:39, Thomas Martitz ku...@rockbox.org wrote:
 Am 15.12.2011 00:21, schrieb Mike Giacomelli:

 The alternative I think is to keep both together indefinitely while
 accepting that people may not want to upgrade from what they're
 already running.  I think this is a bad way forward because in
 practice if people do not upgrade, then we have left HWCODEC
 essentially unmaintained.  People would basically be stuck with
 whatever bugs were built into the release or build they decide to stay
 with.


 I agree with forking out HWCODEC into a legacy series. It has many
 advantages, while the only real advantage that keeping it in has (which is
 that new features also apply to the eldest targets) is nullified if the
 actual users don't upgrade.

 HWCODEC, which includes CPU_SH and CHARCELL, largely unmaintained. There's
 no active development, and it's basically only a PITA for the active devs.
 The devs with HWCODEC targets aren't the active devs. In the meantime people
 try hard just to keep the build table green. They do this without the
 ability to test changes, as people with hwcodec targets are often not
 reachable.

 All this effort is useless if the users don't even upgrade. I wouldn't want
 to force useless effort onto devs, possibly lowering the creativity or rate
 of contributions.

 Another advantage not mentioned yet is that getting HWCODEC out allows for a
 big cleanup in the code base. Lots of the oldest and ugliest code is only
 for one (or more) of the aforementioned characteristics
 (hwcodec,sh,charcell).

 I see little sense in trying to support (yes, we currently don't really
 support it) hwcodec further. Removing the maintenance burden seems much more
 appealing.

 Best regards.

This pretty much sounds like a given, so once we move to git we create
a fork branch and start stripping crud out :)



Re: the preset files

2011-12-17 Thread Jonathan Gordon
On 17 December 2011 15:57, Scott Berry scottbb1...@gmail.com wrote:
 Hey all devs,

 I am going to begin work on the fm presets.  A couple of questions is there
 any way that there could be a branch made for just uploading presets like
 you do the actual code and also is there any way we could automate the zip
 process so that maybe once a month it actually would zip up the presets
 automatically and place them on the correct spot on the wiki?

 Scott Berry
 Loving Rockbox for about four years now

The wiki is where they should go. There is no need to put them in the
svn repo as it would just slow down people contributing (see the
language updates waiting in flyspray as proof)



Re: dreamlayers: r31296 - in trunk/apps: . metadata

2011-12-15 Thread Jonathan Gordon
This is exactly what we don't want to do!

On 16 December 2011 07:58,  mai...@svn.rockbox.org wrote:
 Date: 2011-12-15 21:58:14 +0100 (Thu, 15 Dec 2011)
 New Revision: 31296

 Log Message:
 Add conditionals for functions only needed on SWCODEC targets.

 Modified:
   trunk/apps/metadata/id3tags.c
   trunk/apps/metadata/metadata_parsers.h
   trunk/apps/playlist.c
   trunk/apps/playlist.h
   trunk/apps/talk.c
   trunk/apps/talk.h

 Modified: trunk/apps/metadata/id3tags.c
 ===
 --- trunk/apps/metadata/id3tags.c       2011-12-15 20:48:15 UTC (rev 31295)
 +++ trunk/apps/metadata/id3tags.c       2011-12-15 20:58:14 UTC (rev 31296)
 @@ -92,6 +92,9 @@
     Synthpop
  };

 +#if CONFIG_CODEC != SWCODEC
 +static
 +#endif
  char* id3_get_num_genre(unsigned int genre_num)
  {
     if (genre_num  ARRAYLEN(genres))

 Modified: trunk/apps/metadata/metadata_parsers.h
 ===
 --- trunk/apps/metadata/metadata_parsers.h      2011-12-15 20:48:15 UTC (rev 
 31295)
 +++ trunk/apps/metadata/metadata_parsers.h      2011-12-15 20:58:14 UTC (rev 
 31296)
 @@ -19,11 +19,14 @@
  *
  /

 +#if CONFIG_CODEC == SWCODEC
  char* id3_get_num_genre(unsigned int genre_num);
 +#endif
  int getid3v2len(int fd);
  bool setid3v1title(int fd, struct mp3entry *entry);
  void setid3v2title(int fd, struct mp3entry *entry);
  bool get_mp3_metadata(int fd, struct mp3entry* id3);
 +#if CONFIG_CODEC == SWCODEC
  bool get_adx_metadata(int fd, struct mp3entry* id3);
  bool get_aiff_metadata(int fd, struct mp3entry* id3);
  bool get_flac_metadata(int fd, struct mp3entry* id3);
 @@ -53,3 +56,4 @@
  bool get_sgc_metadata(int fd, struct mp3entry* id3);
  bool get_vgm_metadata(int fd, struct mp3entry* id3);
  bool get_kss_metadata(int fd, struct mp3entry* id3);
 +#endif /* CONFIG_CODEC == SWCODEC */

 Modified: trunk/apps/playlist.c
 ===
 --- trunk/apps/playlist.c       2011-12-15 20:48:15 UTC (rev 31295)
 +++ trunk/apps/playlist.c       2011-12-15 20:58:14 UTC (rev 31296)
 @@ -1068,6 +1068,7 @@
     return steps;
  }

 +#if CONFIG_CODEC == SWCODEC
  /* Marks the index of the track to be skipped that is steps away from
  * current playing track.
  */
 @@ -1089,6 +1090,7 @@

     playlist-indices[index] |= PLAYLIST_SKIPPED;
  }
 +#endif /* CONFIG_CODEC == SWCODEC */

  /*
  * returns the index of the track that is steps away from current playing
 @@ -2638,6 +2640,7 @@
     return index;
  }

 +#if CONFIG_CODEC == SWCODEC
  /* try playing next or previous folder */
  bool playlist_next_dir(int direction)
  {
 @@ -2647,6 +2650,7 @@

     return create_and_play_dir(direction, false) = 0;
  }
 +#endif /* CONFIG_CODEC == SWCODEC */

  /* Get resume info for current playing song.  If return value is -1 then
    settings shouldn't be saved. */

 Modified: trunk/apps/playlist.h
 ===
 --- trunk/apps/playlist.h       2011-12-15 20:48:15 UTC (rev 31295)
 +++ trunk/apps/playlist.h       2011-12-15 20:58:14 UTC (rev 31296)
 @@ -134,7 +134,9 @@
  bool playlist_check(int steps);
  const char *playlist_peek(int steps, char* buf, size_t buf_size);
  int playlist_next(int steps);
 +#if CONFIG_CODEC == SWCODEC
  bool playlist_next_dir(int direction);
 +#endif
  int playlist_get_resume_info(int *resume_index);
  int playlist_update_resume_info(const struct mp3entry* id3);
  int playlist_get_display_index(void);
 @@ -158,7 +160,9 @@
                               bool recurse);
  int playlist_insert_playlist(struct playlist_info* playlist, const char 
 *filename,
                              int position, bool queue);
 +#if CONFIG_CODEC == SWCODEC
  void playlist_skip_entry(struct playlist_info *playlist, int steps);
 +#endif
  int playlist_delete(struct playlist_info* playlist, int index);
  int playlist_move(struct playlist_info* playlist, int index, int new_index);
  int playlist_randomise(struct playlist_info* playlist, unsigned int seed,

 Modified: trunk/apps/talk.c
 ===
 --- trunk/apps/talk.c   2011-12-15 20:48:15 UTC (rev 31295)
 +++ trunk/apps/talk.c   2011-12-15 20:58:14 UTC (rev 31296)
 @@ -957,6 +957,7 @@
     return 0;
  }

 +#if CONFIG_CODEC == SWCODEC
  /* Play a directory's .talk thumbnail, fallback to spelling the filename, or
    go straight to spelling depending on settings. */
  int talk_dir_or_spell(const char* dirname,
 @@ -973,6 +974,7 @@
         return talk_spell_basename(dirname, prefix_ids, enqueue);
     return 0;
  }
 +#endif

  /* say a numeric value, this word ordering works for english,
    but not necessarily for other languages (e.g. german) */

 Modified: trunk/apps/talk.h
 ===
 --- 

Re: HWCODEC

2011-12-14 Thread Jonathan Gordon
On 15 December 2011 10:58, Rafaël Carré fun...@videolan.org wrote:
 Hello,

 Le Wed, 14 Dec 2011 18:21:42 -0500,
 Mike Giacomelli giac2...@hotmail.com a écrit :
 Advantages:

 ***Greatly simplify large parts of the code for SWCODEC targets (see
 JdGordon's forum posts in rockbox general)


So, my issue isnt so much HWCODEC but CHARCELL support (which just
happens to be HWCODEC). (also low ram targets).
As I said in the forum post, all the ui customisability could be
easily be removed in a low-mem config (which could even be done in svn
if that is wanted).

The topic really should be discussing deprecating/dropping
LCD_CHARCELL, HWCODEC engine and what to do with low mem targets.

Jonathan



Re: HWCODEC

2011-12-14 Thread Jonathan Gordon
On 15 December 2011 12:50, Boris Gjenero boris.gjen...@gmail.com wrote:
 Apparently, there are few HWCODEC users, and I'm not sure if any developers
 regularly use current builds on HWCODEC targets. Because of that, there
 isn't much focus on improving things for HWCODEC.

On this point, I remember there was a HWCODEC related bug (I think
involving folder advance in the wps) which went unnoticed for
something like 6 months because of the lack of users *and developers*
regularly using svn.

Also, quite often in IRC there is a call to get HWCODEC testing done
but absolutly no responses. (Admitadly this is an issue with alot of
targets, but there are far fewer HWCODEC targets in the active dev
group than most other targets).


Re: 3.10: Shut down menu entry

2011-11-29 Thread Jonathan Gordon
On 30 November 2011 07:23, Thomas Jarosch t...@simonv.com wrote:
 Hi,

 I've just given rockbox 3.10RC0 a spin and I really like
 the way it improved on touchscreen targets like the Nokia N900.

 This is probably thanks to kugel's list spacer patch.

 One small thing lacking for a nice out-of-the-box experience is
 a Shut down root menu entry. Support for that is already in rockbox
 and built if the target is an Archos Player.

 [Saint] posted a patch to enable the Shut down menu
 entry for all targets: http://pastebin.com/0dTBEnd4

 Any chance we can squeeze something like this in for 3.10
 or is it already too late in the RC phase?

 Do we want this for all targets or just for RaaA?

 Cheers,
 Thomas

This seems to be one of those annoying arguments which goes on forever
because noone can really agree on it.
I don't see the need for a shutdown menu item, but I know people swear
by it, so if it went in the System menu (even though I'd prefer that
dissapeared) or we made it possible to go in the shortcut menu I'd be
happy.


Re: Context and action for mapping keys

2011-11-28 Thread Jonathan Gordon
Please don't add special actions for a single device. We try to make
all rockbox targets consistant with eachother so anyone picking up an
ipod immediately knows how to use it if they are coming from a clip.
Not only that, I'm very suspicious about how you plan on getting all
the standard buttons in with a keypad which doesnt appear any less
limited than the h300?

On 28 November 2011 19:33, Jean-Louis Biasini jlbias...@gmail.com wrote:
 I would like:
 - some WPS to Database action (we have one to browser, one to playlist)

The keymap is the wrong place for this. If you want to provide
wps-file and wps-database it should be a user configurable option
for all targets.

 didn't find one...
 - Context for playlist and action for deleting item in it that could be
 directly mapped to a device


Use the ACTION_HOTKEY action for that.

Jonathan


Re: Updating the theme site

2011-11-27 Thread Jonathan Gordon
On 28 November 2011 11:44, Mike Giacomelli giac2...@hotmail.com wrote:


 Alright does someone want to help me with that?  Zip is 96x96 color.  Fuze+
 is the same gigabeat (320 x 240).


 Mike

Done, though we need target images for the table



Re: FS#5111 - ipod piezo

2011-11-16 Thread Jonathan Gordon
On 16 November 2011 18:37, Thomas Martitz ku...@rockbox.org wrote:
 Am 16.11.2011 05:33, schrieb Jonathan Gordon:

 Hey all,
 Does anyone know why http://www.rockbox.org/tracker/task/5111 is not in
 svn?

 Jonathan

 Last time I looked I complained that it didn't integrate with the software
 keyclick. What's the situation there now?

 According to your comments you include an a change which should better be a
 separate task (don't repeat clicks on button repeats).

 Best regards.


The task has been open for 5 years, so I'm obviously not asking about
changes in the last month. Is there any reason the piezo driver part
has not been accepted so far?


FS#5111 - ipod piezo

2011-11-15 Thread Jonathan Gordon
Hey all,
Does anyone know why http://www.rockbox.org/tracker/task/5111 is not in svn?

Jonathan


Re: Working towards skin engine 2.0 (includes RFC on code!)

2011-11-13 Thread Jonathan Gordon
On 14 November 2011 01:09, Frank Gevaerts fr...@gevaerts.be wrote:
 On Wed, Nov 09, 2011 at 12:17:02AM +1100, Jonathan Gordon wrote:
 I'm not entirely sure what to do about the conversion macros. I am
 going to keep the OFFSETTYPE() one (though open to a better name)
 because it is helpful).

 Is it really? I can't say I'm convinced,

 If someone could come up with a way to make OFFSETTYPE() actually *do*
 something (a clever way to do type checking, maybe), I'd agree. The way
 it stands now it's just as easy to overlook as a comment when changing
 things, and *nothing* will complain (the thing given there doesn't even
 have to be a valid type...).


At a minimum it is needed so the same maasive rework isnt needed in
the theme editor. And yes, I've found it ery useful during development
to know what exatly is being pointed to. I agree it would be 100x
better if the compiler could actually warn if the type isnt correct
but thats not really doable.


Re: Working towards skin engine 2.0 (includes RFC on code!)

2011-11-08 Thread Jonathan Gordon
On 8 November 2011 22:58, Thomas Martitz ku...@rockbox.org wrote:
 Am 07.11.2011 14:54, schrieb Magnus Holmgren:

 I agree that the macros are a bit long. Also, there are no _ chars to
 separate things, making them a little harder to read. What about changing
 SKINOFFSETTOPTR to SKIN_TO_POINTER and PTRTOSKINOFFSET to SKIN_TO_OFFSET?
 Not much shorter perhaps, but easier to read. And the from part isn't that
 important, IMHO.

 Apart from the lengthy names (perhaps SKIN_2OFF, SKIN_2PTR?), it looks like
 the first parameter is exclusively skin_buffer. If that's true, can't it
 be omitted somehow?

 Best regards.


skin_buffer is a placeholder. In all likelihood this will end up being
different for each skin (usually core_get_data(data-buffer)).


Re: Working towards skin engine 2.0 (includes RFC on code!)

2011-11-08 Thread Jonathan Gordon
On 9 November 2011 01:06, Magnus Holmgren magnus...@gmail.com wrote:
 On Tue, Nov 8, 2011 at 14:17, Jonathan Gordon jdgo...@gmail.com wrote:

 Well,
 I honestly didnt think it would be this simple! Attached is the
 changes removing all dynamic pointers from the skin engine!

 One question: Since SKINOFFSETTOPTR and PTRTOSKINOFFSET both include a
 conditional and are used a lot, what happens with binsize? :)

 --
  Magnus


It really doesnt matter, they could add 1KB and we'd still be ~17KB
better off on most targets. The skin buffer currently over allocates
for cabbie by nearly 20KN on some targets.



Re: Working towards skin engine 2.0 (includes RFC on code!)

2011-11-08 Thread Jonathan Gordon
On 9 November 2011 01:12, Michael Chicoine mc2...@gmail.com wrote:
 On Tue, Nov 8, 2011 at 7:17 AM, Jonathan Gordon jdgo...@gmail.com wrote:

 Testing is pretty much: Apply and run your themes. If it works there
 should be no difference at all.


 A quick test on e200v1 with cabbiev2 (r30933) shows the following issues:

 1. FM Radio screen is blank (except backdrop and statusbar). After
 settings clear, it does not even display the No settings found.
 Autoscan? message.
 2. WPS displays incorrect icons: playmode always shows stop icon, hold
 icon shows locked when locked or unlocked, repeat mode shows
 repeat-all in all repeat modes.


Crap! I've been testing with the video sim, it should all work
everywhere :/ thanks for testing


Re: Working towards skin engine 2.0 (includes RFC on code!)

2011-11-08 Thread Jonathan Gordon
Test builds are available at http://jdgordon.info/rockbox/skintestbuilds/output/

On 9 November 2011 09:01, Jonathan Gordon jdgo...@gmail.com wrote:
 On 9 November 2011 01:12, Michael Chicoine mc2...@gmail.com wrote:
 On Tue, Nov 8, 2011 at 7:17 AM, Jonathan Gordon jdgo...@gmail.com wrote:

 Testing is pretty much: Apply and run your themes. If it works there
 should be no difference at all.


 A quick test on e200v1 with cabbiev2 (r30933) shows the following issues:

 1. FM Radio screen is blank (except backdrop and statusbar). After
 settings clear, it does not even display the No settings found.
 Autoscan? message.
 2. WPS displays incorrect icons: playmode always shows stop icon, hold
 icon shows locked when locked or unlocked, repeat mode shows
 repeat-all in all repeat modes.


 Crap! I've been testing with the video sim, it should all work
 everywhere :/ thanks for testing



Branched for 3.10

2011-11-07 Thread Jonathan Gordon
Hi all,

I've just created the 3.10 release branch. To check it out, run:
 svn co svn://svn.rockbox.org/rockbox/branches/v3_10 rockbox-3.10

This means that trunk is again free for regular development. However,
please don't be afraid to concentrate on bugs for the time being, and
maybe don't do invasive and reasonably minor things like big whitespace
cleanups yet, as they could make backporting fixes more difficult than
they need to be.

Let's fix as many bugs as we can during the next week!


Working towards skin engine 2.0 (includes RFC on code!)

2011-11-06 Thread Jonathan Gordon
Hi all,
The only long term solution to the issue of the skin buffer size is to
completly redo how the skin engine uses its buffer, to that end I have
just started the rather large task of replacing every pointer in it to
an offset into the (currently) shared buffer which is then turned back
into a pointer when it is being used.

One annoyance with the current skin engine code is that all skins are
linked together with the single buffer, this means that if any skins
need reloading (user request to change theme perhaps) all of them need
to be reloaded. So one of the goals with this new work is to split
that up so each skin manages its own buflib handle, allowing skins to
be un/loaded on demand (there is no point loading the fm skin and
wasting buffer space if the user never actualy loads the fm screen for
example.)

The plan is to load the skin into the plugin buffer (and buflib
handles for images as done now), then do a memcpy() of the loaded skin
into a new buflib handle of the exact size needed for the skin. This
means people wanting boring themes don't limit people wanting Over The
Top themes (and no extra ram is wasted). Using offsets to do this
means there is also no need to manage pointers when the handle moves
around.

Another ultimate goal is to draw more of the UI using the skin engine,
specifically screens which are currently hard coded for different
screen sizes (the timedate screen is an obvious candidate), this is
very far down the track though so we'll see if that ever actually
happens.

The attached patch is the very start of this work. I'm really writing
this email because before I get too far in I want to make sure the
macros are going to be acceptable.

Three macros and a typedef have been added.
SKINOFFSETTOPTR() and PTRTOSKINOFFSET() convert between the offset and
the real pointer. I originally wanted to put the type in the first
macro and use that instead of void* but not sure if that is really
needed.
the 3rd macro OFFSETTYPE() is added because I realised the structs
used by the various tags will become very confusing if all the members
are skinoffset (or long) types instead of the pointer type which the
data actually contains. So I added this macro to hopefully help
document the code a bit.

Are these going to cause people to grimace? Is there a better way to
do what they are doing?
This is going to be a rather massive diff so if these macros are
unacceptable I really want to know now rather than once it is all
done.

Cheers
Jonathan


P.S This is on my skinengine_to_offsets github branch
https://github.com/jdgordon/rockbox/tree/skinengine_to_offsets and
will likely be pushed upstream as a single patch when it is done, but
would anyone like to help out? :D

P.P.S If you have a patch that touches apps/gui/skin_engine/* please
talk to me on IRC before committing so we dont cause extra work for
eachother.
diff --git a/apps/gui/skin_engine/skin_display.c b/apps/gui/skin_engine/skin_display.c
index 95e4310..a352252 100644
--- a/apps/gui/skin_engine/skin_display.c
+++ b/apps/gui/skin_engine/skin_display.c
@@ -406,7 +406,7 @@ void wps_display_images(struct gui_wps *gwps, struct viewport* vp)
 {
 wps_draw_image(gwps, img, img-display);
 }
-else if (img-always_display  img-vp == vp)
+else if (img-always_display  SKINOFFSETTOPTR(skin_buffer, img-vp) == vp)
 {
 wps_draw_image(gwps, img, 0);
 }
diff --git a/apps/gui/skin_engine/skin_engine.c b/apps/gui/skin_engine/skin_engine.c
index bd875fe..8a6152f 100644
--- a/apps/gui/skin_engine/skin_engine.c
+++ b/apps/gui/skin_engine/skin_engine.c
@@ -48,7 +48,7 @@ void theme_init_buffer(void)
 skins_initialising = false;
 }
 #else
-static char skin_buffer[SKIN_BUFFER_SIZE];
+char skin_buffer[SKIN_BUFFER_SIZE];
 void theme_init_buffer(void)
 {
 skins_initialising = false;
diff --git a/apps/gui/skin_engine/skin_parser.c b/apps/gui/skin_engine/skin_parser.c
index 1557783..bf2179c 100644
--- a/apps/gui/skin_engine/skin_parser.c
+++ b/apps/gui/skin_engine/skin_parser.c
@@ -168,7 +168,7 @@ void *skin_find_item(const char *label, enum skin_find_what what,
 #ifdef HAVE_LCD_BITMAP
 case SKIN_FIND_IMAGE:
 ret = list.linkedlist-token-value.data;
-itemlabel = ((struct gui_img *)ret)-label;
+itemlabel = SKINOFFSETTOPTR(skin_buffer, ((struct gui_img *)ret)-label);
 break;
 #endif
 #ifdef HAVE_TOUCHSCREEN
@@ -295,9 +295,9 @@ static int parse_image_display(struct skin_element *element,
 {
 return WPS_ERROR_INVALID_PARAM;
 }
-id-label = label;
+id-label = PTRTOSKINOFFSET(skin_buffer, label);
 id-offset = 0;
-id-token = NULL;
+id-token = PTRTOSKINOFFSET(skin_buffer, NULL);
 if (img-using_preloaded_icons)
 {
 token-type = SKIN_TOKEN_IMAGE_DISPLAY_LISTICON;
@@ -306,7 +306,7 @@ static int parse_image_display(struct skin_element *element,
  

Re: 3.10 (was Re: saratoga: r30837 - in trunk: apps apps/lang manual/configure_rockbox)

2011-11-06 Thread Jonathan Gordon
On 7 November 2011 01:47, Thomas Martitz ku...@rockbox.org wrote:
 Am 05.11.2011 12:16, schrieb Alex Parker:

 Excellent, thanks very much :)

 Looks like both issues are resolved, so we should be ready to freeze.

 Best regards.


In Alex's absence (as the person who's been pushing the last few
release) can we decide in IRC tonight if we branch and release?


FS#12251 - user shortcuts in the main menu

2011-11-01 Thread Jonathan Gordon
Hi all,
So I've been working on this patch for a while, and when I started I
didnt really intend on pushing for this to get committed, but now it
is pretty much finished I feel that it would be a shame if it isnt as
it fixes pretty much everyones complaint with the menus and
quickscreen (and adds a bunch of extra niceness for the hell of it :)
)

What this does it add a new menu item Shortcuts to the end of the
menu which is populated from /.rockbox/shortcuts.txt. I've made it
very flexible and tried to add anything which anyone would find
useable. The following are the available shortcut types:
* run a file (same as navigating to the file in the browser and
clicking select
* open the browser at a file/folder (at or in a folder. /Music/foo
will put you in /Music with foo selected while /Music/foo/ will put
oyu in /Music/foo with the first file selected)
* show the current playlist context menu on a file or folder (direct
access to that menu for your favorite music)
* open the setting chooser for any setting which can go on the
quickscreen (i.e 99% of the useful ones)
* open any of the debug menu screens

The only type which I really want but cant add is a shortcut into the
database tree, but the usual issues with database prevents that :/ If
there are any other shortcuts people might want I'm open to adding
them.

You can also choose the text and icon of each item, and add spacer
items so help group your shortcuts (if that makes it easier for you).
I've taken over the Add to shortcuts context menu item in the
browser (adds a browse type for the file/folder) and added the context
menu item to the settings and the debug screens menus so they can be
easily added.

I'd like to make a plugin to more easily modify the shortcut file but
it isnt really necessary.

This combined with http://www.rockbox.org/tracker/task/12361 (which
makes config loading much faster) should easily solve any problems
anyone has with the quickscreen or menu.

So try it out and let me know what you think.


Re: FS#12251 - user shortcuts in the main menu

2011-11-01 Thread Jonathan Gordon
On 1 November 2011 22:39, Peter D'Hoye peter.dh...@telenet.be wrote:

 What this does it add a new menu item Shortcuts to the end of the
 menu which is populated from /.rockbox/shortcuts.txt. I've made it
 very flexible and tried to add anything which anyone would find
 useable. The following are the available shortcut types:
 * run a file (same as navigating to the file in the browser and
 clicking select
 * open the browser at a file/folder (at or in a folder. /Music/foo
 will put you in /Music with foo selected while /Music/foo/ will put
 oyu in /Music/foo with the first file selected)
 * show the current playlist context menu on a file or folder (direct
 access to that menu for your favorite music)
 * open the setting chooser for any setting which can go on the
 quickscreen (i.e 99% of the useful ones)
 * open any of the debug menu screens


 I hope/assume your patch removes the existing shortcuts system (which is for
 folders only)? I think having two systems might be confusing

 /P


I didnt touch the existing one which is a plugin. Thats a topic for
another discussion. The plugin based one has the advatange of being
able t handle more than one shortcuts file which this patch cant do.


Re: 3.10 (was Re: saratoga: r30837 - in trunk: apps apps/lang manual/configure_rockbox)

2011-10-27 Thread Jonathan Gordon
On 27 October 2011 19:33, Björn Stenberg bj...@haxx.se wrote:
 Thomas Martitz wrote:
 FS#12279 - Sansa Clip+: Music playback is returned to the head when wps is 
 changed since r30486

 The last one isn't easily fixable (according to jhMikeS), and I also
 don't consider it release critical (though it's technically a
 regression indeed).

 The bug in the title might be non-critical, but the comments contain reports 
 of
 EOM panics.

 --
 Björn


Considering that task hasnt been touched in a month, and that on my
clip+ it works fine (not using svn though so possibly a different fix
fixed it), I think that might be safe to close, unles someone can
confirm it is still there)



Re: fredwbauer: r30826 - in trunk: apps apps/gui firmware firmware/export

2011-10-26 Thread Jonathan Gordon
The patch earlier had a compile error, this one should be good to go.

On 26 October 2011 01:03, Jonathan Gordon jdgo...@gmail.com wrote:
 On 26 October 2011 00:52, Thomas Martitz ku...@rockbox.org wrote:
 Am 25.10.2011 15:40, schrieb Jonathan Gordon:

 There is an argument that setuifont() must call lcd_setfont(), but if
 setfont() sets global_status that would be wrong. Proof of that is
 what should global_status.font_id[SCREEN_MAIN] be set to after a call
 to screens[SCREEN_MAIN].setfont(FONT_UI) ?


 I thought FONT_UI doesn't exist anymore, because the id isn't constant
 anymore. There comes my confusion from.


 Well, some constant is needed for everything to be able to ask for the
 setting font, and FONT_UI is annoyingly everywhere already.

 On a slightly related note: Why is the font id in global_status, and not
 e.g. a member of the screen_access api? IMO it would make more sense in the
 latter seeing that font getters/setters are also there.

 There is really no difference, global_status was a convenient place to
 put it, putting it in the screen_access struct *might* mean it will be
 in the cache when accessed, perhaps. This is very hair-splitting,
 though if whoever gets to commiting wants to do that I have no
 objection.

diff --git a/apps/filetree.c b/apps/filetree.c
index 2407be9..d33b823 100644
--- a/apps/filetree.c
+++ b/apps/filetree.c
@@ -424,13 +424,10 @@ static void ft_load_font(char *file)
 set_file(file, (char *)global_settings.font_file, MAX_FILENAME);
 #endif
 splash(0, ID2P(LANG_WAIT));
-current_font_id = global_status.font_id[screen];
+current_font_id = screens[screen].getuifont();
 if (current_font_id = 0)
 font_unload(current_font_id);
-current_font_id = font_load(file);
-if(screen==SCREEN_MAIN)
-font_set_ui(current_font_id);
-global_status.font_id[screen] = current_font_id;
+screens[screen].setuifont(font_load(file));
 viewportmanager_theme_changed(THEME_UI_VIEWPORT);
 }
 #endif
diff --git a/apps/gui/skin_engine/skin_parser.c b/apps/gui/skin_engine/skin_parser.c
index a9689a8..c6632b86 100644
--- a/apps/gui/skin_engine/skin_parser.c
+++ b/apps/gui/skin_engine/skin_parser.c
@@ -249,7 +249,7 @@ static int parse_statusbar_tags(struct skin_element* element,
 /* viewport_set_defaults() sets the font to FONT_UI+curr_screen.
  * This parser requires font 1 to always be the UI font, 
  * so force it back to FONT_UI and handle the screen number at the end */
-default_vp-vp.font = FONT_UI;
+default_vp-vp.font = 1;
 #endif
 }
 return 0;
@@ -1645,7 +1645,7 @@ static bool skin_load_fonts(struct wps_data *data)
 font_id = skin_vp-parsed_fontid;
 if (font_id == 1)
 {   /* the usual case - built-in fonts */
-vp-font = global_status.font_id[curr_screen];
+vp-font = screens[curr_screen].getuifont();
 continue;
 }
 else if (font_id = 0)
diff --git a/apps/gui/skin_engine/skin_render.c b/apps/gui/skin_engine/skin_render.c
index 369bd46..4d41a6f 100644
--- a/apps/gui/skin_engine/skin_render.c
+++ b/apps/gui/skin_engine/skin_render.c
@@ -658,7 +658,7 @@ void skin_render_viewport(struct skin_element* viewport, struct gui_wps *gwps,
 
 /* fix font ID's */
 if (skin_viewport-parsed_fontid == 1)
-skin_viewport-vp.font = global_status.font_id[display-screen_type];
+skin_viewport-vp.font = display-getuifont();
 #endif
 
 while (line)
diff --git a/apps/gui/statusbar-skinned.c b/apps/gui/statusbar-skinned.c
index 7850e7c..f79672c 100644
--- a/apps/gui/statusbar-skinned.c
+++ b/apps/gui/statusbar-skinned.c
@@ -128,7 +128,7 @@ struct viewport *sb_skin_get_info_vp(enum screen_type screen)
 if (!vp)
 return NULL;
 if (vp-parsed_fontid == 1)
-vp-vp.font = global_status.font_id[screen];
+vp-vp.font = screens[screen].getuifont();
 return vp-vp;
 }
 
diff --git a/apps/gui/usb_screen.c b/apps/gui/usb_screen.c
index 06770b1..2b7d472 100644
--- a/apps/gui/usb_screen.c
+++ b/apps/gui/usb_screen.c
@@ -265,8 +265,8 @@ void gui_usb_screen_run(bool early_usb)
 #ifdef HAVE_LCD_BITMAP
 FOR_NB_SCREENS(i)
 {
-font_unload(global_status.font_id[i]);
-global_status.font_id[i] = -1;
+font_unload(screens[i].getuifont());
+screens[i].setuifont(FONT_SYSFIXED);
 }
 skin_unload_all();
 #endif
diff --git a/apps/gui/viewport.c b/apps/gui/viewport.c
index c5e4427..b17f765 100644
--- a/apps/gui/viewport.c
+++ b/apps/gui/viewport.c
@@ -317,7 +317,7 @@ void viewport_set_fullscreen(struct viewport *vp,
 #ifndef __PCTOOL__
 set_default_align_flags(vp);
 #endif
-vp-font = global_status.font_id[screen];
+vp-font = screens[screen].getuifont();
 vp-line_height = font_get(vp-font)-height;
 vp-drawmode = DRMODE_SOLID;
 #if LCD_DEPTH  1
diff --git a/apps/plugins/bubbles.c b/apps/plugins/bubbles.c
index

re: fredwbauer: r30826 - in trunk: apps apps/gui firmware firmware/export

2011-10-25 Thread Jonathan Gordon
On 25 October 2011 17:35, Thomas Martitz ku...@rockbox.org wrote:
 Plugins can use the screen_access api. Ideally those should be using a
 custom viewport if they want a custom font, right?

Any plugins which are not using the screen acess api are probably so
old that yes, they dont use a custom font anyway (and wont be modified
to any time soon)

 One question about the patch though: Why separate setfont() and setuifont()?
 I imagine one could be sufficient, but perhaps I'm missing something.

 Best regards.


There are a few issues here:
The biggest issue is that the lcd API is full of 10 years of
evolution. First the multiscreen api came in, and then viewports, both
adding different bits of cruft. (not necessarily bad cruft). lcd_*
functions *mostly* work on the current viewport (lcd_setfont()
included), and some work on a specific viewport, and others work on
the actual display as a whole.

lcd_setfont() changes the current viewports font number. Should this
be removed completly? The *only* time the caller doesnt have access to
the viewport struct is with the default viewport (which we are trying
to slowly get away from, and this should only happen in plugins now,
so no, we cant remove it just yet).

screen_access-setfont() is a wrapper for lcd_setfont() to make sure
FONT_UI gets set correctly. screen_access-setuifont() does not call
lcd_setfont() and only sets global_settings.font_ui[screen].
screen_acces-getuifont() just returns the
global_settings.font_ui[screen] value (and there is no lcd_getfont()).


 Might be worthwhile also to check if font_get(vp-font)-height calls can be
 replaced with screen-getcharheight(). I admit I forgot about that one
 before Björn mentioned it and used the former too.


getcharheight() is just a wrapper around
font_get(lcd_getfont())-height so it only works on the current
viewports current font, which isnt necessarily what the caller wants.
even if it is, we want get get rid of lcd_getfont() anyway so at a
minimum this wrapper needs replacing.



Re: fredwbauer: r30826 - in trunk: apps apps/gui firmware firmware/export

2011-10-25 Thread Jonathan Gordon
On 25 October 2011 19:49, Thomas Martitz ku...@rockbox.org wrote:
 Am 25.10.2011 09:36, schrieb Jonathan Gordon:

 One question about the patch though: Why separate setfont() and
 setuifont()?
 I imagine one could be sufficient, but perhaps I'm missing something.

 Best regards.

 There are a few issues here:
 The biggest issue is that the lcd API is full of 10 years of
 evolution. First the multiscreen api came in, and then viewports, both
 adding different bits of cruft. (not necessarily bad cruft). lcd_*
 functions *mostly* work on the current viewport (lcd_setfont()
 included), and some work on a specific viewport, and others work on
 the actual display as a whole.

 lcd_setfont() changes the current viewports font number. Should this
 be removed completly? The *only* time the caller doesnt have access to
 the viewport struct is with the default viewport (which we are trying
 to slowly get away from, and this should only happen in plugins now,
 so no, we cant remove it just yet).

 screen_access-setfont() is a wrapper for lcd_setfont() to make sure
 FONT_UI gets set correctly. screen_access-setuifont() does not call
 lcd_setfont() and only sets global_settings.font_ui[screen].
 screen_acces-getuifont() just returns the
 global_settings.font_ui[screen] value (and there is no lcd_getfont()).


 Okay, I understand the difference. But why does there need to be one? In
 your patch you call them both most of the time.

 I understand setfont() sets the current (i.e. default vp) font, and
 setuifont() sets the id which was previously constant (FONT_UI). FONT_UI is
 the alias for the screen's default font which is in fact simply the font of
 the default vp.
 So, from my understanding you want to do both calls at the same time. Unless
 there is a reason to separate these steps which I'm missing, I suggest the
 behavior to be merged in setfont().

 Best regards.


setuifont() should only be called when actually loading a new font
from disk. I didnt really want to add it at all so its just there for
more completeness.


Re: fredwbauer: r30826 - in trunk: apps apps/gui firmware firmware/export

2011-10-25 Thread Jonathan Gordon
On 25 October 2011 16:50, Jonathan Gordon jdgo...@gmail.com wrote:
 Attached is 90% of the work to do this somewhat more nicely.
 The remaining work is to check each plugin and make sure they don't
 use lcd_setfont() directly (which a quick look shows alot do :( ) the
 quick fix for that is just put a wrapper in the plugin api for it like
 what has been added for the screen_access api.


The plugin api already uses the screean access setfont helper so
nothing more should need to be done... everything should just work
(wishful thinking I know :p )


Re: fredwbauer: r30826 - in trunk: apps apps/gui firmware firmware/export

2011-10-25 Thread Jonathan Gordon
On 26 October 2011 00:52, Thomas Martitz ku...@rockbox.org wrote:
 Am 25.10.2011 15:40, schrieb Jonathan Gordon:

 There is an argument that setuifont() must call lcd_setfont(), but if
 setfont() sets global_status that would be wrong. Proof of that is
 what should global_status.font_id[SCREEN_MAIN] be set to after a call
 to screens[SCREEN_MAIN].setfont(FONT_UI) ?


 I thought FONT_UI doesn't exist anymore, because the id isn't constant
 anymore. There comes my confusion from.


Well, some constant is needed for everything to be able to ask for the
setting font, and FONT_UI is annoyingly everywhere already.

 On a slightly related note: Why is the font id in global_status, and not
 e.g. a member of the screen_access api? IMO it would make more sense in the
 latter seeing that font getters/setters are also there.

There is really no difference, global_status was a convenient place to
put it, putting it in the screen_access struct *might* mean it will be
in the cache when accessed, perhaps. This is very hair-splitting,
though if whoever gets to commiting wants to do that I have no
objection.


Re: fredwbauer: r30826 - in trunk: apps apps/gui firmware firmware/export

2011-10-24 Thread Jonathan Gordon
On 25 October 2011 05:16, Fred Bauer fred.w.ba...@gmail.com wrote:
 If this is reverted, can it be delayed until after the release or
 until there is an alternative? (Jonathan said last night on IRC that
 he would work on it.) Several things were broken before r30826 and
 there are a considerable number of files that still expect
 font_get(FONT_UI) to provide the user selected font instead of the
 largest font index of the loaded fonts. Suppose a theme uses a small
 font with a very limited number of glyphs for a WPS numerical display
 and it happens to be the last font loaded by the skin parser. If all
 these files are not rewritten to use global_status.font_id[] the next
 display of  FONT_UI is going to cause the player to saw through the
 hard disk because of all the cache misses. (There can be three disk
 seeks for every cache miss.)

I didnt have time to work on this last night, hopefully I'll post a
patch tonight


 If r30826 is reverted, all the ~40 files that depend on
 font_get(FONT_UI) should be updated to use global_status.font_id[].
 FONT_UI should then be removed from font.h and considering the time
 that FONT_UI has been around, it would also be helpful to put a
 comment in the SVN commit and in font.h that FONT_UI is now deprecated
 so that contributors will not be publicly upbraided for trying to fix
 it when broken.

Relax, noone is being publicly unbraided. And yes, a note abou FONT_UI
being deprecated should have been done with the buflib commit


 Finally, bug hunting is not especially fun. What incentive does
 someone have to contribute to the project if all their ideas are
 promptly rejected without consideration? I'm aggravated. At this
 point, I really feel like I could write a plugin that cured cancer,
 AIDS, and hangovers and JD would reject it. Improve the v-keyboard? JD
 opposed. Allocate a sensible number of glyphs worth of font memory
 instead of over 1250 for an iPod Video (8818 for a Fuze!)? JD opposed.
 The same for skin fonts? JD accepted but then silently reverted. Save
 a glyphcache file per font? JD agreed with that...after he rejected
 the idea and I wrote an alternative. Now he's giving me grief because
 I didn't know about the unpublished deprecation of FONT_UI. Good
 times!


I have no idea what you're talking about. Yes I initially had
reservations with the glyph allocation patch, but you posted numbers
and I shut up (actually I thought i replied to the tracker but
apparently didn't). Bug fixes should never be done secretly, had you
asked on IRC I would have said that your bugfix wasnt ideal. Actually
thats probably a lie because I think the whole lifetime of this patch
was when I was offline, hence me bringing it up on the mailing list.



 This message: [ Message body ] [ More options ]
 Related messages: [ Next message ] [ Previous message ]

 From: Jonathan Gordon jdgordy_at_gmail.com
 Date: Sun, 23 Oct 2011 16:31:50 +1100

 Can this please be reverted? firmware/fonts.c should now know or care
 about the ui font at all, and post buflib fonts it doesnt.
 screen_access.c added a helper to set the font
 (screens[screen].set_font() ) which should be being used by the
 keyboard and lrcviewer. If those are still having issues then that
 helper might need more work. (actually it appears keyboard.c needs to
 use font_get() which has no helper to get
 global_status.font_id[screen].

 Ideally a font_get_height() function be added which (to the screen
 api) to get the correct font height seen as that is what most calls to
 font_get() actually care about.

 On 23 October 2011 04:13, mailer_at_svn.rockbox.org wrote:
  Date: 2011-10-22 19:13:33 +0200 (Sat, 22 Oct 2011)
  New Revision: 30826
 
  Log Message:
  Add functions font_set_ui() and font_get_ui(). The font returned by 
  FONT_UI used to be fixed at zero but since buflib-fonts (r30589) can be 
  different, depending on the order of loads and unloads. Fixes broken 
  behavoir in virtual keyboard (FS#12336), lyrics player (FS#12306), and 
  hopefully, FS#12337
 
 ...
  ___
  rockbox-cvs mailing list
  rockbox-cvs_at_cool.haxx.se
  http://cool.haxx.se/cgi-bin/mailman/listinfo/rockbox-cvs
 
 Received on 2011-10-23

 --
 Fred W. Bauer
 fred.w.ba...@gmail.com





Re: fredwbauer: r30826 - in trunk: apps apps/gui firmware firmware/export

2011-10-24 Thread Jonathan Gordon
Attached is 90% of the work to do this somewhat more nicely.
The remaining work is to check each plugin and make sure they don't
use lcd_setfont() directly (which a quick look shows alot do :( ) the
quick fix for that is just put a wrapper in the plugin api for it like
what has been added for the screen_access api.

On 23 October 2011 16:31, Jonathan Gordon jdgo...@gmail.com wrote:
 Can this please be reverted? firmware/fonts.c should now know or care
 about the ui font at all, and post buflib fonts it doesnt.
 screen_access.c added a helper to set the font
 (screens[screen].set_font() ) which should be being used by the
 keyboard and lrcviewer. If those are still having issues then that
 helper might need more work. (actually it appears keyboard.c needs to
 use font_get() which has no helper to get
 global_status.font_id[screen].

 Ideally a font_get_height() function be added which (to the screen
 api) to get the correct font height seen as that is what most calls to
 font_get() actually care about.

 On 23 October 2011 04:13,  mai...@svn.rockbox.org wrote:
 Date: 2011-10-22 19:13:33 +0200 (Sat, 22 Oct 2011)
 New Revision: 30826

 Log Message:
 Add functions font_set_ui() and font_get_ui(). The font returned by FONT_UI 
 used to be fixed at zero but since buflib-fonts (r30589) can be different, 
 depending on the order of loads and unloads. Fixes broken behavoir in 
 virtual keyboard (FS#12336), lyrics player (FS#12306), and hopefully, 
 FS#12337

 Modified:
   trunk/apps/filetree.c
   trunk/apps/gui/list.c
   trunk/apps/settings.c
   trunk/firmware/export/font.h
   trunk/firmware/font.c

 Modified: trunk/apps/filetree.c
 ===
 --- trunk/apps/filetree.c       2011-10-22 15:28:09 UTC (rev 30825)
 +++ trunk/apps/filetree.c       2011-10-22 17:13:33 UTC (rev 30826)
 @@ -427,7 +427,10 @@
     current_font_id = global_status.font_id[screen];
     if (current_font_id = 0)
         font_unload(current_font_id);
 -    global_status.font_id[screen] = font_load(file);
 +    current_font_id = font_load(file);
 +    if(screen==SCREEN_MAIN)
 +        font_set_ui(current_font_id);
 +    global_status.font_id[screen] = current_font_id;
     viewportmanager_theme_changed(THEME_UI_VIEWPORT);
  }
  #endif

 Modified: trunk/apps/gui/list.c
 ===
 --- trunk/apps/gui/list.c       2011-10-22 15:28:09 UTC (rev 30825)
 +++ trunk/apps/gui/list.c       2011-10-22 17:13:33 UTC (rev 30826)
 @@ -133,6 +133,7 @@
  static int list_get_nb_lines(struct gui_synclist *list, enum screen_type 
 screen)
  {
     struct viewport *vp = list-parent[screen];
 +    vp-line_height = font_get(vp-font)-height;
     int lines = skinlist_get_line_count(screen, list);
     if (lines  0)
     {

 Modified: trunk/apps/settings.c
 ===
 --- trunk/apps/settings.c       2011-10-22 15:28:09 UTC (rev 30825)
 +++ trunk/apps/settings.c       2011-10-22 17:13:33 UTC (rev 30826)
 @@ -886,6 +886,7 @@
                 if (global_status.font_id[SCREEN_MAIN] = 0)
                     font_unload(global_status.font_id[SCREEN_MAIN]);
                 rc = font_load(buf);
 +                font_set_ui(rc);
                 CHART2(font_load , global_settings.font_file);
                 global_status.font_id[SCREEN_MAIN] = rc;
                 lcd_setfont(rc);

 Modified: trunk/firmware/export/font.h
 ===
 --- trunk/firmware/export/font.h        2011-10-22 15:28:09 UTC (rev 30825)
 +++ trunk/firmware/export/font.h        2011-10-22 17:13:33 UTC (rev 30826)
 @@ -55,7 +55,7 @@

  /* SYSFONT, FONT_UI, FONT_UI_REMOTE + MAXUSERFONTS fonts in skins */
  #define MAXFONTS (FONT_FIRSTUSERFONT + MAXUSERFONTS)
 -#define FONT_UI MAXFONTS
 +#define FONT_UI MAXUSERFONTS

  /*
  * .fnt loadable font file format definition
 @@ -124,6 +124,9 @@
  void font_unload(int font_id);
  void font_unload_all(void);
  void font_lock(int font_id, bool lock);
 +/* set the default UI font */
 +void font_set_ui(int font_id);
 +int  font_get_ui(void);

  struct font* font_get(int font);


 Modified: trunk/firmware/font.c
 ===
 --- trunk/firmware/font.c       2011-10-22 15:28:09 UTC (rev 30825)
 +++ trunk/firmware/font.c       2011-10-22 17:13:33 UTC (rev 30826)
 @@ -88,6 +88,7 @@
  };
  static int buflib_allocations[MAXFONTS];

 +static int font_ui = -1;
  static int cache_fd;
  static struct font* cache_pf;

 @@ -559,7 +560,7 @@

     lock_font_handle(handle, false);
     buflib_allocations[font_id] = handle;
 -    //printf(%s - [%d] - %d\n, path, font_id, *handle);
 +    //printf(%s - [%d] - %d\n, path, font_id, handle);
     return font_id; /* success!*/
  }
  int font_load(const char *path)
 @@ -616,30 +617,54 @@

  /*
  * Return a pointer to an incore font structure

Re: fredwbauer: r30826 - in trunk: apps apps/gui firmware firmware/export

2011-10-22 Thread Jonathan Gordon
Can this please be reverted? firmware/fonts.c should now know or care
about the ui font at all, and post buflib fonts it doesnt.
screen_access.c added a helper to set the font
(screens[screen].set_font() ) which should be being used by the
keyboard and lrcviewer. If those are still having issues then that
helper might need more work. (actually it appears keyboard.c needs to
use font_get() which has no helper to get
global_status.font_id[screen].

Ideally a font_get_height() function be added which (to the screen
api) to get the correct font height seen as that is what most calls to
font_get() actually care about.

On 23 October 2011 04:13,  mai...@svn.rockbox.org wrote:
 Date: 2011-10-22 19:13:33 +0200 (Sat, 22 Oct 2011)
 New Revision: 30826

 Log Message:
 Add functions font_set_ui() and font_get_ui(). The font returned by FONT_UI 
 used to be fixed at zero but since buflib-fonts (r30589) can be different, 
 depending on the order of loads and unloads. Fixes broken behavoir in virtual 
 keyboard (FS#12336), lyrics player (FS#12306), and hopefully, FS#12337

 Modified:
   trunk/apps/filetree.c
   trunk/apps/gui/list.c
   trunk/apps/settings.c
   trunk/firmware/export/font.h
   trunk/firmware/font.c

 Modified: trunk/apps/filetree.c
 ===
 --- trunk/apps/filetree.c       2011-10-22 15:28:09 UTC (rev 30825)
 +++ trunk/apps/filetree.c       2011-10-22 17:13:33 UTC (rev 30826)
 @@ -427,7 +427,10 @@
     current_font_id = global_status.font_id[screen];
     if (current_font_id = 0)
         font_unload(current_font_id);
 -    global_status.font_id[screen] = font_load(file);
 +    current_font_id = font_load(file);
 +    if(screen==SCREEN_MAIN)
 +        font_set_ui(current_font_id);
 +    global_status.font_id[screen] = current_font_id;
     viewportmanager_theme_changed(THEME_UI_VIEWPORT);
  }
  #endif

 Modified: trunk/apps/gui/list.c
 ===
 --- trunk/apps/gui/list.c       2011-10-22 15:28:09 UTC (rev 30825)
 +++ trunk/apps/gui/list.c       2011-10-22 17:13:33 UTC (rev 30826)
 @@ -133,6 +133,7 @@
  static int list_get_nb_lines(struct gui_synclist *list, enum screen_type 
 screen)
  {
     struct viewport *vp = list-parent[screen];
 +    vp-line_height = font_get(vp-font)-height;
     int lines = skinlist_get_line_count(screen, list);
     if (lines  0)
     {

 Modified: trunk/apps/settings.c
 ===
 --- trunk/apps/settings.c       2011-10-22 15:28:09 UTC (rev 30825)
 +++ trunk/apps/settings.c       2011-10-22 17:13:33 UTC (rev 30826)
 @@ -886,6 +886,7 @@
                 if (global_status.font_id[SCREEN_MAIN] = 0)
                     font_unload(global_status.font_id[SCREEN_MAIN]);
                 rc = font_load(buf);
 +                font_set_ui(rc);
                 CHART2(font_load , global_settings.font_file);
                 global_status.font_id[SCREEN_MAIN] = rc;
                 lcd_setfont(rc);

 Modified: trunk/firmware/export/font.h
 ===
 --- trunk/firmware/export/font.h        2011-10-22 15:28:09 UTC (rev 30825)
 +++ trunk/firmware/export/font.h        2011-10-22 17:13:33 UTC (rev 30826)
 @@ -55,7 +55,7 @@

  /* SYSFONT, FONT_UI, FONT_UI_REMOTE + MAXUSERFONTS fonts in skins */
  #define MAXFONTS (FONT_FIRSTUSERFONT + MAXUSERFONTS)
 -#define FONT_UI MAXFONTS
 +#define FONT_UI MAXUSERFONTS

  /*
  * .fnt loadable font file format definition
 @@ -124,6 +124,9 @@
  void font_unload(int font_id);
  void font_unload_all(void);
  void font_lock(int font_id, bool lock);
 +/* set the default UI font */
 +void font_set_ui(int font_id);
 +int  font_get_ui(void);

  struct font* font_get(int font);


 Modified: trunk/firmware/font.c
 ===
 --- trunk/firmware/font.c       2011-10-22 15:28:09 UTC (rev 30825)
 +++ trunk/firmware/font.c       2011-10-22 17:13:33 UTC (rev 30826)
 @@ -88,6 +88,7 @@
  };
  static int buflib_allocations[MAXFONTS];

 +static int font_ui = -1;
  static int cache_fd;
  static struct font* cache_pf;

 @@ -559,7 +560,7 @@

     lock_font_handle(handle, false);
     buflib_allocations[font_id] = handle;
 -    //printf(%s - [%d] - %d\n, path, font_id, *handle);
 +    //printf(%s - [%d] - %d\n, path, font_id, handle);
     return font_id; /* success!*/
  }
  int font_load(const char *path)
 @@ -616,30 +617,54 @@

  /*
  * Return a pointer to an incore font structure.
 - * If the requested font isn't loaded/compiled-in,
 - * decrement the font number and try again.
 + * Return the requested font, font_ui, or sysfont
  */
 -struct font* font_get(int font)
 +struct font* font_get(int font_id)
  {
 -    struct font* pf;
 -    if (font == FONT_UI)
 -        font = MAXFONTS-1;
 -    if (font = FONT_SYSFIXED)
 +    struct buflib_alloc_data *alloc;
 +    struct font *pf;
 +    int 

Re: Ladies and gentlemen, we have sound on HiFiMan HM-601.

2011-10-18 Thread Jonathan Gordon
On 18 October 2011 19:14, Francisco Vila paconet@gmail.com wrote:

 Good work. BTW I like this style of announcing, with ladies and
 gentlemen and a log of the first track played and so on.  Gives an
 epic flavour to the whole thing. Thanks


It is quite a tradition here :) http://www.rockbox.org/wiki/GentlemenMails



Re: Trimming the lcd api

2011-10-18 Thread Jonathan Gordon
On 13 October 2011 07:19, Björn Stenberg bj...@haxx.se wrote:
 However there is certainly a point in not requiring all arguments all the 
 time, so two functions make sense. Since they are vararg enabled, I'd say a 
 more suitable name than lcd_puts is lcd_printf and its extended companion 
 lcd_xprintf.

Sounds good.



Re: SUMMARY: FS#10849 - Sleep timer options: persistent duration and start on boot

2011-10-15 Thread Jonathan Gordon
On 13 October 2011 19:48, Thomas Martitz ku...@rockbox.org wrote:
 Am 13.10.2011 10:42, schrieb Thomas Martitz:

 Alright, thanks for the comments discussion. I'll commit part 1) (move TD
 to settings) and 3) (the actual sleep timer remake)  and leave the
 System-About rename out for now.

 That is, in a few hours if there's no further objection.


So one objection isnt enough? commit the sleep timer rework by all
means, but dont move td unless the rest of the discusison is settled.



Re: SUMMARY: FS#10849 - Sleep timer options: persistent duration and start on boot

2011-10-15 Thread Jonathan Gordon
On 15 October 2011 20:53, Björn Stenberg bj...@haxx.se wrote:
 Jonathan Gordon wrote:
 So one objection isnt enough? commit the sleep timer rework by all
 means, but dont move td unless the rest of the discusison is settled.

 Your objection as I understood it was that the patch does less than you'd 
 like, not that what it does is wrong. If that is a misunderstanding, could 
 you please clarify your stance?

 --
 Björn


No, that's correct.



Re: My view on Rockbox for Android

2011-10-10 Thread Jonathan Gordon
On 10 October 2011 20:27, Thomas Martitz ku...@rockbox.org wrote:
 I'd like to add that it's worthwhile to invest into our UI code. We
 shouldn't forget that android isn't the only touch-enabled target. We should
 have something workable on other touch targets as well.

HELLO! I don't think we've met. My name is Jonathan and I've been
secretly working on the UI for the last year or so :)
As of now the skin engine knows exactly which screen has focus (err of
the list of 16 or so avilable) and with nothing more than an sbs I
should be able to implement the context menu dialog Bjorn wants (with
the exception of the touch feedback colouring). The entire UI is now
skinnable to whatever the user wants with very fine grained control.
And anything that it cant do can be added with a few lines in the skin
engine draw code.

I admit there are some unimplemented things which need fixing, but
please don't go blaming rockbox's look on the lack of themeability.

I've been slowly trying to move all GUI drawing from hard coded c to
dynamic drawing with the skin engine. The remaining screens are pretty
much the graphic EQ, colour chooser (no real point there), and the
recording screen.


Re: FS#12321 - Touchscreen: List line padding, to more easily select lines

2011-10-10 Thread Jonathan Gordon
On 11 October 2011 08:45, Björn Stenberg bj...@haxx.se wrote:
 Thomas Martitz wrote:
 I agree it's less than ideal that there are two list
 implementations, and I see the skin engine eventually taking things
 over.
 But as of now there just are two implementations, and the classic
 lists aren't going to go away very soon. Plus the skinned lists have
 bugs and are imcomplete/not capable enough yet. Until that's worked
 out I'd still like to see my patch in for the meantime since it does
 improve usablity a lot _now_.

 Fine, implementing this in the classic list until the skinned list is ready 
 to replace it sounds like a good plan to me.

 --
 Björn


Doing that gives little motivation for someone to finish off the
missing bits of skinned list



Re: FS#12321 - Touchscreen: List line padding, to more easily select lines

2011-10-09 Thread Jonathan Gordon
On 9 October 2011 18:38, Jonathan Gordon jdgo...@gmail.com wrote:
 30min of fiddling around, attached is the screenshot from the e200 sim
 and the patch to do it, notice there is no code needed anywhere
 outside of the skin engine (infact its only done there because that
 pulls in the generated .sbs if none is specified). No actual code
 needs changing.

 Notes about the screenshot:
 * I'm not sure why the images arent being displayed, that might be a
 skin engine bug, more likely PEBCAK.
 * Gradient isnt completly setup in that example

 Now, I haven't seen screenshots from kugels patch, but if it colours
 the whole line instead of just the text, then that can be fixed with
 minor changes to the skin engine (wanted to do that anyway). Though it
 requires the lcd driver get a draw more which is use the current
 pixels for the transparent pixels instead of the backdrop image) which
 IIUC amiconn has mentioned he wants to add for a while now. If kugels
 patch is the same as that screenshot then the above is still something
 which should be done :) (hardly a reason to not go by this
 implementation anyway).


 With a bit more fiddling this could be *easily* extended to *any*
 skinned list theme by just fiddling with the Lb() tag.

 Jonathan


DAMMIT! pressed send before ataching the patch
diff --git a/apps/gui/statusbar-skinned.c b/apps/gui/statusbar-skinned.c
index ad9a391..d2b3a0d 100644
--- a/apps/gui/statusbar-skinned.c
+++ b/apps/gui/statusbar-skinned.c
@@ -192,7 +192,7 @@ void sb_skin_set_update_delay(int delay)
  */
 char* sb_create_from_settings(enum screen_type screen)
 {
-static char buf[128];
+static char buf[512];
 char *ptr, *ptr2;
 int len, remaining = sizeof(buf);
 int bar_position = statusbar_position(screen);
@@ -263,6 +263,21 @@ char* sb_create_from_settings(enum screen_type screen)
 len = snprintf(ptr, remaining, %%ax%%Vi(-,0,%d,-,%d,1)\n, 
y, height);
 }
+remaining -= len;
+ptr += len;
+/* now do the skin list */
+int font_height = font_get(global_status.font_id[screen])-height;
+int padding = 6; // get it from wherever
+int icon_size = get_icon_width(screen);
+len = snprintf(ptr, remaining, 
+%%V(0,0,0,0,0)\n /* %Lb() needs to be in a viewport */
+%%Lb(a,%d,%d)\n /* setup the list viewport rectangle */
+%%Vl(a,0,%d,%d,%d,-)\n%%xl(i,__list_icons__,%d,%d)\n%%?LI%%LI%%xd(i,%%LI)\n /* the icon */
+%%Vl(a,%d,%d,%d,%d,1)\n%%?Lc%%Vs(gradient)%%s%%LT\n /* draw the text */,
+screens[screen].getwidth(), font_height+padding*2,
+padding, icon_size, icon_size,icon_size, icon_size,
+icon_size+2, padding, screens[screen].getwidth() - icon_size+2 - icon_size, font_height);
+printf(%s, buf);
 return buf;
 }
 


Re: FS#12321 - Touchscreen: List line padding, to more easily select lines

2011-10-09 Thread Jonathan Gordon
On 9 October 2011 21:03, Thomas Martitz ku...@rockbox.org wrote:
 Am 09.10.2011 09:38, schrieb Jonathan Gordon:

 30min of fiddling around, attached is the screenshot from the e200 sim
 and the patch to do it, notice there is no code needed anywhere
 outside of the skin engine (infact its only done there because that
 pulls in the generated .sbs if none is specified). No actual code
 needs changing.

 Notes about the screenshot:
 * I'm not sure why the images arent being displayed, that might be a
 skin engine bug, more likely PEBCAK.
 * Gradient isnt completly setup in that example

 Thanks for this, this actually proves that the skin engine is largely unable
 to do it currently.

 Except for the points you mentioned, if you tried it on a touchscreen target
 (what this patch is all about) you would have realized that this is
 completely broken (no kinetic scrolling, no scrollbar, no list title, I even
 got it to crash).

 That the line selector draws over the line and not only the text is a
 significant portion of the patch which is needed in any event.

 FWIW, even if the skin engine would be able to do it. This patch isn't about
 theming. It makes Rockbox usable independantly of the current theme (though,
 it doesn't prevent themes from overriding it).

 Best regards.


Did you completly ignore the part where I said it was a *quick proof*
that your patch is wrong? I've already admitted that the skin engine
is lacking some minor issues *which need to be fixed anyway*. Of
course instead of fixing actual issues you want to do another half
assed and wrong patch. :


Re: FS#12321 - Touchscreen: List line padding, to more easily select lines

2011-10-09 Thread Jonathan Gordon
On 9 October 2011 21:32, Thomas Martitz ku...@rockbox.org wrote:
 Am 09.10.2011 12:24, schrieb Jonathan Gordon:

 Did you completly ignore the part where I said it was a *quick proof*
 that your patch is wrong? I've already admitted that the skin engine
 is lacking some minor issues *which need to be fixed anyway*. Of
 course instead of fixing actual issues you want to do another half
 assed and wrong patch. :

 The issues I found are not minor. I can add to the list that the item you
 click is not what's being selected (it looks like the code that calculates
 the selection doesn't take the empty space into account), which is a pretty
 major issue.


 Why are you saying it's another half assed patch?

 Best regards.


The issues you mentioned are things that need to be fixed on skin
lists anyway (or that i didnt bother implementing in the 5 line
snippet).

Take your pick:
Its a half assed patch because:
a) Either its a usability feature - in which case completely ignoring
the skin engine makes no sense as its hard to argue that the skin isnt
the thing that the user interacts with 100% of the time. Or is a theme
issue in which case doing it only for the inbuilt list is absolutely
incorrect as it is sort of being phased out in favour of skinnable
lists (or indeed native lists).
b) It's a quick band aid fix for your single use case simply because
noone has got round to fixing the issues which prevent this being done
by the skin engine (which need to be done anyway), in effect adding
nothing but bloat and more hacks into an already messy API (the
lcd/viewport framework).

I wouldnt be arguing so aggressively if I hadnt spend the better part
of the last X years working *in the code which this deals with* if
that doesn't count for anything then I don't see the point of the
thread

I have no issue with the actual feature, just the implementation (and
even then, the configuration bit is fine).
To do it correctly:
* get kinetic scrolling working in skin lists (actually, move it up
one layer so it works directly on the list widget and not the drawn
list. I don't know how it works or what needs fixing to make it work
(needs to be done anyway so is irrelevant to this topic)
* get a lcd draw mode working so transparent doesnt use the background
image (again, something which has been a wishlist item for ages, and
mostly irrelevant to this topic)
* add the logic to automatically create the sbs (finish my attached patch).
* Add logic to extend custom skins to automatically deal with this
(trivial to do, just resize the %Lb rectangle)

Nothing I've said is hard or contentious, just a different way of
dealing with the problem.

Kindest regards


possible fix for skin buffer issues

2011-10-09 Thread Jonathan Gordon
Braindump for someone to pick up to fix the skin buffer memory
allocation issue.

The problem is that The vast majority of the skin buffers static
allocation goes to the struct skin_element which is allocted for every
single item in the skin file. Every line, tag, subline, comment, etc.
on e200 this comes out to 41KB (going from memory). The next biggest
allocation is struct skin_token, which is 20x smaller in total than
skin_element.

So obviously there is no point doing anything except move skin_element
onto buflib.
This is *problematic* because they are used, well everywhere.

My braindump fix is this:
1) give the lib a callback which will allocated 1000 skin_element
structs in a block (we obviously dont want 600 allocations for this)
2) in skin_alloc_element() allocate from the above array instead of
the skin_buffer (when it is full allocate another 1000 or so)
3) in skin_render_viewport() (and a few other places) lock all those handles
4) in the move callback, walk the entire skin tree (this means every
tree loaded) and update each pointer. This is going to be slow and
crap, but hopefully not done often.

I *think* those items are only used when traversing the tree so it is
possible the pointer updating could even be done delayed (so store the
diff somewhere and update the first time it is traversed after the
move - Though this could be very dangerous, actually yeah, dont do
this. hidden viewports are ignored at render time).

I'm not sure what the alternatives are, this is a pretty nasty
solution. Hopefully someone comes up with a better one (or implements
this as I don't particularly have interest in doing it right now, but
thought it would be good to explain the issue and a solution.)

Jonathan


Re: possible fix for skin buffer issues

2011-10-09 Thread Jonathan Gordon
On 9 October 2011 23:48, Thomas Martitz ku...@rockbox.org wrote:
 Am 09.10.2011 14:37, schrieb Jonathan Gordon:

 3) in skin_render_viewport() (and a few other places) lock all those
 handles

 Is locking even needed? The lcd functions (except the final
 lcd_update()/_rect()) don't yield().

 Best regards.


This is where we disagree Locking when not needed is far better
than not locking when it is.


Re: SUMMARY: FS#10849 - Sleep timer options: persistent duration and start on boot

2011-10-09 Thread Jonathan Gordon
On 10 October 2011 01:38, Thomas Martitz ku...@rockbox.org wrote:
 Bah,
 Move the whole System menu into settings and call it whatever you
 want. It really doesnt deserve such a high placement in the menu
 system

 Is this a strong objection, or just stating that the patches don't go far
 enough in your opinion?

 Best regards.


My position since the start of this thread has not changed. Any patch
which doesnt remove either settings or system from the top level
should not be done. I don't care what it gets called or how they are
merged, but moving one item s a waste of time and jut means it is less
likely they will get merged properly.


Re: FS#12321 - Touchscreen: List line padding, to more easily select lines

2011-10-08 Thread Jonathan Gordon
On 8 October 2011 05:18, Thomas Martitz ku...@rockbox.org wrote:
 Hello folks,

 I finally uploaded the patch that's lived long in my git repo (although in
 very hacked together fashion). I'm making this post here too since I want to
 get it in quickly. So please have a play with it and speak up if you have
 objections.

 This patch adds line padding to lists on touchscreens, in order to make
 lists reasonable useful without huge fonts.

 It's configurable:
 * Automatic (default, line height calculated using a lcd dpi aware
 function)
 * Off (status quo, line height = font height)
 * X pixels (from 2 to 50 in even steps)

 The automatic setting should/aims to Just Work Out Of The Box on all
 targets

 IMO this patch is needed to make Rockbox on touchscreen/RaaA remotely
 usable.

 Please comment or make suggestions!


 Best regards.


This will probably be ignored but meh.
This is the wrong patch for 2 reasons:
1) this can already be done in the themes
2) the only reason this is wanted at all is because Raaa doesnt use
native lists. The effort shold be put into making native lists work
(which is trivial once someone figures out how to draw the List in the
Canvas rockbox uses. androids list api and rockbox's is pretty
similiar (similar enough that a mostly thin wrapper is all that is
needed).


Re: FS#12321 - Touchscreen: List line padding, to more easily select lines

2011-10-08 Thread Jonathan Gordon
On 8 October 2011 23:36, Dave Hooper d...@beermex.com wrote:
 If this is already possible in the theme, then I agree with Jon and don't
 see the need for the patch. Putting the padding into the theme itself makes
 more sense to me, since the theme author will already appreciate what
 spacing works/doesn't work, for that specific device, for that specific
 theme.

I will admit that putting it into the theme makes it more annoying for
users to modify, but then they will have to do it anyway for any theme
which uses skined lists (as this patch would break all themes if it
wasnt ignored by skins, which iiuc it is).
However, that is a very fixable solution, either in code at parse (or
even run)time, or with a script on the PC to modify a skin file (or a
plugin on the DAP).
Further, the internal sbs is already autogenerated from the user
settings, so even if this (mis)feature was really wanted, all it would
require is the setting and half a dozen lines in
apps/gui/statusbar-skinned.c  (in sb_create_from_settings() ) to setup
the list viewport correctly. This would also then allow
apps/gui/bitmap/list.c to be heavily butchered as *all* lists would be
drawn by the skin which means much cleaner code and no more code
duplication.

Jonathan


Re: FS#12321 - Touchscreen: List line padding, to more easily select lines

2011-10-08 Thread Jonathan Gordon
On 9 October 2011 07:43, Torne Wuff torne+rockbox...@wolfpuppy.org.uk wrote:
 On 8 October 2011 11:47, Jonathan Gordon jdgo...@gmail.com wrote:
 On 8 October 2011 05:18, Thomas Martitz ku...@rockbox.org wrote:
 * Automatic (default, line height calculated using a lcd dpi aware
 function)

 1) this can already be done in the themes

 The automatic setting *can't* currently be done in themes, since
 themes are only resolution dependent, not dpi dependent. Android
 devices have a pretty wide range there...

 --
 Torne Wuff
 to...@wolfpuppy.org.uk


Is that a challenge? :) everything about this patch can already be
done in svn, and the configuration can be added. This is *not* a
useability thing because it is only going to work reliably (err, at
all) for themes which *dont* use skinned lists (which hopefully wont
be very many once themers get the hang of them).


On 9 October 2011 07:31, Frank Gevaerts fr...@gevaerts.be wrote:
 1) While it's true that there has been work in that direction, I don't
   think it's entirely fair to say it can be done today. Skinnable
   viewports still have some issues, and I'm not convinced we have all
   infrastructure in place to fully implement the default lists.


nothing impossible to fix.

Anyway, dead horse beaten. This patch adds nothing which cant already be done.



Re: SUMMARY: FS#10849 - Sleep timer options: persistent duration and start on boot

2011-10-08 Thread Jonathan Gordon
On 9 October 2011 12:31, Thomas Martitz ku...@rockbox.org wrote:
 Am 23.08.2011 01:08, schrieb sideral:

 So here's the plan:

 * Move the entire Time  Date menu out of System to Settings

 * Rename System to About

 * In the Time  Date menu:
   * Sleep Timer offers the last-used timer value as its default.
     (This value is made persistent by way of the settings code.)  When
     the timer is running, the entry changes to Cancel Sleep Timer
     (hh:mm), showing the remaining time.

   * Add Start sleep timer on boot option after Alarm Wake up Screen.



 I uploaded a patch series to the task doing exactly this. I intend to commit
 it within the next week (perhaps wednesday) someone speaks up against
 (please also do if you have already voiced your opionion).

 I just don't want this to die.

 Best regards.


Bah,
Move the whole System menu into settings and call it whatever you
want. It really doesnt deserve such a high placement in the menu
system



Re: jdgordon: r30599 - in trunk/apps: . gui/skin_engine

2011-10-05 Thread Jonathan Gordon
On 5 October 2011 18:19, Thomas Martitz ku...@rockbox.org wrote:
 Am 28.09.2011 09:00, schrieb Thomas Martitz:

 TBH, I would like to revert this commit.

 Best regards.


 Alright, as a few people agreed with me I'm going to revert this in a few
 hours.

 Best regards.



Go ahead, just remember you're then responsible for breaking users
ability to load their otherwise working themes. Frankly the 2 users
whom I've personally helped trumps your (very likely personal) reasons
against the change.

Heck, read the thread, you're the only one who has said anything about
the actual feature, everyone else just doesnt like the implementation
(myself included to be completely honest). But unless you're going to
do something about the apparent ram waste, or reimplement this is a
nicer way, then you reverting is just a dick thing to do.

Best regards to you too :)

Jonathan


Re: jdgordon: r30599 - in trunk/apps: . gui/skin_engine

2011-10-05 Thread Jonathan Gordon
On 5 October 2011 18:50, Hayden Pearce saint.lascivi...@gmail.com wrote:
 I found the wording of a few people agree, so I'm reverting amusing.
 The amount of times a few people agreed something should be committed, and
 it wasn't, most often blocked by the same one or two people, vs the ease of
 the descision to revert something here is quite funny.

 sits back

I think the word you want is irony :)


Re: jdgordon: r30599 - in trunk/apps: . gui/skin_engine

2011-10-05 Thread Jonathan Gordon
I already explained why it was needed, noone replied to that email so
apparently noone actually cares.
I see its been reverted, oh well.

On 6 October 2011 01:23, pondlife pondl...@pondlife.me wrote:
 is just a dick thing to do

 Hrmf, don't be a shitweasel.*

 Reverting this is definitely not personal, I thought you agreed that this
 was a badly implemented semi-secret feature too.

 I'm sure those 2 users can use 3.9.1 in the meantime, while we try to get to
 a fully stable 3.10 release.

 pondlife

 p.s. I apologise, sincerely**.

 (*Ironic use of entirely inappropriate personal abuse, for which I should
 apologise.)

 (**Non-ironic; I really don't like this personal abuse thing - I'm sure
 there are dedicated MLs elsewhere for that sort of thing.)






Re: jdgordon: r30599 - in trunk/apps: . gui/skin_engine

2011-10-05 Thread Jonathan Gordon
On 6 October 2011 14:10, Dave Hooper d...@beermex.com wrote:
 I did reply to your email. I could repost my response if you think it would
 be useful.

You replied to the thread, but not to the email explaining the situation.

Someone may as well go and bump the #define in skin_engine.h now as
I'm pretty confident noone is going to bother working on a better
solution.


Re: Im out

2011-10-01 Thread Jonathan Gordon
On 28 September 2011 21:54, XavierGr xavie...@gmail.com wrote:
 On 28 September 2011 10:15, Jonathan Gordon jdgo...@gmail.com wrote:

 I'm saying this publicly because I don't want anyone to think I'd
 simply lost interest in the project or found another itch elsewhere. I
 very much have stuff I want to keep working on and am leaving because
 of (mostly) 2 individuals. I'll continue working on rockbox on my git
 repo, and of course you're free to cherry-pick the changes you want,
 but I wont be pushing changes back anymore (not regularly anyway).

 If you are going to continue working on Rockbox on a personal repository,
 why do you have to leave? Your contribution to this project has been huge
 all these years, why deny future features from the userbase for just 2
 controversial people.
 It is certainly frustrating to have to develop under the pressure of such
 thankless and sometimes aggressive behaviour, but we are talking about a
 very small (albeit vocal) minority. In the end, it mostly sounds like you
 want to avoid further controversy and not that you just lost interest to the
 project, which for me seems rather unfortunate.


If I continue fiddling with rockbox it will be outside of the main
repo simply because it means I can do the changes I want without being
accused of pushing horrible aggressive agendas. It feels like every
discussion I've been involved in the last 6+ months has simply turned
to poo-flinging with the same few individuals. I'm not saying I didnt
throw my share too but untill recently I did my best to keep emotion
out of it. After however many months of trying to keep things civil I
just don't see the point anymore.

Probably the biggest frustration was the majority of devs who simply
said nothing. Obviously the constant arguments reflect badly on the
community but noone seemed to want to throw in their weight and force
an outcome (whether it be drop it or let him do the change). The
RSB is supposed to be a post-crisis response but it has failed in that
by the time it is called it is already too late.

My second biggest annoyance is the loud minority of non-contributing
devs who stand in the way of progress. (of course non-contributing
dev is a bit of an oxymoron, but people who did some work
months/years ago and do little more than chat in irc really should
learn to shut up and get out of the way of the active developer
group).

Finally, the general aggression amongst some of the regular devs. This
includes a disturbingly high level of passive aggression in irc.
Until people can be trusted to keep an aggressive tone out of simple
discussions there is simply no point talking to them. (case in point
is kugels reply to the playback gsoc thread, or the amazing response I
got to the *suggestion of talking about* of code reviews in gerrit).

I know for sure some people are reading this thinking I'm begin a
massive hypocrite, but then those people are the very reason I've
left.

So, To the people nice enough to say thanks and try to try my mind,
fix the above and we'll see (I'd be incredibly surprised if anything
changes).
I do apologise for the timing a bit, I know the font commits brought
some instability which I feel a bit bad about (Another issue with the
group is the tiny proportion of devs willing to test other devs'
patches), but it looks like there is always someone else ready to pick
up the ball.

I'll continue to be in IRC and I'll happily reply to rockbox related
emails for the code that I wrote/maintained.

Jonathan


Re: jdgordon: r30599 - in trunk/apps: . gui/skin_engine

2011-09-28 Thread Jonathan Gordon
On 28 September 2011 17:24, pondlife pondl...@pondlife.me wrote:
 TBH, I would like to revert this commit.

 FWIW, I agree.  A solution is needed to this memory waste, but this isn't
 it.

 pondlife





Very quickly, the skin buffer always was a memory hog, but now that
the big items (images, fonts) are in buflib there is completly no
sensible size that can be used. something like 50% of the usage is now
on the tree elements which are alloced once for *each* item, ilike on
the nano does 41KB for that while cabbie does 15ish. there is no way
to reduce this by enough to make it worth it (reducing the struct by 4
bytes still isnt enough to make a real dent).

if someone were to make the whole skin system buflib aware then ok,
but that isnt going to happen, this solution (admittedly quirky) is
the only solution that allows people to enjoy *big* themes without
forcing people who want very bare theme to lose a considerable amount
of buffer space.

So go ahead and revert it if you all care that much. I ignored the
thread initially because of the attitude and not the suggestion. Had
it been brought up in a civil manner then things may be different
(Though unlikely).

Hope someone picks up skin_engine maintenance soon.



Re: jdgordon: r30599 - in trunk/apps: . gui/skin_engine

2011-09-26 Thread Jonathan Gordon
I'm happy to talk about technical issues...

On 26 September 2011 16:15, Amaury Pouly amaury.po...@gmail.com wrote:
 Hi All,

  Furthermore this user setting is implemented in a totally obscure way (a
  magic file containing a magic number, which isn't checked if it's even
  plausible). It's pattern that doesn't exist yet, while there's a clear
  existing implementation of user settings. It's rather hidden without a
  chance to be discovered by the user. I don't think I need to mention
  that the manual doesn't tell anything about this.

 I don't like the way this setting is implemented, we have a framework for
 that and see no good reason to not use ot.

 Regards


It doesnt really sit well with the current setting system, and I need
to double check settings are loaded early enough (which is mostly the
reason I did it this way). I'll have another look though as yes if it
can use config.cfg it would be better.


Re: The next release version

2011-09-24 Thread Jonathan Gordon
a) 3.10
b) because history says that whatever I suggest will be outright rejected

On 25 September 2011 09:59, Alex Parker parker.ale...@gmail.com wrote:
 Hi guys,

 So the next release is due towards the end of October, and in the past there
 has been some discussion as to whether the version number should be 3.10 or
 4.0.  Myself I lean towards 3.10, and keeping 4.0 for e.g. an Android
 release but I am not too determined either way.

 What do people think? I'd appreciate a two part answer:

 a) 3.10 or 4.0
 b) why?

 Cheers chaps, looking forward to your input :)

 Alex




Re: The next release version

2011-09-24 Thread Jonathan Gordon
serious response:
a) 4.0
b) 1 - there has been plenty of stuff commited in the last release
which gives us a good enough reason to bump the major number
 2 - 3.10 looks a bit funny
 3 - no other target has caused a major bump so why should android
be different?
 4 - the numbers are pretty arbitrary anyway

On 25 September 2011 09:59, Alex Parker parker.ale...@gmail.com wrote:
 Hi guys,

 So the next release is due towards the end of October, and in the past there
 has been some discussion as to whether the version number should be 3.10 or
 4.0.  Myself I lean towards 3.10, and keeping 4.0 for e.g. an Android
 release but I am not too determined either way.

 What do people think? I'd appreciate a two part answer:

 a) 3.10 or 4.0
 b) why?

 Cheers chaps, looking forward to your input :)

 Alex




Re: The next release version

2011-09-24 Thread Jonathan Gordon
2011/9/25 Jonas Häggqvist ras...@rasher.dk:
 +1. 2.x-3.x was when SWCODEC was added (and initially 3.0 was planned for
 Archos+Iriver Hxx0 only IIRC). I'd say the addition of (releasable) Raaa is
 on the same level.


From a technical point of view, RaaA is a pretty uninteresting target.
SWCODEC brought an entirely new playback engine, RaaA brings very
little. On the other hand the user-hidden changes which have gone in
recently easily could qualify for a major bump (of course the number
is entirely artificial anyway so whatever).
Things like the software mixer which *finally* fixed the very long
standing bug of no voice while playback was paused, buflib in core,
skinlists (so the entire ui is themeable), etc



FS#12273 - load fonts into buflib needs testing!

2011-09-18 Thread Jonathan Gordon
Hi all,
http://www.rockbox.org/tracker/task/12273 Is very ready for mass
testing so please everyone get it into your private builds and help
out!
What this does is use buflib instead of a static array for *all* fonts
loaded by the system. This is a big patch and im not really willing to
just commit and fix bugs later.

All you need to do to test is put it in your build and use your DAP as
usual. changing themes and fonts would be useful too! :)

I'd like to commit this before the end of the week.

There is one known issue where changing themes could make one line in
the theme chooser list dissapear for some unknown reason! leaving
the menu clears it up, and as I have no idea where its coming from,
and it isnt a crash bug I'm not too concerned, though if anyone can
reliably crash it I'd be very intersted in repro steps.

Cheers
Jonathan

P.S please remember to get the latest version of the patch :)


Re: Git/gerrit migration status and next steps

2011-09-07 Thread Jonathan Gordon
On 7 September 2011 00:40, Torne Wuff torne+rockbox...@wolfpuppy.org.uk wrote:

 4) We write up policies and documentation on how to use git via
 gerrit, though only two parts are crucial at this point: how to clone
 the repositories, and how committers can commit directly to master.


Do we really want to allow commits directly? so little of our code
ever gets reviewed so I would quite happily force everyone to go
through gerrit and require someone else to OK it. It isnt hard to get
someone else in IRC to have a quick look and push the button.


RE: Git/gerrit migration status and next steps

2011-09-07 Thread Jonathan Gordon
On Sep 7, 2011 5:43 PM, bryan.chi...@rbs.com wrote:

  Do we really want to allow commits directly? so little of our code ever
gets
  reviewed so I would quite happily force everyone to go through gerrit
and
  require someone else to OK it. It isnt hard to get someone else in IRC
to have
  a quick look and push the button.

 Absolutely against requiring this.

 We trust committers to do the right thing.

 If you have  a patch you think needs this - submit to refs/for/master.

 For small commits, this would be a completely pain in the arse, and
totally unwarranted.


 Bryan Childs

This isn't about people doing the wrong thing. This is about trying to get
code quality. And yes it will be a minor pain sometimes, but a small fix has
just as much fuck-up-ability as a big one and is *easier* to sanity check
then the required debugging later.

This is something gerrit makes easy and we should absolutly do it. Remember
that you push directly to gerrit, this doesn't require you to open your
browser.


Re: Git/gerrit migration status and next steps

2011-09-07 Thread Jonathan Gordon
On 7 September 2011 21:02, Nils Wallménius nils.wallmen...@gmail.com wrote:
 On Wed, Sep 7, 2011 at 10:53 AM, Thomas Martitz ku...@rockbox.org wrote:
 Am 07.09.2011 10:29, schrieb Nils Wallménius:

 I think it could be interesting to test it actually.



 Testing it will probably show that it works in the beginning, but it will
 not show that this way degrades to clicking away commits like ads (without
 even looking) in the long (or even short) term.

 We can't force people to actually review. And we can't force them to do it
 properly. This way has no hope of working as intended.

 What it may work for is to open the ability to point at someone else for
 every controversial/bad/unwanted commit. This even reduces the motivation to
 review (and to work on Rockbox in general).

 Best regards.


 That's the whole point of a test, to see how it works out, you seem
 certain that it *can't* work which i find overly negative. Of course
 we can't force people to do anything but i think this will encourage
 people to review, if it turns out it doesn't work, we disable it.

 I don't see why people have focused on the blame-sharing as someone
 called it in IRC how many commits do we get that are actually bad?

 Anyway, i agree with Torne about taking one step at a time.

 Nils



I honestly have no idea how the whole blame shareing thing happened
either. anyone who does software engineering should know that review
means implementation review and not feautre review.
I also mentioned it now because it is a valuable thing to talk about,
I gess I should use an alias now seen as obviously everything I
suggest is so unbeleivably wrong!



Re: Git/gerrit migration status and next steps

2011-09-07 Thread Jonathan Gordon
On 7 September 2011 21:38, Nils Wallménius nils.wallmen...@gmail.com wrote:

 I guess we will see if people use the review stuff voluontarily but i
 expect not as it's easier to just push directly (as we do it now). If
 everyone has to get their changes reviewed i think that would make
 people more inclined to review other people's chnges. But that's just
 a guess and as i said i would find it interesting.

 Nils


My intuition says that commiters will use it exactly like flyspray,
i.e very rarely. I am looking forward to being able to push new
contirbuters patches more quickly though.



Re: Settings reordering.

2011-09-03 Thread Jonathan Gordon
On 25 August 2011 22:50, Paul Louden paulthen...@gmail.com wrote:
 Okay, what I'd like to see here is this:

 People who actually want to directly contribute to this process
 (filling out a list of categories at each step, etc), please respond
 to this saying so, so that we can come up with dates and get this
 thing moving, as well as trying to begin selecting our conflict
 resolution group (which we won't need immediately, but will need at
 the end of the first phase).


Well, its been 9 days with nothing happening. This is pretty much dead
in the water unless people who have said they want to help start doing
something.


Re: Discussion regarding reordering the main/root menu

2011-08-29 Thread Jonathan Gordon
On 29 August 2011 16:22, Johannes Linke johannes.li...@gmx.de wrote:
 As users will need Settings way more often than System, it seems absurd to
 give System a higher priority and move Settings one level down.


From my origional email:

As a compromise I would accept removing the settings submenu and
moving all those items up one level if that was what was more
preferred.

--

I also dont necessarily agree with your statement


Re: Discussion regarding reordering the main/root menu

2011-08-29 Thread Jonathan Gordon
On 29 August 2011 03:13, Al Le al...@gmx.de wrote:
 That said, a patch to even just allow simple reordering of the main
 menu isnt going to be trivial so we should figure out a better layout

 I once tried to do exactly that (in FS#6718 (only the main menu) and FS#7809
 (general menu reordering with the option of hiding)), but both patches were
 rejected. I don't remebmer why exactly -- because it is (or was) a forbidden
 feature or because of the way it was implemented.


Just looking at FS#6718 now (it probably still applies cleanly!). It's
not the way I would have done it, but I see no reason other than our
customisation-phobia to not include it. a plugin frontend could easily
be added to make it more user friendly also. I'm a bit scared to look
at the other one though. I read automatic reordering and get shivers
:)

Anyway, I'd like to try and talk about the static order first, then
(or at least separately) talk about giving the user the choice.

I know this thread hasnt been going long but it would be nice to hear
from more people. we have half a dozen very similar proposals and
conservative guesses suggest 40+ active developers of which most havnt
said a word one way or another.


Discussion regarding reordering the main/root menu

2011-08-28 Thread Jonathan Gordon
*Everyone* agrees Time  Date does not belong in system as it
currently is. Hypothetically, if it were moved out of System and put
into Settings it begs the question as to the point of the system menu
at all. Most people agree Rockbox Info is the most important item in
that menu, and running time can be removed (I've heard noone spak
out against that suggestion). The remaining two items are debug (keep
out) and credits.

My view regarding debug is that there is nothing there that is
actually useful once a port is up and running *or during development*
when dealing with custom builds anyway so disabling that by default
(but adding an easy configure item to enable it) removes clutter.
Credits is only there because it should go somewhere and nowhere else
makes any more sense.
So hypothetically system is down to just rockbox info and
credits. Why are they given such a high priority by being directly
accessable from the main menu?

Equally, why are the settings given such high priority in the main
menu? Sure they are useful, but after the initial player setup how
many settings does one need access to? and given how poor the setting
layout is it is likely regularly used configs are saved in .cfg anyway
(and put on the quickscreen).

My suggestion has always been that System and Settings be merged in
some meaningful way.
My favored layout is then:
System:
Rockbox info
Time  Date
Settings
   Sound settings
   playback settings
   general settings
   recording settings
   theme settings
   manage settings
Credits
Debug

As a compromise I would accept removing the settings submenu and
moving all those items up one level if that was what was more
preferred.

Now, while we are here it makes sense to talk about the order of the
other items deamed to be the most important parts of the UI.

I think it makes sense to group items based on what they give the
user. Files and Database are Find music to play and should be
grouped. WPS, radio, recording are listen to music and should be
grouped. System and Plugins are misc and should be grouped. Playlist
catalog and bookmarks are find previously saved playlists and should
be grouped.

The order I'd like to see is then:
Files
Database
WPS
Recording
FM
System
Plugins
Playlist Catalogue
Bookmarks

Just about every player has direct access to the wps/fm screen with a
single button so they are sensible to put in the middle.
Files/Database are the most often accessed (based on a number i pulled
from my tuches), system is the least important and so in the middle
below music, PC and bookmarks are relativly important and thanks to
the list wrapping very accessible.

An alternate main menu which I would like moves P.C and bookmarks
between database and WPS so all the ways to find music are grouped
together (though thinking about it the first one makes more logical
sense to me).


There are two final options. Do nothing. And move TD into settings
and call it done. My views of both are well know hence the above
proposals.

Discuss


Re: Discussion regarding reordering the main/root menu

2011-08-28 Thread Jonathan Gordon
On 28 August 2011 23:22, Paul Louden paulthen...@gmail.com wrote:

 Equally, why are the settings given such high priority in the main
 menu? Sure they are useful, but after the initial player setup how
 many settings does one need access to? and given how poor the setting
 layout is it is likely regularly used configs are saved in .cfg anyway
 (and put on the quickscreen).

 I don't really think your speculation on what the most likely way
 users use the settings is means much. The honest truth is, we don't
 know. People have asked for a quickscreen that can hold more settings,
 meaning that there's at least 5 settings they want to change
 regularly. There are many people who didn't even know you could save
 custom .cfg files. We have no real way of knowing how the typical user
 experiences Rockbox. Even a poll would give a decidedly skewed result.
 We should be making our decisions based on something else. Perhaps
 what will help a new user most easily find what they need. I'm not
 saying that explicitly, but rather, forward-looking ideas rather than
 I think this is how users already use our software guesses.


That is all a failure of the manual then, that's not to say the menus
aren't to blame also (we agree the settings menus suck), but not
understanding config files is a pretty fundamental failure of the
manual.


 I think it makes sense to group items based on what they give the
 user. Files and Database are Find music to play and should be
 grouped. WPS, radio, recording are listen to music and should be
 grouped. System and Plugins are misc and should be grouped. Playlist
 catalog and bookmarks are find previously saved playlists and should
 be grouped.

 I don't see how grouping is beneficial. Is the idea that they need to
 be in groups to make it easier for users to spot them? Are users even
 going to recognize that playlist catalog and bookmarks are the
 same group? I certainly don't see bookmarks as a playlisting function,
 at least. Recording isn't related to listening to music at all, and
 is arguably the exact opposite. The groups seem a little arbitrary in
 the first place, but I don't see a reason grouping is the approach
 we should take with this list.


Well of course the list is entirely arbitrary. My suggestion was to
try to add some logic to it. I can't be the only one to have an
opinion on this, so why dont people respond with their ideas?

 To me, rather, two things should be taken into account. What's visible
 on the screen on our smallest screen targets with the default theme,
 and what individual items are most likely to be sought. The screen
 visibility seems important because when a user first turns on our
 player, especially during the period they're unfamiliar, they're going
 to want to glance at the screen and see useful options. Not remember
 whether they need to go up or down to get to the ones they want. I
 think it's convenient for the most used options to be the least
 presses away, but visibility on boot is a little more important.


Indeed that should be a consideration but not *the* major one.

 That being said, the below list isn't bad. On the Clip+ though, the
 menu is six items I believe. This means Recording, FM and System are
 visible on boot where Bookmarks and Playlist Catalogue are not. I'd
 actually recommend moving FM below WPS, and squeezing those two
 between FM and Recording. I don't mean to belittle the value of
 recording, as such, but many of these devices don't even have the
 feature, and even those that do are mostly sold as music players.
 Features like Bookmarks and Playlists should be made more
 visible/accessible. Someone who's bought a unit primarily for
 recording purposes probably checked that Rockbox can still record
 before installing it, and will scroll down to look for it..


Bookmarks is hidden by default.

Now the real question is, why are we so bone-headedly against
customisable (main)menu layouts? I personally never use the database,
recording or fm so why must they have priority in my list? I know for
sure others use the database primarily and see the files as being
arrogantly in the wrong place.
Additionally, why are we so against changing the quickscreen to be a
list for those that want it like that?

That said, a patch to even just allow simple reordering of the main
menu isnt going to be trivial so we should figure out a better layout,
or admit we have no clue and leave it as it - which still leaves us
with the question of system/settings merging.


 The order I'd like to see is then:
 Files
 Database
 WPS
 Recording
 FM
 System
 Plugins
 Playlist Catalogue
 Bookmarks




Re: Discussion regarding reordering the main/root menu

2011-08-28 Thread Jonathan Gordon
On 29 August 2011 02:04, Johannes Linke johannes.li...@gmx.de wrote:
 Hi,

 why can't you just move System into Settings? Ok, credits are not a setting,
 as well as Rockbox info isn't. But look at this from a users point of view:
 meaningful menu item names are for remembering where to look for something.
 If you're searching for settings, you will find them quickly because the
 menu item is called Settings.

 But who searches for Credits? Nobody. And who searches for Rockbox info? The
 only info which is relevant to a user is battery level and amount of space
 used on disk. The battery level is shown in the statusbar and in most WPS's,
 so I woudln't worry about that. And since most people do massive data moving
 with their PC, i wouldn't consider disk space used to be problematic.


 My proposal: Move System into Settings. The name inconsistency is
 negligible, as names are for remembering where to find certain things, but
 nobody needs to find System. I'm quite certain that not a single user will
 complain about that move. The only problem I see is, that some developers
 can't stand inconsistencies.


 Regards,

 Johannes


I find it very hard to imagine a user who was cluey enough to find
rockbox but then not be able to find the settings under the system
menu. Gnome2 puts everything under System, Windows put its under
Control Panel. Bad names are always bad when there are alternatives.


Re: SUMMARY 2: FS#10849 - Sleep timer options: persistent duration and start on boot

2011-08-27 Thread Jonathan Gordon
On 28 August 2011 06:15, Thomas Martitz ku...@rockbox.org wrote:

 Its unfortunate that you let the single hypocrite win. Really everyone else
 agreed with moving td to settings.

 Perhaps I'll just move it afterwards...just cause.

 Best regards.


how to win friends and influence people (y)


Re: Remove the System menu!

2011-08-24 Thread Jonathan Gordon
On 23 August 2011 18:29, Jonathan Gordon jdgo...@gmail.com wrote:
 Hi,
 This is my view on the other thread about moving the Time  Date menu.
 Get rid of the System menu. it is entirely pointless.

 The items hat will be in it once TD is moved (which sounds like a
 given) are: Rockbox info, credits, running time, debug.
 Rockbox info has very little actually useful information, credits can
 be got from the plugins menu, running time is nothing more than a
 waste of binary space, and debug serves basically no purpose and
 should be disabled by default (We've already talked about disabling it
 in release builds).

 What I'd much prefer to see is the current settings menu be renamed
 to system and either: 1) settings be a submenu, or 2) just copy
 all the items from the current system menu into the new
 settings/system menu.

 The main menu order should then be:
 * (hidden bookmarks)
 * Files
 * Database
 * playlist catalogue
 * resume playback
 * plugins
 * system

 and now the main menu makes some sense. Items are prioritized from the
 top down starting with ways to get to your music on to less important
 items. Resume playback i pushed down because just about all targets
 have a dedicated button to get there. I'd be happy to swap plugins and
 system.

 If the system menu didnt have settings as a submenu it would look
 like this (making it very obvious what is and isnt a setting):
 * rockbox info
 * sound settings
 * playback settings
 * general settings
 * Time  Date
 * theme settings
 * manage settings
 * running time (I'd rather nuke this though)
 * debug menu (remove also)
 * Credits


 Jonathan


As usual, the only way to get any discussion is to commit and fend off
the hordes in IRC after. So, This patch will be commited tonight (if I
have time) or sometime over the weekend.
All this does is move the contents of the settings menu into the
system menu, and move the system menu to where settings currently is.
It also changed the rockbox info icon. No other changes.

With this done the sleep timer patch can put all its settings in the
TD screen with no issues.

Jonathan
diff --git a/apps/menus/main_menu.c b/apps/menus/main_menu.c
index c5758d1..91d6411 100644
--- a/apps/menus/main_menu.c
+++ b/apps/menus/main_menu.c
@@ -108,6 +108,32 @@ MAKE_MENU(manage_settings, ID2P(LANG_MANAGE_MENU), NULL, Icon_Config,
 /**/
 
 /***/
+/*MAIN MENU*/
+
+
+#ifdef HAVE_LCD_CHARCELLS
+static int mainmenu_callback(int action,const struct menu_item_ex *this_item)
+{
+(void)this_item;
+switch (action)
+{
+case ACTION_ENTER_MENUITEM:
+status_set_param(true);
+break;
+case ACTION_EXIT_MENUITEM:
+status_set_param(false);
+break;
+}
+return action;
+}
+#else
+#define mainmenu_callback NULL
+#endif
+/*MAIN MENU*/
+/***/
+
+
+/***/
 /*  INFO MENU  */
 
 
@@ -390,7 +416,7 @@ static bool show_info(void)
 return simplelist_show_list(info);
 }
 MENUITEM_FUNCTION(show_info_item, 0, ID2P(LANG_ROCKBOX_INFO),
-   (menu_function)show_info, NULL, NULL, Icon_NOICON);
+   (menu_function)show_info, NULL, NULL, Icon_Rockbox);
 
 
 /* sleep Menu */
@@ -440,50 +466,22 @@ MENUITEM_FUNCTION(show_runtime_item, 0, ID2P(LANG_RUNNING_TIME),
 MENUITEM_FUNCTION(debug_menu_item, 0, ID2P(LANG_DEBUG),
(menu_function)debug_menu, NULL, NULL, Icon_NOICON);
 
-MAKE_MENU(info_menu, ID2P(LANG_SYSTEM), 0, Icon_System_menu,
+MAKE_MENU(info_menu, ID2P(LANG_SYSTEM), mainmenu_callback, Icon_System_menu,
+show_info_item,
+sound_settings,
+playback_settings,
+settings_menu_item, theme_menu,
+#ifdef HAVE_RECORDING
+recording_settings,
+#endif
+manage_settings,
 #if CONFIG_RTC
   timedate_item,
 #endif
-  show_info_item, show_credits_item, show_runtime_item, 
 #if CONFIG_RTC == 0
   sleep_timer_call, 
 #endif
+  show_credits_item, show_runtime_item, 
   debug_menu_item);
 /*  INFO MENU  */
 /***/
-
-/***/
-/*MAIN MENU*/
-
-
-#ifdef HAVE_LCD_CHARCELLS
-static int mainmenu_callback(int action,const struct menu_item_ex *this_item)
-{
-(void)this_item;
-switch (action)
-{
-case ACTION_ENTER_MENUITEM:
-status_set_param(true);
-break;
-case ACTION_EXIT_MENUITEM:
-status_set_param(false);
-break;
-}
-return action;
-}
-#else
-#define mainmenu_callback NULL
-#endif
-MAKE_MENU(main_menu_, ID2P(LANG_SETTINGS), mainmenu_callback,
-Icon_Submenu_Entered,
-sound_settings,
-playback_settings,
-settings_menu_item, theme_menu,
-#ifdef HAVE_RECORDING

Remove the System menu!

2011-08-23 Thread Jonathan Gordon
Hi,
This is my view on the other thread about moving the Time  Date menu.
Get rid of the System menu. it is entirely pointless.

The items hat will be in it once TD is moved (which sounds like a
given) are: Rockbox info, credits, running time, debug.
Rockbox info has very little actually useful information, credits can
be got from the plugins menu, running time is nothing more than a
waste of binary space, and debug serves basically no purpose and
should be disabled by default (We've already talked about disabling it
in release builds).

What I'd much prefer to see is the current settings menu be renamed
to system and either: 1) settings be a submenu, or 2) just copy
all the items from the current system menu into the new
settings/system menu.

The main menu order should then be:
* (hidden bookmarks)
* Files
* Database
* playlist catalogue
* resume playback
* plugins
* system

and now the main menu makes some sense. Items are prioritized from the
top down starting with ways to get to your music on to less important
items. Resume playback i pushed down because just about all targets
have a dedicated button to get there. I'd be happy to swap plugins and
system.

If the system menu didnt have settings as a submenu it would look
like this (making it very obvious what is and isnt a setting):
* rockbox info
* sound settings
* playback settings
* general settings
* Time  Date
* theme settings
* manage settings
* running time (I'd rather nuke this though)
* debug menu (remove also)
* Credits


Jonathan


Re: Remove the System menu!

2011-08-23 Thread Jonathan Gordon
On 23 August 2011 18:29, Jonathan Gordon jdgo...@gmail.com wrote:
 Hi,
 This is my view on the other thread about moving the Time  Date menu.
 Get rid of the System menu. it is entirely pointless.

 The items hat will be in it once TD is moved (which sounds like a
 given) are: Rockbox info, credits, running time, debug.
 Rockbox info has very little actually useful information, credits can
 be got from the plugins menu, running time is nothing more than a
 waste of binary space, and debug serves basically no purpose and
 should be disabled by default (We've already talked about disabling it
 in release builds).

 What I'd much prefer to see is the current settings menu be renamed
 to system and either: 1) settings be a submenu, or 2) just copy
 all the items from the current system menu into the new
 settings/system menu.

 The main menu order should then be:
 * (hidden bookmarks)
 * Files
 * Database
 * playlist catalogue
 * resume playback
 * plugins
 * system

 and now the main menu makes some sense. Items are prioritized from the
 top down starting with ways to get to your music on to less important
 items. Resume playback i pushed down because just about all targets
 have a dedicated button to get there. I'd be happy to swap plugins and
 system.

 If the system menu didnt have settings as a submenu it would look
 like this (making it very obvious what is and isnt a setting):
 * rockbox info
 * sound settings
 * playback settings
 * general settings
 * Time  Date
 * theme settings
 * manage settings
 * running time (I'd rather nuke this though)
 * debug menu (remove also)
 * Credits


 Jonathan


The general consensus in IRC seem to be merging the two menus is a
good idea and we are tied up on how and what to call it.
Attached are 3 diffs:
1) move_settings_into_system.diff - does what it says, makes the
settings menu a submenu of system
2) merge_menus.diff - merges the items of the two menus with a bit of
order tweaking and calls it system in the main menu
3) merge_and_call_settings.diff - same as 2 except the menu name is
Settings. This is to keep people happy although I think this is
absolutly wrong.
diff --git a/apps/menus/main_menu.c b/apps/menus/main_menu.c
index c5758d1..467f477 100644
--- a/apps/menus/main_menu.c
+++ b/apps/menus/main_menu.c
@@ -108,6 +108,32 @@ MAKE_MENU(manage_settings, ID2P(LANG_MANAGE_MENU), NULL, Icon_Config,
 /**/
 
 /***/
+/*MAIN MENU*/
+
+
+#ifdef HAVE_LCD_CHARCELLS
+static int mainmenu_callback(int action,const struct menu_item_ex *this_item)
+{
+(void)this_item;
+switch (action)
+{
+case ACTION_ENTER_MENUITEM:
+status_set_param(true);
+break;
+case ACTION_EXIT_MENUITEM:
+status_set_param(false);
+break;
+}
+return action;
+}
+#else
+#define mainmenu_callback NULL
+#endif
+/*MAIN MENU*/
+/***/
+
+
+/***/
 /*  INFO MENU  */
 
 
@@ -390,7 +416,7 @@ static bool show_info(void)
 return simplelist_show_list(info);
 }
 MENUITEM_FUNCTION(show_info_item, 0, ID2P(LANG_ROCKBOX_INFO),
-   (menu_function)show_info, NULL, NULL, Icon_NOICON);
+   (menu_function)show_info, NULL, NULL, Icon_Rockbox);
 
 
 /* sleep Menu */
@@ -440,50 +466,22 @@ MENUITEM_FUNCTION(show_runtime_item, 0, ID2P(LANG_RUNNING_TIME),
 MENUITEM_FUNCTION(debug_menu_item, 0, ID2P(LANG_DEBUG),
(menu_function)debug_menu, NULL, NULL, Icon_NOICON);
 
-MAKE_MENU(info_menu, ID2P(LANG_SYSTEM), 0, Icon_System_menu,
+MAKE_MENU(info_menu, ID2P(LANG_SETTINGS), mainmenu_callback, Icon_System_menu,
+show_info_item,
+sound_settings,
+playback_settings,
+settings_menu_item, theme_menu,
+#ifdef HAVE_RECORDING
+recording_settings,
+#endif
+manage_settings,
 #if CONFIG_RTC
   timedate_item,
 #endif
-  show_info_item, show_credits_item, show_runtime_item, 
 #if CONFIG_RTC == 0
   sleep_timer_call, 
 #endif
+  show_credits_item, show_runtime_item, 
   debug_menu_item);
 /*  INFO MENU  */
 /***/
-
-/***/
-/*MAIN MENU*/
-
-
-#ifdef HAVE_LCD_CHARCELLS
-static int mainmenu_callback(int action,const struct menu_item_ex *this_item)
-{
-(void)this_item;
-switch (action)
-{
-case ACTION_ENTER_MENUITEM:
-status_set_param(true);
-break;
-case ACTION_EXIT_MENUITEM:
-status_set_param(false);
-break;
-}
-return action;
-}
-#else
-#define mainmenu_callback NULL
-#endif
-MAKE_MENU(main_menu_, ID2P(LANG_SETTINGS), mainmenu_callback,
-Icon_Submenu_Entered,
-sound_settings,
-playback_settings,
-settings_menu_item

Re: Settings reordering.

2011-08-23 Thread Jonathan Gordon
I've already put my hand up to help out. Ideally as many comitters as
possible say they are either in principle backing the committe and
waiting for the results, against the whole notion of this change, or
abstain completly (including the final vote).

This is going to be alot of work for those who get involved and
knowing what sort of opposition there is going to be from the start is
important.

It would also be good to get people who want to be activly invovled to
put your hands up soon (i.e before the weekend) because we'd like to
have this done for the next release feature freeze which is currently
set for oct 17.


On 23 August 2011 23:45, Paul Louden paulthen...@gmail.com wrote:
 The overall deadline for the project would be the beginning of the
 feature freeze for the next release, so that the new menu layout can
 be available with the next Rockbox release.

 Again, this whole process is up for comments before we start. If we
 get enough people willing to commit to contributing their suggestions
 at each phase, we'll take it and go. If you object to the results of
 the phase after the phase is over, don't waste time starting
 arguments, just vote against it at the end. If the version contributed
 to by everyone is voted down, then we can entertain voting on
 contributed .diffs, but the real goal here is to use a process by
 which we incorporate a little bit of everyone's input into something
 that is, if never near perfect, better than what we have now.


Just on that last paragraph. Te vote should be a simple yes/no, with
no veto-ing possible (unlike RFC-type posts which are expected for
normal features).

This is going to be alot of work., and we want as many people to be *
actively involved* as possible, but we dont want people who dont have
an open mind, or only want to do little changes.

So, If you want in let us know. If you feel there is nothing wrong
with the menus then that is fine, but please stay out of our way.


Re: Settings reordering.

2011-08-23 Thread Jonathan Gordon
On 24 August 2011 00:05,  bryan.chi...@rbs.com wrote:
 I think it's worth trying to set a guideline for the maximum desirable menu 
 depth too - I don't think we'd want to go to fine grained for everything 
 unless it's just otherwise unworkable?


Yes, but lets agree that we are going to do this first, then that and
other questions need to be answered. Max list length is another
important one


Re: Remove the System menu!

2011-08-23 Thread Jonathan Gordon
On 23 August 2011 22:39, Paul Louden paulthen...@gmail.com wrote:
 You should probably also mention that included in those diffs is also
 a rearranging of the main menu as you mentioned in your first message,
 but left out in your summarized descriptions here.


Like you said, it was mentioned in the first post, to which noone has
replied. I see this patch as orthogonal to the settings work and would
like to get a answer on it to help the sleep time patch go through
smoothly also.
If that means taking out the playlist catalogue swap then so be it.


Re: SUMMARY: FS#10849 - Sleep timer options: persistent duration and start on boot

2011-08-22 Thread Jonathan Gordon
On 23 August 2011 09:08, sideral side...@rockbox.org wrote:
 Given that it looks like there's overwhelming support for (and little to
 no concern about) moving Time  Date to Settings, I now think it's fine
 to do that change along with the proposed sleep-timer extensions.  Also,
 I'd like to pick up Thomas Martitz's proposal and get rid of an explicit
 setting for the default sleep time in favor of the sleep-timer menu
 offering the last selected value as its default value.

 So here's the plan:

 * Move the entire Time  Date menu out of System to Settings

 * Rename System to About

 * In the Time  Date menu:
  * Sleep Timer offers the last-used timer value as its default.
    (This value is made persistent by way of the settings code.)  When
    the timer is running, the entry changes to Cancel Sleep Timer
    (hh:mm), showing the remaining time.

  * Add Start sleep timer on boot option after Alarm Wake up Screen.

 sideral



I disagree with the first part of this change.
Time  date doesnt make any more sense in settings than it does in
system (or info if you want to rename it). timedate isnt a
setting. And it does make sense to keep anything related (sleep timers
and alarms) together,



Re: SUMMARY: FS#10849 - Sleep timer options: persistent duration and start on boot

2011-08-22 Thread Jonathan Gordon
On 23 August 2011 10:27, Paul Louden paulthen...@gmail.com wrote:
 On Mon, Aug 22, 2011 at 7:10 PM, Jonathan Gordon jdgo...@gmail.com wrote:

 I disagree with the first part of this change.
 Time  date doesnt make any more sense in settings than it does in
 system (or info if you want to rename it). timedate isnt a
 setting. And it does make sense to keep anything related (sleep timers
 and alarms) together,



 Unless Time and Date became immutable while I wasn't looking and can't
 be set by the user, I'd say they're settings. It could possibly be
 argued that the screen where you view the time and date could be
 separated from the setting that allows you to set them, but there is
 absolutely a setting involved. The design of the setting menu may be
 non-standard in that it, for some reason, previews the values by
 showing them on layer above where you set them, but that doesn't
 instantly make it not a setting.


What has how you set it got anything to do with it being a setting or
not? Time does not magically change when you give your DAP to your
sibling.


  1   2   3   4   5   6   7   >