All Projects

It was reported on IRC (http://www.rockbox.org/irc/log-20140916#03:37:18) that there are playback problems (e.g. gaps between gapless songs) if/when Gather Runtime Data is active and playback is started via database.

This panic just occurred in an application build with “(buffering != audio)” with SDL threads.

Backtrace
1: mutex_unlock()
2: sim_read()
3: buffer_handle()
4-5: <somewhere in buffering.c>

How can this happen. Shouldn’t mutexes be 100% safe?

This patch reworks powermgmt.c and related files to allow it to compile for hosted targets.

Since it assumed the ability to read voltage, I made it aware of other concepts in order to make it useful on RaaA. Some targets can only read battery level (android), some can read level & estimated time (Maemo, making our own estimation superflous). And some can read nothing (SDL app).

This patch introduces new defines CONFIG_BATTERY_MEASURE and compiles powermgmt.c for all of them, including the simulator. This makes hosted/powermgnt.c unneeded. Runtime estimation works with voltage and level reading, provided a battery capacity is given. power_history[] tracks battery level if voltage can’t be measured.

Furthermore, the behavior of the sim is changed to implement a targets API (i.e. _battery_voltage()) instead of re-implementing powermgmt.c so it’s much closer to real targets now.

Rename battery_adc_voltage() to _battery_voltage() for consistency.

So, I would like some tests, and naturally comments are welcome as well.

Now that there’s alpha blending support for images, I would like to replace the icons in SVN.

The attached patch modifies the script to pick better icons from tango icon set, and changes the clef logo a bit to have a border (so it doesn’t look huge compared to the other icons).

IMO these icons look really nice and are definitely an improvement, and actually effectively needed on RaaA.

I put this on the tracker because it’s a not yet finished (viewer icons missing) and there’s one concern I have: alpha blending is a bit slower. I’m unsure if it’s fast enough on older targets to universally replace SVN’s 12×12 icons. AAF on e200v1 are indeed slower (meaning less responsive UI), but I haven’t noticed any difference with the icons alone.

The attached icons should work in SVN so give them a try.

The second patch adds the ability to read and use alpha channel information in 32bit bitmaps. The font drawing function is re-used to enable alpha blending. I tested it with BMPs created by imagemagick and gimp, it’s so far enabled for iconsets and skin engine (I don’t think it works with resizing, so no support for it in album arts).

The first patch adds two APIs, lcd_bmp() and lcd_bmp_part() and is required for the second. It’ll most probably committed anyway. It simplifies drawing bitmaps that come out of read_bmp_fd/_file.

Please give it a test and comment. I attach also 3 iconsets for testing. I tested in a few sims (e200, h300, h100, mr500) and RaaA so far.

This patch adds the ability to show a horizontal line separator in lists, as a visual indicator for the list lines.

It’s configurable in the theme menus, where it can be enabled and its color be changed.

As this is a theme setting, cabbiev2 is updated to include this setting. I think it includes it on all targets because I don’t think wpsbuild has the magic to make stuff touchscreen specific.

Apply  FS#12321  before this.

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!

This patch adds buflib (and the core_ wrappers) to the core and replaces almost all buffer_alloc() and buffer_get_buffer() [see  FS#12159 ]. The remaining calls are in target-specific code which I cannot verify so I’m post-poning the replacement there.

This patch depends on  FS#12159 .

Replace all direct accesses to audiobuf with buffer API functions.

Namely, introduce buffer_get_buffer() and buffer_release_buffer().
buffer_get_buffer() aquires all available and grabs a lock, attempting to
call buffer_alloc() or buffer_get_buffer() while this lock is locked will cause a panicf() (doesn’t actually happen, but is for debugging purpose).
buffer_release_buffer() unlocks that lock and can additionally increment the audiobuf buffer to make an allocation. Pass 0 to only unlock if buffer was used temporarily only.
buffer_available() is a replacement function to query audiobuflen, i.e. what’s left in the buffer.

Further changes happened to mp3data.c and talk.c as to not call the above API functions, but get the buffer from callers. The caller is the audio system which has the buffer lock while mp3dat and talk mess with the buffer.
mpeg.c now implements some buffer related functions of playback.h, that no #ifdefs are needed anymore around audio_get_buffer().

audiobuf and audiobufend are local to buffer.c now.

I put up a few dircache related patches for review/discussion.

0001-Dircache-Remove-dircache_entry-name_len.patch:
- This removes the name_len memeber from struct dircache_entry. This saves 4 bytes per file in the cache which is easily a couple of KB. The name_len was intended as a means to save strlen() calls but that was not really useful as the shortcut was hardly used. The patch also replaces memcpy with strcpy calls to ensure nul-termination (needed to make strlen() work).

0002-Dircache-Rework-and-simplify-dircache_copy_path.patch:
- The only place where name_len was useful was dircache_copy_path(), for the final string length calculation. This patch rewrites the function to use recursion so that the length calculation isn’t needed and it doesn’t need to go through the linked list twice anymore.

0003-Dircache-Return-the-size-of-the-result-string-in-dir.patch:
- Extends the above dircache_copy_path() to return the string length so that callers don’t need it anymore.

0004-Fix-dircache_size-calculation-bug-introcuced-3-revis.patch:
- just fixes a bug introduced by the 1st patch (separate patch to not mess with my local git history).

0005-Dircache-Change-internal-cache-layout.patch:
- This changes the layout of the dircache. struct dircache_entrys are allocated from the beginning, the d_name strings from the back. This allows the buffer to be moveable/relocatable which is beneficial for h100 users (as the dircache_root pointer doesn’t need to be the same across reboots anymore) and ultimatively needed for my gsoc buflib project.

The mod metadata parser checks if the file in question is a mod file in a very poor manner: not at all. This is a problem when building the database on a PC, because any file that has .mod is accepted, including those text files generated during Linux kernel compilation.

Based on MikiMod, I made this patch to fix it. Please let me know if it rejects any *valid* mod file. It should now accept the same files that the MikMod plugin accepts.

This patch adds load_/store_context for PC platforms to enable us to use our own threading library.

With this patch we can finally simulate our own cooperative threading in the simulator and get rid of the somewhat hacky SDL thread implementation.

For unix platforms I use make/swapcontext which is widely available; for win32 targets Fibers are used (available since win2k). The swapcontext implementation even allows us to check stack usage, although the stack usage on a PC is higher than on target so it might not be overly useful.

tools/configure probes swapcontext support and falls back to the SDL thread if it’s not available; win32 builds always use the Fibers.

Tested with fuze simulator and mingw/wine so far.

This adds track information to the notification system on android in to ways:
a) on track change, the notification on the bar announces the new track (<title> - <artist>)
b) when the notifications are expanded then it’ll show the track information with title in bold, similarly to the built in media player.

