Rockbox

This update includes some enhancements:
- cpu unboost during recording
- integrated in the rockbox build system
- save quality settings
- show quality settings with enc-type and bitrate
- mono recordings

tested ok:
- file split on recording
- cpu unboost
- prerecording
- talk menues (except recording_screen->recording_menu)
- usb connect in recording screens
- mono recordings (wav,wv,mp3)
- settings save: quality

Patch added (See above)

Hey Toni, great work.

I noticed you don't add the WAV RIFF chunks using
WavpackAddWrapper() before packing. The chunks are
required for the files to be valid (even though they don't
really need to be there). For example, the Wavpack
author's utils cannot unpack these files back to .wav
without them. The original wav2wv.c already does this.

You also should set the config sample count to -1
(=unknown), cache the first block written to disk (or load
it at the end of recording), then at the end of recording
call WavpackUpdateNumSamples() and WavpackGetWrapperLocation
() (add the protos from wputils.c to wavpack.h) to update
this block with the final sample count / length, then
rewrite it to disk. Otherwise Wavpack has to scan the
entire file to find the length each time it's opened.

Contact me at gl [at] ntlworld [dot] com, or on the dev
list.

Update, I see you already take care of the Wavpack header.
But the RIFF info still needs to be added and updated at
the end.

New update added:
- use standard cpu_boost() (works now after changing dma-ing to 16byte mode)
- insert RIFF header into wavpack file data
- some cleanups

Known issues:
- Wavpack file size display not always correct (due to worst case compression estimation)
- in rare cases the player hangs AFTER long recordings after quitting the recording_screen

This update includes a simple workaround to avoid hangs after quitting the
recording_screen if voice is enabled: Now leaves a minimum of 8000Bytes
at audiobufend unused. It seems, that in current builds some parts of the
software rely on the audiobufend data. Wether this is a bug or intended I
don't know.

Known issues:
- Wavpack file size display not always correct (due to worst case compression estimation)

New update:
- general: max encoder chunk size increased to 20000 Bytes
current usage: wav=8192Byte, mp3=4608Byte, wv=20000Byte
- wavpack: block index (time stamp) corrected
- wavpack: improved file size estimation (for display)
- wavpack: blocksize changed from 8192 to 20000 (slightly better compression)

any chance of getting Vorbis in on this? O:-)

Update coming today if I feel the radio stuff is straghtened out...Just have to test compile some other builds.

Test compiled for Archos, Ondio FM, H300, iPod Video, x5 sim and of course x5 since that seemed representative of the variation it will deal with.

Adds the following features in addition to the original for X5/h100/h120/h300:
-Integrates with recent recording upgrades
-FM Radio selection for the recording source
-FM Radio recording and settings from fm menu
-Full FM Radio control external to radio.c
-A 224 kBit/s option for MP3
-All recording settings are saved

Cleanup:
-Adds central database of audio formats instead of scattered parallel arrays
-Centralized audio source switching with powering of selected source

Fixes some things:
-Voice hangs when moving rapidly to recording screen when voice menus are enabled
-Wrong path_buffer was used after triggering was added. Deleted local instance in recording_screen and all was good.
-Miscellaneous tweaks for smoother operation
-Other consoldation of lower level code to clean up screens.

Known Issues (though this patch works reliably):
-Time progress in WPS stops about 2s before end. WPS knows the correct length though and files play to correct length. Files at beginning and middle of splits don't seem affected. Progression is correct on PC based players. Problem independent of format or bitrate.
-Duplicate file names in /recordings: This showed up a couple times but I cannot say it is related to the patch. The dupe dissapeared after reboot so it could be due to dircache.
-The voice for kBit/s doesn't work right...starts saying digits instead of units.
-Please test SPDIF! It should work fine but I cannot test it myself. The code should follow the same powerup ordering but is now done in rec_set_souce.

To do a bit later:
-Needs better display in recording screens of format: Just shows "MP3, WAV or WV".
-Want to add encoder selection with advanced option screens instead of using the quality setting for setting format and bitrate together. This is obviously a relic of mas codec players. Codec file format will need to be updated slightly and I have a plan.

Hey Mike, great work on the progress of this patch, it's coming up really well.

A few things I noticed while playing with it:
1) Codec loading could be handled a bit better, when I first tried the patch, I only upadted the rockbox.iriver file, and when I tried to go to the recording screen it froze the player.
2) There's a bit of a glitch in the recording screen. When you start the recording, a progressbar thing appears (flashing on and off - it's quite annoying), and recording won't start until this progressbar has reached 100%. At this point, the progressbar resets to 0% and continues cycling through over and over while recording.
3) If you have selected WV encoding, just sitting in the recording screen without actually recording anything causes the peak meters to lag, probably because of a strained CPU. Does the encoder have to be running at that point? And does the WV codec boost the CPU?
4) In the recording screen, it mentions that it's recording at 24kHz, but the files created are 44.1kHz. Is it actually recording at 24kHz and then upscaling, or is this just a case of displaying the wrong string?
5) On a related note, is the sampling frequency adjustable?

