FS#2939 - Encoder codec interface + encoder codecs
|
DetailsThis patch includes the stuff to support encoder
codecs. I have ported the current rockbox encoders (wav_2_wav / wav_2_mp3 / wav_2_wv) to this encoder codec interface. Included in the attached zip file: - compiled encoder codecs - patch file with changes to the main program - source code for 3 encoder codecs Current main flow: - wav2wav, wav2mp3 and wav2wv encoders can be selected via the quality setting (MP3=0-5, WV=6, WAV=7) - the encoder (in encoder thread) starts on recording screen opening - two circular buffers are used: pcm-buffer (2MB), enc- buffer (remaining space) - upon DMA writing the codec starts encoding - pcm_callback function collects the processed chunks and writes the data to file on record stop and on enc_buffer full Known issues (independent from this patch) - no beep on recording - no peakmeter if file writing is active - input: PCM 44100 stereo only - quality settings are not saved to disk for SW_CODEC recorders - new codecs have not been added to the build system |
encoder_codec.zip
This task depends upon
Closed by Michael Sevakis (MikeS)
Monday, 28 August 2006, 22:56 GMT
Reason for closing: Accepted
Additional comments about closing: Development shall continue...
Monday, 28 August 2006, 22:56 GMT
Reason for closing: Accepted
Additional comments about closing: Development shall continue...
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.
- 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
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)
- 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)
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.
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?
The patch works pretty much as expected, it's great!
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.
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
- 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)
-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.
-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!
thx, Sander
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.
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.
The latest one is the one.
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.
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).
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.
Compress it? http://www.rockbox.org/tracker/?getfile=12249 is not zipped or anything.