IMO this both is done better than most existing media player apps (they don’t a) at all and my b) looks more nicely)

The patch is quite big because it adds some kind of notification api. Do you think it’s OK or too much?

This patch changes the hardcoded scroll threshold to be runtime evaluated and DPI-dependant.

It should make list much more usable, especially on high dpi screens.

On Android, it calls the corresponding API. On all other touchscreen I reimplemented Android’s the calculation, so the effect should be about the same.

The first uses wakeup_wait() instead of polling for waiting for the YesnoActivity result.
The second uses our lang strings for yes and no on the buttons in the dialog.

This adds kinetic list scrolling to touchscreen targets and RaaA, like all other OSes on touchscreens have. It’s only enabled in the absolute point mode, so for non-touchscreen and in grid mode nothing changes.

I would like to see comments from cowond2 and onda users to see how it performs there. Other comments are appreciated as well too of course.

For each target and each affected plugins, the corresponding keymaps in the manual need to be updated.

This task should serve as a place for comments on my gsoc work. I also plan to upload patches here occasionally, although my work will always be available at http://repo.or.cz/w/kugel-rb.git.

See also: http://www.rockbox.org/wiki/RockboxAsAnApplication2010

With this patch the wheel is read via IRQ (like on the e200v1). It feels a tad bit better than the polling solution from the fuzev1 IMO (the difference is smaller than I expected though).