Ooops, after properly updating the build and resetting my settings, the strange progresspar thing (point 2) completely disappears.
The patch works pretty much as expected, it's great!

The CONFIG_BLOCK_VERSION was changed to 50 at some point after I had changed it to 50 locally as the patch doesn't contain a diff for it. It must be incremented from whatever it is at the time before committing.

I'll get to work on the finer points when it's clear that it's ok to add to CVS. I'd like to have at least this much in the bag because this won't break any long established features and seems reliable.

1) 2) As you said, a full update and settings reset take care of that.
3) The WV encoder is a decent CPU load and the codecs boost the CPU. The run as soon as they're started for now.
4) Doesn't do sampling rates but that would be added in a timely manner. I didn't get into that and instead focused on getting the basic interface working well.
5) See 4

Great, works like a charme on the H120 after solving some minor issues:
- minor SPDIF conflicts during compiling
- disable automatic shutdown, because audio_status() does not take care for recording
- removing the boost_counter logics (to always boost in file writing)

-Now that you mention it, I forgot to try compiling with HAVE_SPDIF! oops
-I suppose auto shutdown should be avoided it you're recording.
-Which boost_counter logics? It's boosted when codecs are loaded and probably should remain that way. I'm told by the one that added SPDIF that it should always be boosted for SPDIF. The CPU boost for SPDIF has to move to the rec_set_souce function now that I think about it or else if you switch to that later, it won't have the extra count for it.

-SPDIF compile problems cured - power handling fixed
-audio_status() now takes care of recording...it just calls pcm_rec_status() and ores those flags with its own (for now). It is now the only caller of pcm_rec_status btw. The behavior is then consistent with the mpeg.c version.
-FM Radio checks for poweroff are now just like playback (playing: power stays on, paused: can shutdown)
-Radio status is shown wherever you are as it should be including on the normal recording screen.
-Boosting will be refined and after a commit. I don't want to be holding this huge bulk of code and continue to refine it. I can do that later. There will be little delay in doing so.
-I want to commit it soon...so please everyone interested (you know who you are! :) please test, test, test!

Do I need the encoder patch and rhe recording patch? If so the encoder patch does not apply to current cvs.

thx, Sander

Hi MikeS:
The point with boost_counter is the following:
must_boost is only checked at the start of the file writing process. But currently each codec can unboost the cpu in the codec thread at any time. I don't know, why the boost_counter check has been added to pcm_record.c. To my opinion his handling is unnecessary or even unwanted. In a correct implementation it is enough to boost the cpu at the beginning of the file writing and unboost at the end of file writing.

Unfortunately I have no SPDIF input device, so no chance to test this.

Toni:

Your Tuesday, 28 February 2006, encoder04.diff. had the call in pcm_record.c and well as in the recording screen. The boost counting for SPDIF is fine and is only a special consideration for SPFDIF. Other sources will not do this. I asked about removing that and got an emphatic "NO, NO NOT". Look in the codec entry points and you'll see that the unboost not a problem (it never was). you'll just have the boost count going 0->1->2->1->0 instead of 0->1->0.

Sander:

The latest one is the one.

Toni:
See: http://www.rockbox.org/irc/rockbox-20060823.txt starting at 1) 18.13.35 and 2) 22.27.35 between myself (jhMikeS) and preglow.

Hi MikeS,
I understand somewhat, that SPDIF recording needs the cpu beeing boosted.
But if I didn't miss something, then the current patch does not assure, that file writing is done at full speed (except for SPDIF). This is because of the must_boost logics at the beginning of file writing. At this point the cpu can be in both states (boosted or unboosted).

I think you might have...the CPU being boosted by the codec should make it bypass that since it's already boosted. If the codec is near empty it unboosts itself as you know. Each of these things individually tracks it's own effect so that makes it safe over assuming boost/not boost from elsewhere. pcm_rec_callback will only unboost if it itself added a count. The counter integrity will be maintained.

Now that I look, the unboost in the error case should be removed or should reset must_boost once it removes its own boost and cpu_boost(false) will be done twice after only one increment (but that aint my bug :). That will be taken care of but isn't worth creating a whole new patch over. I'll just commit the minor change right after committing the patch.

Please don't compress the patch, it just makes it harder to review it.

Linus:

Compress it? http://www.rockbox.org/tracker/?getfile=12249 is not zipped or anything.

My bad. I looked at the wrong files.

See http://forums.rockbox.org/index.php?topic=6152.0 for some ready made builds for Archos and a couple iPods. The reasons for this are explained there.