I’m not entirely happy with the coding style, but it’s difficult since the isr for GPIOA must be shared with the sd driver for microsd monitoring.

Please give it a try, scrollwheel acceleration has lots of parameters which can be used to tweak it. Also comment on my solution to share the isr.

This patch modifies skin loading and buffering so that it enables resizing of the skin buffer.

Skin parsing happens in 2 passes now.
- the first pass parses everything but fonts and images (i.e. everything that fits into a sane backup buffer)
- the second pass loads the bitmaps and fonts (the filenames are saved, so the second pass doesn’t mean reparsing)

Upon parsing, the first pass reports the size needed. if it exceeds the existing buffer it asks for a reboot. in any event, it will drop a .skin_engine_%d file where %d has the size needed. Upon reboot, the skin buffer will use this for its size. A 10k backup buffer is always added.

This is all complicated because of the skin buffer architecture (bidirectional) but seems to work.

2 issues:
- backdrop handling is a bit stupid right now, it will assume loading one even if the skin reuses an already buffered one. that’s not a big issue as it will be right after the next reboot (the parsing at boot dumps the real size, not the one reported by the first pass).
- everything is pretty messed up if the buffer wasn’t enough, like corrupted images etc. this needs better handling.

This bug is very apparent on the Fuze, I assume it relates to the small audio buffer size.

If you have track skip set to “skip to outro” you can skip to the almost end of the song (~5s before).
If you do so, everything is good. but if you then skip back again to the beginning, weird this happen. The display can corrupt, data aborts can happen (the simulator simply segfaults) and more.

I’ve tracked down this bug to buffer_handle().
r23680 commented a part out which should handle overwriting the next handle (it didn’t work). This can indeed happen (but shouldn’t). Accessing that handle later leads to bad effects.
The other problem is, that when rebuffer_handle() runs (skipping to the beginning on the fuze leads to rebuffering often because of the small buffer), the WPS isn’t notified and doesn’t refetch metadata. This leads to garbage in the next track info [at the end of the song it gets the next track metadata from the buffer, but at the beginning it gets it from the static mp3entry struct. when skipping back it still tries to get the data from the buffer).

My patch tries to fix both problems. I’m not entirely sure if it’s correct but I can’t reproduce the problems on my fuze.

I’ll commit soon if nobody spots a problem or blames me for coding shit.

This patch greatly improves SD driver performance, at the cost of memory.

The memory cost is zero for 8MB targets, because the sd buffer is used to IRAM, which is nearly unused as of now (this isn’t visible in the RAM usage which rockbox-info.txt gives). It’s about 10k for 2MB targets (additionally the buffer cannot be in IRAM on those).

I’ve made some analysis on different settings: increasing the aligned_buffer, moving it to iram, not using it for properly aligned transfers. The outcome of the analysis is visible here: http://www.alice-dsl.net/simonemartitz/rockbox/fuze_sd_performance.pdf

It turns out that a big ( aligned_buffer at IRAM gives a huge (~250%) transfer rate increase over SVN. A 32*SECTOR_SIZE buffer gives good performance too, but is noticable slower. Using the passed buffer (if word aligned) directly together with cache coherency functions is very fast too, but only increases performance on aligned transfers.

So, I decided to use the fastest setting on 8MB Samsas (64*SECTOR_SIZE buffer at IRAM).
on 2MB samsas I used a combo of 32*SECTOR_SIZE and using the passed buffer directly (this saves 15k RAM, at the cost of speed, code complexity and messing with the caches).

There’s 2 problems: a) This separates the way the driver works for the Samsas (not so much code wise, but logic wise), I don’t really like that.
b) the code path for 2MB targets gives data and prefetch aborts on my clip, but works fine on my fuze. I have no idea why!

I honestly think the 2MB targets, being flash devices, should also use the fastest setting. That would cost 25k over SVN (this patch adds 10k anyway, so it’s effectively only 15k more RAM), but I think the unified code paths and better performance are worth it IMO.

I’ve some code here that creates a dedicated section for init functions (i.e. those that are only called once and very early), inspired by the Linux Kernel.

There’s little point in keeping that code in otherwise usable RAM. That’s why my patch would copy move that code into a section that’s overwritten later.

For example, as my current patch works for e200v1: The code is in the same address area as bss. Before bss is zeroed, the init code is copied to the codec buffer. After calling main(), the init code is called from the codec buffer. The bss, following sections are moved over the init code, which effectively frees the code size init functions take. I use a very similar approach as iram code does.

On my e200, it yields ~6.7k. I would like to know if this kind of self-modifying code is considerable for SVN, or if it’s too dirty and/or dangerous. -6.7k isn’t *that* much.

Where the init code lands (codec or plugin buffer), and if the mechanism is really used, is entirely up to 1 ifdef in config.h and the target’s app.lds.

So, I noticed a visual glitch with the Azure Ultimate theme (http://themes.rockbox.org/index.php?target=e200) where a grey bar would show in the progressbar viewport (which is defined above). See http://imagebin.org/62908 (this is actually with the custom statusbar patch, but the same problem happens in SVN with the non-custom-sb version of the theme).

It turns out that it was caused by r22618.

I moved the progressbar viewport away, which made that grey bar show up in the viewport that was above progressbar one. I also saw the grey bar is actually the top few pixels of a the keypad locked image (the keypad locked image shows instead of “Now Playing” at the very top if hold is on).

This means: The this image is in a conditional viewport, and shows up in the viewport defined above. It also shows in the viewport it’s defined in.

I’ve attached a patch which seems to fix it (the line is suspicious because an index should be always num_* -1 (if index is starting with 0, and num_ with 1) ? ). But I have no idea if that’s the proper fix. I also have no idea why it apparently only happens with condition viewports, but that may just be my theme which triggers it.

If you enable both together (and actually change the timestrech/preamp value, 100%/+0.0dB works fine since they’re effectively inactive then), the output is plain garbage.

Using either is fine.

r22554

BTW: Did anyone think of moving limiter preamp into the replaygain setting with a somewhat better name (such as intelligent preamp?).

So, this is my work on a custom statusbar. It uses the full skin engine, and accepts all WPS tags.

It replaces the classic statusbar, but you can still use the old one. It integrates as 1) an extra setting (statusbar: custom) and a skin file (<theme>.[r]sb) which is specified with sb: <path/to/file>.sb (like wps files). If it’s placed next to a corresponding wps file, it uses the same image folder.

It mostly uses the existing statusbar framework w.r.t. to updating and stuff, but the sb file loads regardless of the setting (unless you specify statusbar: -, i.e. an invalid filename), so that one can toggle between classic and custom statusbar seamlessly.

It works on remotes independently of the main screen.

However, there are 2 rules.
1) You must use viewports. Statusbars without viewports are not supported. The default viewport is deactivated (but you can still define images within it). A fullscreen default viewport as the wps has totally messes up list and wps drawing.
2) It is highly recommended that you specifiy a UI viewport the statusbar is supposed to work with (using the new %Vi tag). This makes it possible to use any theme with any statusbar, since the vp behind %Vi will be copied into the UI viewport. The WPS will also use this as default viewport. Not using %Vi is likely to cause nasty drawing problems if no custom ui viewport is used (and with the wps also).

I changed wpsbuild.pl to allow for pre-packaging custom statusbar enabled themes, but I guess we need to discuss if we really want to do that. The cabbiev2.176x220x16.sb I added is for testing and is probably not going to be committed.

Added 2 test themes. The first is for e200. The second is for fuze/h300/ipod color. It uses slick_letitblue for the main screen, and a cabbiev2.rsb and .rwps for the remote.

EDIT: You can always get the latest changes from my git repo: http://repo.or.cz/w/kugel-rb.git?a=shortlog;h=refs/heads/statusbar

This rewrites the code to extend the output of the (standalone) mkamsboot, so that OF version and model version (e.g. v4 for m200v4) are shown.

I reworked PLA to only offer two contexts, one of which is for remotes.

The aim is to remove the currently pretty useless solution. It has so many contexts that tend to clash on targets in various situations.

That means, this patch greatly simplifies the system, giving only 1/2 contexts with a minimal set of buttons.

With this patch, the actions are:
PLA_UP*, _DOWN*, _LEFT*, _RIGHT*, _START* and _QUIT. Additionally, scrollwheel targets have _SCROLL_FWD* and _SCROLL_BACK*

* plus the according _REPEAT action.

Affected plugins: Bubbles, demestify, dice, fire, jackpot, maze, mazezam, metronome, robotfindskitten, rocklife and clock

I looked into the manuals for the targets to decide whether my mapping makes sense, but I guess it still needs testing.

Another thing, we can consider adding a _MENU* action, but it was not really needed, and I’m not sure every target has another free button for that.

After this is in, I’d like to convert more plugins to PLA (mostly demos), as it greatly simplifies and adds a good deal of consistency.

You enabled timestretch and when into the pitchscreen → timestretch mode.

If you didn’t use speed corrected pitch before, speed-only change will not work. It creates silence on my e200, the song elapsed time goes really fast and sometimes the next track info is messed up.

Observed on r21305 and r21304.

This hopefully fixes the buttonlight for the mentioned targets.

It’s annoying how much hackery in the SD driver is needed for it to work, but I think I got it.

Please test and report if you notice weirdnesses.

This vastly improves the lcd_write_data() performance on my fuze (from <50fps to 90fps unboosted [numbers with  FS#10048  appled) by filling the FIFO to full and using an interrupt to detect that it is full.

I’m getting blue bars on the screen if lcd updates are happening when boosted, this however might be due to  FS#10048  applied.

Weird: I can’t get above exact 100.0 fps with boosted. We either hit the maximum of the FIFO or miss some bits in the Fuze’s LCD initialization.

Please try to adapt for e200v2 (and others) and report numbers.

This patch adds pictureflow to the database’s album note to start it.

Additionally it makes you able to start playback within pictureflow. It does so by creating a new playlist and starting it (overwriting the current one).

It doesn’t feature the context menu though (i.e. no insert/queue actions). That can hopefully be added to pictureflow later.

It works pretty well for me. Tell me if it’s committable (I hope it is :D )

This patch uses the tick_task timer to poll the scrollwheel every 5ms (the tick tasks stay at every 10ms of course).

This makes the scrollwheel on my fuze pretty much perfectly usable.

I’d like to hear opinions (from e200v2 owners too). It’s a bit of a hack, but I’m fairly sure it cannot be done better (without an interrupt, that is), so I hope it’s acceptable.

So, I have a folder, which contains 1 big music file and its cue sheet. I have repeat set to all.

When I skip the last song, so that the first is played again, the metadata is incorrect. Incorrect means: sometimes totally messed up (such as showing a square, instead of the artist), sometimes read from the directoy and file names.

When I let the last song play, so that the first song is played automatically, the metadata is incorrect too, but only for 2-3 seconds or so.

In either way, the metadata is correct after skipping another song.

The cue sheet contains correct meta data and works fine for skipping normally through the big file
noticed on r20481.

This patch implements the idea that came up here: http://forums.rockbox.org/index.php?topic=20739.0

Read the topic for my reasonings, why I made this patch.

Short summary: It separates the tracklock feature of skip length out to a seperate setting, so that skip length is actually usable too for normal music playback.

One note: It does NOT prevent track changing due to normal playback (i.e. when the current track ends). For this, you’ll want to combine with Repeat: One (this gives more flexibility to the setting too).

This patch doesn’t change how tracklock works, it really just seperates the setting (e.g. the above also happens with the current skip length).

This patch unifies much of the now duplicated code for e200v2 and fuze in several new files (dbop-e200v2-fuze.c, lcd-e200v2-fuze.c, button-e200v2-fuze.c).

As far as my experience goes, this two targets have too much in common to have the code split and duplicated.

Needs testing on the e200v2. Also, I’m wondering if this good at all, since the other ams sansas are still split, and most functions cannot be static anymore.

This patch adds some functions to the REC button of the e200 for easier handling of playlists for playlist oriented people.

In the database and file browser, hit REC to insert what you have selected. You can insert single files as well as folders (recursively). This basically behaves like the insert option in the context menu.
In the WPS, hit the REC button to show the current playlist.

I made this patch since inserting and viewing the playlist always requres at least 1 long press on Select and 2 further clicks, and since the REC button is free, why not use it?

NOTE: This could very well be extended to other targets. If you have some ideas which buttons could be mapped, just tell me! This is limited to the e200 only because don’t have other (supported) targets.

My windows 7 beta image is reported to be of a size of -937,678,848 (Rockbox file properties plugin). The correct filesize is 3,357,288,448

Besides that it’s entirely wrong, being negative is weird on top.

Looking through the code shows that the dircache_entry’s size member is of type long, making negative values possible.

Changing it to unsigned long/size_t results in the correct filesize. Compilation was warning-less.

Can an expert comment on this and if the change is good and won’t break anything.

This patch implements cancelling changes in the quickscreen. If the changes are discarded, a splash will appear for some 250ms (HZ/4).

The side effect: For most targets, the button that exited the quickscreen will now cancel. So, in order to save the settings, the usual ok/select button must be used (except for buttons which are used to change options, like right, on some targets)

The changes to the keymap are to make sure, that the buttons which changes the options don’t also act as ok button. E.g. if the usual select is mapped to right (in addition to select), the quickscreen must make sure that right is remapped to not act as ok button.

Here’s some code police, suggested by Zagor, and going slightly farer.

-rename USE_BACKLIGHT_SW_FADING and USE_BACKLIGHT_CUSTOM_FADING_BOOL to HAVE_* to be more consistent with the other config #defines
-add a BACKLIGHT_ON_TYPE config, so that the #if <target> in backlight.c are not so target-specific
-reword/rewrite comments and add some to clear things up

Note1: should be tested on affected targets for safety (grep didn’t show a missing rename), mainly on the BACKLIGHT_ON_HARDWARE targets (I tested on my BACKLIGHT_ON_SETTING e200)
Note2: I’d also like to have backlight-thread-fading.[c/h] renamed to backlight-sw-fading.[c/h], I didn’t do that since svn diff doesn’t do well with svn mv’d files, those should be renamed at committing or in a seperate commit

PS: I’d really like to fuse the backlight drivers for e200,c200,fuze,e200v2,sa9200 (probably c200v2), since I noticed that they’re all entirely the same, except for buttonlight (that should be kept seperate). Opinions on that?

So, after the trouble with  FS#9661 , this patch fixes the confusion by adding a 4th item. The 4th item acts the same as the existing three.

It also tries to handle remotes, as it shows only three items (not the top one) on those, since the space and buttons are limited.

So, it keeps ACTION_QS_DOWNINV for remotes and recorder. Basically those screens that have this can only show 3 items.

It also rewrites the viewport preparation in the quickscreen a little bit (not too much), was a bit messy to get the 4th item properly working without.

TODO:
- Veryify that it works on remotes; I tried to test, it “looks” correct going by the sims, but those aren’t very good to test buttons and such I noticed
- Handle Recorder so that he doesn’t show the 4th item

Please test and leave some comments!

This task aims to add the c200v2 as a build target. It seems suitable as we have a c200v2 owner and tester (and I think he also codes).

The first patch makes mkamsboot create patched firmwares. Well, at least if you have a valid bootloader file, which isn’t possible yet. Please review that one as it’s critical (linuxstb?).

This patch enables backlight sw fading for both mentioned targets. Looks nice on the Fuze.

It’s also supposed to fix buttonlight. It generally works, but it needs more work.
It’s quite unreliable!
It generally works if you press a button shortly after the very first buttonlight flash while booting, and it starts working if you do filesystem access. There may be other conditions.
It generally doesn’t work if you boot normally, until something mentioned above happens.

I just want to add: buttonlight ain’t trivial from what I’ve seen. Simply setting the pin is broken as soon as you add a MicroSD, since the MicroSD is blocking the GPIOD port. You basically cannot change xpd_func_sel to GPIO as long as you’re using the MicroSD. And the buttonlight doesn’t keep on just by setting the pin if you change to xpd.

What I did is what I found in the disassembly. It sets the Rod control register. It seems to work a bit, even though I have no idea what it actually is.

Enough talk, here’s the patch