Rockbox

Tasklist

FS#12176 - New Chiptune codec pack based on Game_Music_Emu library ;)

Attached to Project: Rockbox
Opened by Mauricio Garrido (gama) - Friday, 01 July 2011, 14:40 GMT
Task Type Patches
Category Codecs
Status Unconfirmed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No

Details

Hi, this is a codec pack containing all codecs based on blargg's
Game_Music_Emu library.

I have ported all code to C to work in ROCKbox, and i have replaced the
MAME versions of some emulators with others compatible with the GPL license.
It was a lot of work but i did it because i really love chiptune music and having the
possibility to listen to a lot of classic video game system's music in a small player
is really great ;).

Original Game_Music_Emu library here: http://slack.net/~ant/libs/audio.html

Tested on: Sansa Fuze v2
Created from revision: 30084

The following formats are contained in the pack:

- AY (ZX Spectrum, Amstrad CPC)
- GBS (Nintendo Game Boy)
- HES (NEC TurboGrafx-16, PC Engine)
- KSS (MSX Home Computer, other z80 systems)
- NSF, NSFE (Nintendo NES, Famicom)
- SGC (Sega Master System, Game Gear, Coleco Vision)
- VGM, VGZ (Sega Master System, Mark III, Sega Genesis, Mega Driver, BBC Micro)

Additional features:

- 44100 Khz, stereo playback.
- Support all sound chips in NSF/NSFE formats.
- Support MSX-AUDIO and MSX-MUSIC in KSS format .
- Support ADPCM samples in HES format.
- Support for extended m3u playlists created specially for
some chiptune formats.

Known issuses:

- Some (or all) codecs might not work in some targets, specially
if they have an small iram size, or if the codec is too cpu intensive,
like VGM and KSS.
- Subsong change is not working properly, specially when it happens
automatically.
- Most vgz tracks will be truncated due to the small amount of free
memory available to uncompress them.
- Some SCC+ soundtracks might not work correctly.


** There might be several bugs and issues to be found yet, so please feel free to
report them here or to my mail address.


Special thanks to:

- Shay Green (blargg) the original author of the great Game_Music_Emu
library.
- Chris Moeller (kode54) who made some nice improvements to the library.
- Mitsutaka Okazaki, author of the YM2413 emulator.
- The OpenMSX team for the YM8950 emulator.
- Stéphane Dallongeville for the YM2612 emulator.
- Joshua Chang, Haiku Konaru and everyone else who has helped me to
test the codecs.

This task depends upon

Comment by Thomas Martitz (kugel.) - Saturday, 02 July 2011, 09:33 GMT
Interesting!

Do you replace the current nsf codec with one based on this library?

What is this bios in sgc.c? Where do you get it from?
Comment by Andree Buschmann (Buschel) - Saturday, 02 July 2011, 09:40 GMT
I also think this an interesting patch.

Some questions additionally to kugel's:
- The codecs are all played via a single library. Is IRAM shared for the whole library?
- Can you provide a sample for each format? We need such for future performance and regression tests.
- Can you add a patch to enhance the manual especially in the codec and metadata section?
- Does this patch incorporate or replace your other patches  FS#12021 ,  FS#11999 , FS#11978,  FS#11945  and  FS#11906 ?
Comment by JoshuaChang (JoshuaChang) - Sunday, 03 July 2011, 09:42 GMT
the new 2612 emulator is great, speedup vgm codec by 20~60%
also, a little problem i've forgotten to mention longlong ago, the nes_fds_apu's regs declaration was conflict with some targets‘s define(gigabeat fx?), i've modified the files, see attachment
Comment by JoshuaChang (JoshuaChang) - Sunday, 03 July 2011, 09:43 GMT
the files :)
Comment by Thomas Martitz (kugel.) - Sunday, 03 July 2011, 10:24 GMT
What did conflict exactly? Perhaps we should rather change our target defines rather than imported code.

Another question: Some codec(s) in the patch use malloc. Is that malloc better or can it be replaced with with the tlsf_malloc() we have in the codeclib?
Comment by JoshuaChang (JoshuaChang) - Sunday, 03 July 2011, 12:40 GMT
well~ i didn't remember, maybe build the gigabeat fx/s targets with gme codec could reproduce this bug
Comment by Mauricio Garrido (gama) - Monday, 04 July 2011, 03:12 GMT
Sorry for the delay, some answers ;),

> Do you replace the current nsf codec with one based on this library?

Yes, i believe the nsf codec based on Game_Music_Emu library has superior quality in both emulation and sound. Someone mentioned the
current nsf codec had better playback controls though.

> The codecs are all played via a single library. Is IRAM shared for the whole library?

All codecs compile independently using only the files they need. Some people suggested it would be a better idea to put all files in the same folder.

> Can you provide a sample for each format? We need such for future performance and regression tests?

Well i guess i could get some help here, maybe search for tunes that are cpu intensive.

> Can you add a patch to enhance the manual especially in the codec and metadata section?

I could get some help here too ;)

> Does this patch incorporate or replace your other patches  FS#12021 ,  FS#11999 , FS#11978,  FS#11945  and  FS#11906 ?

Yes, it incorporates and also replaces all patches, except for FS#11978, the Atari sc68 patch.


Comment by Andree Buschmann (Buschel) - Monday, 04 July 2011, 06:11 GMT
Ok, I have closed the duplicate tasks and linked those a related tasks. Additionally I linked  FS#12113  which did an IRAM optimization to the current nsf implementation -- we should be abel to keep tor reinvent this as the speed up was massive (an example file is attached in this task as well).

> Well i guess i could get some help here, maybe search for tunes that are cpu intensive.

Searching for files that are CPU intensive is a good idea, but not mandatory for a first step. Do have a test file for each codec available -- independent of CPU needs?

> I could get some help here too ;)

I have attached an example for the manual enhancement using "GBS Nintendo Game Boy". You will need to change 3 parts of the manual section for each codec:
1) Add it to the codec tables. All of your codecs from this patch will all be "Others" (not lossy, not lossless). Add any comment, if the codec has some special limitations (e.g. seek subtracks instead of seconds)
2) Does the codec use some standardized metadata tag (ID3, ASF, ...) or does it use its own format (like GBS does from what I saw from your patch) or does it not support metadat at all?
3) If the codec uses its own metadata format: what kind of data is supported? I filled the table for GBS based on your patch's metadata source code.


Comment by Mauricio Garrido (gama) - Tuesday, 05 July 2011, 00:24 GMT
Here is a manual patch based on Buschel's sample. Please feel free to make the necessary corrections and modifications.
Maybe a couple of notes should be added, for example, the possibility to use an extended m3u to get some extra metadata info
and subtrack remap. But not sure where to put those.

I also included a test file from each format, with their respective credits in the README file.
Comment by Andree Buschmann (Buschel) - Tuesday, 05 July 2011, 06:19 GMT
Thanks! I have made some minor changes in the manual. Now the tables will look fine again

Few more questions:
- vgz is just zipped vgm and handled by the same metadata parser and decoder? If so, I would just make a single codec entry to the manual, add both suffixes and add a note regarding the zipped version.
- Is the same metadata set supported for vgm and vgz? If so, this should be added to the manual as well.
Comment by Mauricio Garrido (gama) - Tuesday, 05 July 2011, 12:26 GMT
> vgz is just zipped vgm and handled by the same metadata parser and decoder?
Yeah vgz are just zipped vgm (using gzip algorithm), but the only way to get the metadata is uncompressing them.

> Is the same metadata set supported for vgm and vgz?
For now the metadata parser only handles vgm files. I guess it would be too slow to uncompress each vgz to get the
metadata.
Comment by Andree Buschmann (Buschel) - Tuesday, 05 July 2011, 19:37 GMT
I had to define assert() to nothing (libgme/blargg_source.h, lines14-20) to get the patch compiled for simulation. Afterwards I compiled the patch successfully for iPod nano2g (sim + target) and Gigageat F/X (target). Playback only tested shortly, worked for all of the provided files under win32 simulation.

Edit: I am not sure if it is a good idea to have all of those codecs in a single directory. Is it possible to move the different codecs to seperate paths? Common used parts could be placed in libgme. This is similar to what we do with libm4a or libasf.
Comment by Andree Buschmann (Buschel) - Tuesday, 05 July 2011, 21:12 GMT
IRAM was completely disabled with the provided patches. I needed to correct all ICODE_ATTR attributes as they were not used correctly. After that I could enable IBSS_ATTR, ICODE_ATTR and ICONST_ATTR for an iPod nano2G build -- which has large IRAM. Please see apps/codecs/libgme/blargg_common.h (line 23-35) for configuration tests.

IRAM is still not used with the attached updated patch, but the patch allows to enable IRAM now with some minor code changes. Please see apps/codecs/libgme/blargg_common.h (line 23-35) for details.
Comment by Andree Buschmann (Buschel) - Tuesday, 05 July 2011, 21:27 GMT
I get lots of "warning: C99 inline functions are not supported; using GNU89" warnings when building the simulation now. I am not sure whether this is an effect of my latest changes. I need to undo my changes and recompile again. I will update the information tomorrow.
Comment by Mauricio Garrido (gama) - Tuesday, 05 July 2011, 22:35 GMT
> IRAM was completely disabled with the provided patches.

Yeah sorry about that, i tryied to use it based on the old nsf and spc codecs (before r29886) but didn't have any
way to benchmark or even test it.
Comment by Mauricio Garrido (gama) - Tuesday, 05 July 2011, 22:47 GMT
> Is it possible to move the different codecs to seperate paths? Common used parts could be placed in libgme. This is similar to what we do with libm4a or libasf.

The problem is that only a two c files are common between all codecs: blip_buffer and multi_buffer. The other common files are headers: blargg_xxxx.h, m3u_playlist.h, etc. Would there be any advantage doing it?.
Comment by JoshuaChang (JoshuaChang) - Wednesday, 06 July 2011, 00:12 GMT
>I had to define assert() to nothing (libgme/blargg_source.h, lines14-20) to get the patch compiled for simulation.

try to add NDEBUG 1 in blargg_common.h
Comment by Thomas Martitz (kugel.) - Wednesday, 06 July 2011, 05:50 GMT
fwiw, as this is imported code I would change it as little as possible, including the directory structure.
Comment by Andree Buschmann (Buschel) - Wednesday, 06 July 2011, 06:40 GMT
Ok, the warnings when compiling the simulator were not introduced by my changes. I will just leave them for now.

> Yeah sorry about that,
No need to say sorry. Let's just keep working on this stuff ;)

Regarding the path-structure: It is just not my personal taste to have it all in one directory. But if a) this is the way GME is structured and b) there is only few common code just leave it like it is. Will make it easier to update GME.
Comment by Thomas Martitz (kugel.) - Wednesday, 06 July 2011, 07:05 GMT
- "Will make it easier to update GME."
Also to push fixes upstream (although I'm aware we historically don't do this).
Comment by Mauricio Garrido (gama) - Wednesday, 06 July 2011, 13:01 GMT
> No need to say sorry. Let's just keep working on this stuff ;)

I am trying to get one of the players that have large IRAM for testing, but i can't find one here in my country and Amazon doesn't
ship electronics to Mexico. I know about the ipod nano, gogear, sansa c250. Could someone tell me what other players benefit from using IRAM?.
Comment by JoshuaChang (JoshuaChang) - Thursday, 07 July 2011, 00:03 GMT
well, the cowon d2 does benifit from using iram, though it's iram is not very large, 96k
Comment by JoshuaChang (JoshuaChang) - Thursday, 07 July 2011, 00:10 GMT
also, AFAIK, your sansafuzev2 does have the largest iram among the rockbox native targets
Comment by Andree Buschmann (Buschel) - Thursday, 07 July 2011, 04:47 GMT
Some devices (like Sansa fuze) hold the whole codec in IRAM. In this case you will not need to configure the codec at all. When it comes to IRAM optimization it is better to start on devices with the standard IRAM size of 96 KB. For other devices with larger IRAM (e.g. iPod Video, M5/X5 nano 2G, ...) this can be enhanced later.

gama, is there any CPU intensive codec contained in this patch? I do not insist on fully optimizing this stuff before bringing it to svn. We can do this later -- especially as I do not expect too high CPU requirements for most of these codecs.
Comment by JoshuaChang (JoshuaChang) - Thursday, 07 July 2011, 05:02 GMT
kss eat around 17mhz cpu, hes eat around 21mhz cpu, test based on cowon d2, for multitrack couldn't be tested, i use buffer info to watch the cpu speed, maybe not accurate.
Comment by JoshuaChang (JoshuaChang) - Thursday, 07 July 2011, 05:10 GMT
oh, forgot vgm, the most powerful cpu killer: eat 110+mhz cpu on cowon d2+, eat 145+mhz cpu on gigabeat f
Comment by Mauricio Garrido (gama) - Thursday, 07 July 2011, 13:09 GMT
> Some devices (like Sansa fuze) hold the whole codec in IRAM.

Didn't know about that ;).

The most cpu intensive CODEC is VGM, we can try including some profiling code first to find the most cpu intensive parts, i think we can use the spc_profiler as basis. I'm pretty sure that since the original library was wrote in c++, there must be a lot of room for optimization. For instance, someone may write an ASM resampler ;).
Comment by Thomas Martitz (kugel.) - Thursday, 07 July 2011, 14:01 GMT
I think we shouldn't think about optimizations before this is in SVN at all.

Would it be possible to provide a patch against upstream "libgme"? To see what's effectively your code and what's imported.
Comment by MichaelGiacomelli (saratoga) - Thursday, 07 July 2011, 15:56 GMT
>The most cpu intensive CODEC is VGM, we can try including some profiling code first to find the most cpu intensive parts, i think we can use the spc_profiler as basis.

Usually this helps a lot more then IRAM. Its mostly older CPUs that benefit a lot from IRAM. On newer devices the difference is often fairly small due to better caches.

>For instance, someone may write an ASM resampler ;).

Are you actually resampling? Our playback engine should be able to take most common sampling rates, and then resample them if needed (using our fast ASM resampler).
Comment by Mauricio Garrido (gama) - Thursday, 07 July 2011, 22:12 GMT
> Would it be possible to provide a patch against upstream "libgme"? To see what's effectively your code and what's imported.

Mmm, i don't think that would be possible. The original Game_Music_Emu library was written in pure C++ code, the files that are not originally distributed with Game_Music_Emu are: emu2413, emu8950, emuadpcm, 281btone.h, 2413tone.h, vrc7tone.h, emutypes.h, msxtypes.h, emutables.h, opltables.h and ymtables.h.

By the way the files ymdeltat.c and ymdeltat.h are not used by any codec, they need to be removed from the patch ;P.

> Are you actually resampling? Our playback engine should be able to take most common sampling rates, and then resample them if needed (using our fast ASM resampler).

Yeah blargg resamples the output of the ym2612 emu to the desired sample rate (this case 44100), i believe that is done to achieve a better accuracy. But i am not sure at which rate the ym2612 operates (52khz?), neither if the resampler (downsampler actually) can be removed.
Comment by Thomas Martitz (kugel.) - Thursday, 07 July 2011, 22:21 GMT
So you converted everything to C and even added new emulators/codecs?
Comment by Postolati Maxim (tails_) - Thursday, 07 July 2011, 22:56 GMT
>But i am not sure at which rate the ym2612 operates (52khz?)
Yeah, it's 52847.22Hz to be exact, also i don't like blargg's resampler - it cuts that crystallic FM sound :(
Comment by Mauricio Garrido (gama) - Thursday, 07 July 2011, 23:06 GMT
> So you converted everything to C and even added new emulators/codecs?

Yep, well i didn't really add anything, i just replaced some MAME emulators with GPL'd ones.

> Yeah, it's 52847.22Hz to be exact, also i don't like blargg's resampler - it cuts that crystallic FM sound :(

I'm using the fast downsampler added in gme6pre, maybe using a FIR resampler would result in better quality. But also i'm using
an old ym2612 emulator since the new one (the on from MAME) is not under GPL.
Comment by Postolati Maxim (tails_) - Thursday, 07 July 2011, 23:39 GMT
Ah yeah, can be MAME emulator used in this patch by replacing source file?
P.S. I noticed that blargg relased SPC emulator library, are you going to include this one too into bundle?
Comment by Mauricio Garrido (gama) - Friday, 08 July 2011, 00:03 GMT
Replaced tabs with spaces and doing some code cleanup, in case someone was thinking of doing it ;).
Will upload patch as soon as i finish.
Comment by Mauricio Garrido (gama) - Friday, 08 July 2011, 00:08 GMT
> Ah yeah, can be MAME emulator used in this patch by replacing source file?

Yes, but only for private builds. And there is already an optimized spc codec for rockbox based on gme, so i thought it wasn't necessary to port it again.
Comment by Postolati Maxim (tails_) - Friday, 08 July 2011, 01:32 GMT
Well yeah but it don't supports reverb\echo effects
Comment by Mauricio Garrido (gama) - Friday, 08 July 2011, 02:40 GMT
Ouch, i haven't included the effects_buffer in the port yet. But now that it is in a better shape i think i can give it a try. But the equalizer stuff is out of the question because i only ported the fast version of blip_buffer.

But i still think that re-porting the SPC codec would be a waste of time ;(.
Comment by JoshuaChang (JoshuaChang) - Friday, 08 July 2011, 09:00 GMT
>Well yeah but it don't supports reverb\echo effects

well i don't remember that sfc console itself support reverb effect, about echo, you can activate it manually in spc_codec.h
Comment by MichaelGiacomelli (saratoga) - Friday, 08 July 2011, 12:30 GMT
> I noticed that blargg relased SPC emulator library, are you going to include this one too into bundle?

The current rockbox decoder is already a heavily optimized version of blargg's decoder combined OpenSPC.

>Well yeah but it don't supports reverb\echo effects

No it does. Although some effects are disabled on slower targets. Take a look at spc_codec.h

Comment by Mauricio Garrido (gama) - Tuesday, 12 July 2011, 00:51 GMT
In order to add the effects support (echo, reverb and stereo panning) i ported the latest version of blargg's blip_buffer, and it seems the audio quality is better, but it is also more cpu demanding ;(. I have attached a beta patch with the new blip_buffer code, could someone confirm that the sound quality is a little better now? Maybe is just my imagination, but i'm pretty sure the genesis tracks sounds better now.

If sound quality is not better i guess we should stick with the old blip_buffer, if it is better we could keep both versions and enable the old one for slower targets. Please let me know your thoughts about this, so i can get working on the effects code.

P.S. This patch doesn't have the effects support yet, just the new blip_buffer code.
Comment by Mauricio Garrido (gama) - Tuesday, 12 July 2011, 13:15 GMT
On a second thought it seems there is no increase in quality, i think i will stick with the old blip_buffer code ;¬|.
Comment by JoshuaChang (JoshuaChang) - Wednesday, 13 July 2011, 00:00 GMT
have tested new patch, no noticeable improvement in quality, but the vgm has a 20% speed down :(
Comment by Mauricio Garrido (gama) - Wednesday, 13 July 2011, 14:00 GMT
Thanks for testing Joshua, i found something interesting; the decrease in speed was not caused by the new blip_buffer :0,
in the previous patch the oversampling was always disabled, so working on the new blip_buffer i fixed that bug, and enabling
oversampling caused a ~20 % speed decrease.

The new blip_buffer does have an impact on the speed test though, but it is very low, around 1-2 %. So i stil have to decide what version
of the effects buffer to use; the old effects buffer adds echo, stereo pan and reverb, and the new one adds stereo pan, echo and surround, and lets you set each value individually.

What do you think?

Comment by JoshuaChang (JoshuaChang) - Wednesday, 13 July 2011, 14:21 GMT
1~2% speed lost was not noticeable
about the blip_buffer, i think the gme simulation should do like a real machine, instead of add more effects.
ps: we can surely add echo/stereo pan/reverb in rockbox's dsp stage.
Comment by Mauricio Garrido (gama) - Wednesday, 13 July 2011, 19:25 GMT
I think you are right, i should better work on porting the remaining gym format ;).
Comment by Mauricio Garrido (gama) - Friday, 15 July 2011, 01:26 GMT
Ok, i couldn't resist, i ported the new effects buffer to hear how it sounds ;P. And i have to say i really like the
surround effect, especially with nes tunes ;).

I really think we should keep these changes, effects are available by default for those who want to test them or
they can be disabled by removing the Sound_set_stereo_depth from each codec or they can be completely removed with the GME_DISABLE_STEREO_DEPTH option in blargg_config.h.

The effects can be adjusted with the Sound_set_stereo_depth (from zero to 1.0), or they can be individually changed with
Sound_set_effects (see vgm.c line 109 and libgme/multi_buffer.h line 18).

Also the speed test only reported an speed decrease of about 10 - 15%, so i don't think it affects performance that much.

Comment by JoshuaChang (JoshuaChang) - Friday, 15 July 2011, 02:21 GMT
the surround effect is applied as a post-processing? or it make different effect in different channel?
if it just act like a post-processing unit, then i still recommend that we can move these thing to the dsp stage
Comment by Mauricio Garrido (gama) - Friday, 15 July 2011, 23:07 GMT
Blargg was kind enough to answer some of the above comments ;), he can't post in here in the moment so
he ask me to do it, here they are:

@tails_, as I remember, the SPC player in Rockbox was ported several
years back, and the author made lots of optimizations so it would run
well even on older devices. It's based on my older OpenSPC-based DSP,
so it doesn't have the accuracy improvements my current DSP has. On my
PowerPC machine, the FM sound chips have always been by far the
biggest CPU users, with SPC significantly lower, so if FM sound chips
work on devices now, a port of the SPC player in gme6pre might be
feasible.

@JoshuaChang, the effects processing is done in GME because it can
operate on the individual sound channels from the sound chips, rather
than the final sound stream. Many systems output mono, so you can't do
as much with a post-effects processor. I agree that such effects
should be entirely optional, and shouldn't hurt performance if
disabled. I partly added them to make headphone listening more
pleasant. The depth control allows a subtle level of effects that
aren't really obvious, but still make it easier to listen to than mono
for a long time.

I agree with blargg, listening to the nes music with the effects enabled is more pleasant especially using headphones,
so i think we should keep them in the patch, they can always be easily disabled in blargg_config.h.

And it might be a good idea to update the spc codec with the new DSP by blargg too.
Comment by JoshuaChang (JoshuaChang) - Monday, 18 July 2011, 12:43 GMT
have tesed new 0.5 patch, yes, the new effect have noticeable stereo enhancement in nsf/gbs formats, well, i can save my cpu usage in those files:) thanks gama~~

i've made a quick speed test on cowon d2 target, only tested vgm, for it's the most cpu intensive codec on my target:
gme 0.1
168.45% / 113.98mhz

gme 0.4beta
137.87% / 139.26mhz

gme 0.5
168.93% / 113.65mhz
Comment by JoshuaChang (JoshuaChang) - Tuesday, 19 July 2011, 02:10 GMT
i should mention again: the nes_fds_apu's regs declaration was conflict with gigabeats‘s define. please rename regs to a different name(i use regsn instead)
Comment by Thomas Martitz (kugel.) - Sunday, 31 July 2011, 10:08 GMT
One question of mine is still unanswered: What is this bios in sgc.c? Where do you get it from? Do we need it?
Comment by Andree Buschmann (Buschel) - Sunday, 31 July 2011, 11:32 GMT
I just worked on this task and had some discussion with kugel about it. The following patch is the result:

1) It is based on v03 as the following patch versions do not compile anymore. I will not make the newer ones compilable but aim towards submitting this working patch. The additional optimizations in the synth/buffer stuff can be provided as seperate patches later. This way the changes become more visible and can be reviewed easier .
2) It changes the "regs" definition to "regs_nes" to not collide with other declarations.
3) It removes all "//" comments and replaces them with "/*" comments in the rockbox specific files (apps/codecs/. apps/metadata/.).
4) It removes support for Coleco-SGV for now to not depend upon local BIOS-files which were not proved with this patch.
5) It removes the gme-/blargg-specific metadata add-on (apps/metadata/gme_m3u) for now. This can be provided, discussed and decided in a seperate patch as well.

Edit 1: re-sync'ed against r30225.
Edit 2: Removed residual parts of blargg-specific m3u-handling from rockbox-specific files. libgme path is still untouched.
Comment by Andree Buschmann (Buschel) - Sunday, 07 August 2011, 20:49 GMT
With r30264 and r30265 I have submitted the patch from above. There are a few things we need to work on:

The RAM usage of NSF was vastly enlarged, the RAM usage of VGM, SGC and KSS is very high as well. As a result I had to disable all of those codecs for low memory targets. For NSF this means we have lost the playback ability for this codec on such targets. Main reason seem to be the arrays in emu2413.c, ym2612_emu.c (relevant for KSS only) and emu8950.c (relevant for KSS only) -- especially tllTable[][][][].

@gama: Are there possibilites to configure these codecs to use less RAM? Most important for me is to support NSF on low memory targets again.
Comment by Andree Buschmann (Buschel) - Monday, 08 August 2011, 21:39 GMT
Attached patch migrates the tempo-setting from floating point to fixed point.
Comment by Andree Buschmann (Buschel) - Wednesday, 10 August 2011, 06:34 GMT
Attached patch migrates the gain-setting from floating point to fixed point. Additional it enhances the precision of "tempo" fixed point (submitted with r30274) to 24 bit.
Comment by Mauricio Garrido (gama) - Wednesday, 10 August 2011, 18:45 GMT
Thanks, i can't believe it made it to the SVN :).

> The RAM usage of NSF was vastly enlarged, the RAM usage of VGM, SGC and KSS is very high as well. As a result I had to disable all of those codecs for low memory targets. For NSF this means we have lost the playback ability for this codec on such targets. Main reason seem to be the arrays in emu2413.c, ym2612_emu.c (relevant for KSS only) and emu8950.c (relevant for KSS only) -- especially tllTable

Yeah those codecs use very large data arrays, will see what can be done about that ;). How can i now exactly how much RAM a codec uses?.
Comment by Mauricio Garrido (gama) - Wednesday, 10 August 2011, 19:32 GMT
I took a quick look at the ym2612 codec, and i guess we can save some ram removing some arrays, i made the mistake to declare a second array to store the precalculated coefficients, since there is already an array declared for the whole table (like TL_TAB), we can use that one.

Also in emu2413 we might be able to compute the tl values dinamically as done with the dphase table, but that may have a cpu impact if UPDATE_TLL is called too many times, and we would have to use fixed point math when calculating the tl value.

But then again, is there anyway to get an exact value of how much RAM is being used, and how much RAM can be used in low memory targets?.

Comment by MichaelGiacomelli (saratoga) - Wednesday, 10 August 2011, 19:37 GMT
The smallest memory targets are the Sansa ClipV1 and c200v2 (both using the same CPU). They have 294912 bytes available. You can see how much memory a codec uses by checking its map file in the build directory after compiling it. You can test if you're using too much memory by trying to build one of those targets and checking if the codecs link.

Comment by Andree Buschmann (Buschel) - Wednesday, 10 August 2011, 20:13 GMT
Attached patch migrates all volume-settings from floating point to fixed point. Next step from my side will be to migrate sampling rate and clock rate calculations to fixed point.

@gama: Good to see you're back :) We will need to put some work into this stuff. The nsf codec is not only bigger, but also much slower than the previous implementation. This will be most likely due to the floating point calculations. The emu-code will need to be migrated to fixed point as well for the emulators.

Btw, you can also see details about the codec RAM usage when calling ../tools/codecscan.pl from your build directory.
Comment by Mauricio Garrido (gama) - Wednesday, 10 August 2011, 23:22 GMT
> Good to see you're back :)

No problem, i was just taking a break ;).

> We will need to put some work into this stuff. The nsf codec is not only bigger, but also much slower than the previous implementation. This will be most likely due to the floating point calculations.

I'm pretty sure no floating point calculations are done during playback (except for vrc7?), i believe it's slower than the previous codec because the emulation is more accurate and thus has better sound quality. I asked blargg about this, i hope he can share his thoughts about it. Will let you know.

Also maybe the porting process produced some unoptimized code, i must run a speed test comparing the c++ and the c versions to verify that.
Comment by Andree Buschmann (Buschel) - Thursday, 11 August 2011, 06:27 GMT
I submitted the volume-related patch with r30278. With this patch I have also changed the residual floating point code in blip_buffer -- now AY and GBS do not use floating point anymore.

With another submit I reduced the gain of the VGM synthesis. This avoids clipping which was present since the first commit to svn. The clipping could be heard when playing "test.vgm" from gama's test files.
Comment by PurlingNayuki (yzflcyq) - Thursday, 11 August 2011, 11:50 GMT
Strange problem when I try to apply it on my own build. However I sync to the latest source (r30279) and re-apply it it works properly.
My device is Onda VX747, a nice one.

Good job.
Comment by Andree Buschmann (Buschel) - Thursday, 11 August 2011, 19:24 GMT
@gama: freqbase in ymdeltat.c/.h is not initialized. As a result DELTAT->step might be calculated from an unitialized value as well. What is the intended functionality?
Comment by Andree Buschmann (Buschel) - Thursday, 11 August 2011, 21:03 GMT
This patch removes all residual floating point code except from ym2413_emu and ym2612_emu. AY, GBS and HES do not use any floating point now.
Comment by Mauricio Garrido (gama) - Thursday, 11 August 2011, 21:08 GMT
Actually the ymdeltat files are not used at all, i removed them from the gme_patch_v05 (stereo effects test).zip patch but the older ones
still had them included.

By the way i talked to blargg, and he gave some advices about optimization: First he confirmed that no floating point operations are used during playback so we should focus on other aspects as profiling and using IRAM correctly. Currently i'm working on the big arrays from the fm chips.
Comment by Michael Sevakis (MikeS) - Thursday, 11 August 2011, 22:15 GMT
@Buschel: The scratchy noise problem in VGM really is not deterministic on gigabeats S. It plays, then at some point the noise starts. I have no recipe other than to let it just play until it happens. Playing the same track again can work just fine but at some point it starts and persists thereafter in variable severity. Skipping manually seems to clear it but not an auto skip.

This is with the "Sonic the Hedgehog (1991)(Sonic Team)(Sega)" set of vgm's.
Comment by Andree Buschmann (Buschel) - Friday, 12 August 2011, 06:26 GMT
I have tried to reduce the size of the TL-table -- w/o success, NSF and VGM (ym2413) sound strange. Seems like the table lookup does not work as easy as I thought ;) I have attached the patch for review.

Regarding floating point you are right. I also reviewed the code and did not find any calculations which are used *outside* pre-calculation of tables.
Comment by Mauricio Garrido (gama) - Friday, 12 August 2011, 13:09 GMT
@Buschel: I am trying to calculate the tll values dynamically, that way we can remove the whole tll table. But it surely will have an impact in performance (how big?), and of course we would have to use fixed point operations to be usable in a real target. Currently it works ok in the simulator.

Please look at line 697 in emu2413.c, UPDATE_TLL function, that's where the tll values are calculated. Unfortunately i will have some time until monday, if you find some free time you can try using fixed point there. Hope it works decently so we can use the same method for emu8950.
Comment by Mauricio Garrido (gama) - Friday, 12 August 2011, 13:10 GMT
Here is the patch.
Comment by PurlingNayuki (yzflcyq) - Friday, 12 August 2011, 13:46 GMT
@gama: Isn't a waste of time to caculate it at run-time and is it fast enough?
Comment by Andree Buschmann (Buschel) - Friday, 12 August 2011, 17:31 GMT
This patch reduces the RAM size of the buffer to 1/4, simply by using uint8 instead of uint32. The values in the arrays of emu2413 already were within [0..255], the values of emu8950 must be scaled by >>1 to be in the range of [0..255]. This allows to enable NSF and SGC on low memory targets.

To me it seems like the ttl[]-arrays in emu2413 and emu8950 may contain the same data. If so, we can remove one of them, save another 32 KB and may be able to activate KSS and VGM on low memory as well.
Comment by Andree Buschmann (Buschel) - Friday, 12 August 2011, 19:43 GMT
This patch only uses one instance of the tll[]-array. Nevertheless KSS and VGM cannot be enabled for low memory targets.
Comment by Mauricio Garrido (gama) - Saturday, 13 August 2011, 02:27 GMT
@Buschel: could you try this, it removes the TL_TAB defined in ym2612_emu.h, should save some RAM. Not sure if that would be enough.
The code is a bit ugly, i made it in a hurry. Haven't tested many songs yet either :P, just the Thunder Force IV soundtrack and seems to play
fine (???).

By the way did you tryied the TL_V02_floating_point.patch?, if we use that with fixed point operations we could remove the whole TLL array.
Comment by Michael Sevakis (MikeS) - Saturday, 13 August 2011, 02:35 GMT
Might I recommend that fading only be used for tracks above a certain length? Very short ones consist almost entirely of fade.
Comment by Andree Buschmann (Buschel) - Saturday, 13 August 2011, 08:24 GMT
gama, with the latest submit the memory consumption for the tll[]-table is 32 KB in total. This is no problem at all, so we do not need to further tweak this one. To be able to enable KSS and VGM we would need to save another 100+ KB (if I recall correctly about 120 KB for VGM about 170 KB for KSS). Such savings cannot be reached via the tll[]-tables. The memory consumption seems to be caused by emualted CPU RAM.
Comment by Andree Buschmann (Buschel) - Sunday, 14 August 2011, 10:57 GMT
Here is a kind of "proof-of-concept" patch to use IRAM in the nsf-codec to speed up synthesis (+7% for PP5022, +2% on S5L8701).

@gama: Do you haveprofiling results? So, information about which parts of the synthesizers consume the most CPU time?
Comment by Andree Buschmann (Buschel) - Sunday, 14 August 2011, 19:08 GMT
*Very* simple IRAM optimization for emu2413. Speeds up synthesis by +6% against svn (r30313) for PP5022.
Comment by Mauricio Garrido (gama) - Monday, 15 August 2011, 02:36 GMT
> To be able to enable KSS and VGM we would need to save another 100+ KB (if I recall correctly about 120 KB for VGM about 170 KB for KSS)

Ok, i will take a look at those then. By the way did the ym2612_no_TL_TAB_v01.patch made any difference?
Comment by Andree Buschmann (Buschel) - Monday, 15 August 2011, 18:26 GMT
gama, the general idea of ym2612_no_TL_TAB_v01.patch looks very promising. Can you update and verify this patch against latest svn? I would like to make some performance measurements before submitting such patch as it may slow down one of the slowest codecs (VGM)...
Comment by Mauricio Garrido (gama) - Monday, 15 August 2011, 21:14 GMT
OK, i modified the patch to work with the latest revision. Hope it doesn't affect performance that much.

By the way about KSS, if there is no other solution we can always remove only a couple of chips instead of removing the
whole codec, we could, for example, only remove the emu8950 chip (msxaudio) in the opl_apu files and see if it works in a low memory target.
Comment by Andree Buschmann (Buschel) - Tuesday, 16 August 2011, 06:08 GMT
TL_TAB_v02 saves impressive 90 KB (we still need additional 16 KB for low memory targets). The synthesizer is slowed down by 6% on my iPod Video (127.9 MHz -> 135.5 MHz) for the file "test.vgm". I made some minor changes to remove compiler warnings.

If you find reasonable options to further reduce memory consumption in the ym2612 tables we might use the slower but memory optimized configuration for low memory targets, and keep the faster configuration for other targets.

Regarding KSS I am leaning towards keeping it as it is. I do not really like to have different feature sets for the same codec on different targets.
Comment by Mauricio Garrido (gama) - Wednesday, 17 August 2011, 19:55 GMT
@Buschel: Could you try this patch (i used the same name cuz it still includes the TL_TAB changes ;P), it saves some more RAM by replacing the second stereo_buffer with a single blip_buffer in vgm_emu, and replaces the resampler with that found in the gme_patch_v05 (stereo effects test).zip. Also included the disable_oversampling fix found in gme_patch_v05. The codec compiled now for the sansa clip, but not sure if it works OK since i don't have the player.

By the way the gme_patch_v05 all structs have the largest items at the end, i read somewhere it could help performance a little. That patch also it has some nice changes: fixed a couple of small bugs, replaced all tabs with spaces, it has the latest blip_buffer and multi_buffers, and the optional stereo_effects. So I hope some of that changes make it to the SVN later, we can easily remove the stereo effects for low memory targets. I use that version right now in my sansa fuze v2 with the stereo effects enabled and i really like how it sounds, i'm sure other people will like it too.
Comment by Andree Buschmann (Buschel) - Wednesday, 17 August 2011, 22:26 GMT
I splitted your patch into two submits and made some (minor) changes.
1) r30326: Changes to ym2612 emulator, the changed code now selects the fast setting (=tables) for normal targets and the slow (=no tables) for low memory targets
2) r30227: Rest of your patch + enabling VGM for low memory + updating the manual
Thanks!

The next step I am interested in is to bring in the fixes you seem to have included in your gme_patch_v05 version. Can you make an updated patch against latest svn? What in detail is fixed?
Comment by Mauricio Garrido (gama) - Wednesday, 17 August 2011, 23:14 GMT
Now that i'm looking at the v5 patch, i have to say that it has mostly changes, the only fixes i remember were: fixed the disable_oversampling variable in vgm_emu and made the NSF_EMU_APU_ONLY and NSF_EMU_NO_VRC7 options work as expected.

And the changes were:

- replaced all tabs characters with four spaces.
- converted all long variables to int as done in the latest version of gme6pre.
- updated blip_buffer and multi_buffer with the latest versions from gme6pre.
- added support for enhanced stereo effects for classic emulators.
- made the necessary changes to all x_emu.c/h files to support the above changes, like correctly calling buffer_set_channel_count, etc.
- moved most of the large items to the end of structures (i remember someone said this could improve performance a bit).

and not really sure but i think i removed some unnecesary code from here and there.

Well making a diff from the latest revision will take some time, especially because of the fixed point changes, but if you are OK with it i can work on it ;).
Comment by Michael Sevakis (MikeS) - Tuesday, 23 August 2011, 03:43 GMT
Hmmm....not sure what got changed where but the "wax cylinder" sound seems to be history on the beast. I'll report back if I spoke too soon but so far today with maybe an hour of listenening, nothing, where previously it only took a few tracks at most before it would show.
Comment by Thomas Martitz (kugel.) - Wednesday, 24 August 2011, 07:22 GMT
Is nobody interested in getting the playlist handling in?
Comment by Marvin Miller (Mouser X) - Wednesday, 24 August 2011, 08:28 GMT
Are you kidding? I've been constructing M3U playlists for weeks, waiting for that feature to be implemented into SVN. I just don't know any programming, so I've left it to those who are more capable. PLEASE work on that! I haven't actually used the new implementations (my Gigabeat S's file table is producing errors), but I liked how NSF worked before (NSF has changed, so I have something to compare to). Knowing that a change has taken place, I'm a little worried that I'll need those M3U playlists, to be able to listen to my music the way I'm used to.

So.... Just because no one has said anything doesn't mean the feature should be ignored. Please work on it. I'm really looking forward to it! With this codec in SVN, the only format that Rockbox can't play (that I frequently listen to) is PSF.... But those M3U playlists make useability much better for the formats that use them (namely GBS, as it has no real meta-data otherwise).

Last, in case it wasn't obvious, thank you so much for porting this to Rockbox! I can finally listen to VGM/VGZ files on Rockbox! WooHoo! Rockbox has essentially replaced my desktop computer music player now. Mouser X over and out.
Comment by MichaelGiacomelli (saratoga) - Wednesday, 24 August 2011, 18:41 GMT
Thats a very long way of telling us you're not going to do it :)
Comment by Mauricio Garrido (gama) - Wednesday, 24 August 2011, 21:39 GMT
Well, i think we are leaving M3U support for later, first we need to merge all the latest changes to the library. I'm still waiting for Buschel response about the changes found in the v5 patch, specially the stereo enhancement effects.

Also we are working on making all formats work on targets with small RAM and we are also looking how to optimize the code to use less cpu in some formats like VGM or KSS.

By the way, some time ago i wrote a PSF plugin based on sexypsf, i haven't released it yet cuz i'm working on the gme codecs now. It is in a very beta stage and it would need a LOT of optimizations to be usable i most targets but if anyone is interested just let me know.
Comment by Andree Buschmann (Buschel) - Wednesday, 24 August 2011, 22:10 GMT
gama, sorry for the late reply -- I was on vacation for a few days. I would prefer to have the following changes as next steps:

Moving to gme6pre, in detail (from your last post):
- converted all long variables to int as done in the latest version of gme6pre.
- updated blip_buffer and multi_buffer with the latest versions from gme6pre.
- made the necessary changes to all x_emu.c/h files to support the above changes, like correctly calling buffer_set_channel_count, etc.

After bringing this in I would like to discuss the residual open points stereo enhancement and m3u-support.

1) Is stereo enhancement available for all codecs? How will this impact performance?

2) The m3u-support looks blargg-specific to me. How will this handling fit into rockbox' playlist handling?
Comment by Mauricio Garrido (gama) - Wednesday, 24 August 2011, 23:14 GMT
> gama, sorry for the late reply -- I was on vacation for a few days.

No problem, that's what i thought ;).

Ok, i can start working on the first three steps, if you like. Should i make a different patch for each one?

About the last two questions:

1) The stereo effects are available for all classic emus (NSF, GBS, SGC, HES, KSS and master system VGM). And if i remember well, enabling the stereo enhancement did have an impact in performance; 10 - 15 % decrease in codec speed test. It's not that much and we can always try to optimize it later.

2) Yeah those m3u playlists are more like chiptune specific, it seems the creators didn't follow the official extended M3U specification. So i'm not really sure how could we implement it in rockbox. We would need to find a way to detect the files and parse it accordingly, i think there would be no way to parse the general metadata (the one with # at the start of the file), but maybe we could parse the track (subtune) info. Hope someone take a look at it.
Comment by Andree Buschmann (Buschel) - Wednesday, 24 August 2011, 23:46 GMT
> Ok, i can start working on the first three steps, if you like. Should i make a different patch for each one?

If I understood correctly, the gme6pre changes are kind-of an upstream patch? So, updating rockbox' libgme to the latest gme sources? If so, just provide a single patch that implements this update.

> 10 - 15 % decrease in codec speed test. It's not that much and we can always try to optimize it later.

Well, this will at least hurt for VGM and NSF as those might become problematic for more powerful CPU's (e.g. the nano 2G's) with this change. If you can provide a patch (based on the first gme6pre patch) I will make some measurements on my target.

> Hope someone take a look at it.

Which might take a while :o)
Comment by Mauricio Garrido (gama) - Wednesday, 24 August 2011, 23:58 GMT
Ok, i will work on a patch containing the first three steps. And later one with the stereo enhancement to make some performance tests.
Comment by Mauricio Garrido (gama) - Thursday, 25 August 2011, 00:50 GMT
Buschel, it will take a while to make the patch, i may finish it until next week, i will try to merge other changes from gme6pre too.
Comment by Marvin Miller (Mouser X) - Thursday, 25 August 2011, 04:53 GMT
^ Sorry about that.... My best guess is that I refreshed the page, which apparently reposted my comment. :( If it could be removed, I'd appreciate it (I don't see the option available to me to do that).

Regarding PSF (yes, slightly out of place here...), I know a guy who got a "working" sexyPSF port on his Sansa e200. It was a few years ago, but what I recall him saying was that the amount of optimization necessary to get it working was unrealistic. For the "slower" targets, perhaps, but I use a Gigabeat S, and I'm reasonably confident that the Gigabeat S could play PSFs fairly well.... Obviously, you'd want to get it working on as many targets as possible. I'm glad to see that someone else it looking into it! That said, I appreciate your work on the GME codecs, and agree that finishing this up is the better idea.

Regarding the M3U support (I didn't realize the implementation was different for GME), would it help at all if the file extension was changed (so as to not interfere with Rockbox's regular M3U reader)? Perhaps instead of M3U, use "*.GME" or something? Just a thought, but if it makes it easier (so that you don't have to much about with Rockbox's M3U parser), I'd say it's a reasonable suggestion. Mouser X over and out.

Edit: Unintended comment Removed (MikeS)
Comment by Mauricio Garrido (gama) - Friday, 26 August 2011, 13:08 GMT
@Buschel: I started working on the patch, currently i have finished the ay and gbs codecs. I won't be able to work on it on weekend but will continue next week. By the way i'm also including some small changes from gme6pre that will help to remove some redundant code in all codecs, like fading, silence detection, etc. And i will take another look at gme6pre in case other changes were missed.

gama over and out. (Mmm, I think i read that somewhere ;))



Comment by Andree Buschmann (Buschel) - Friday, 26 August 2011, 16:34 GMT
Ok, take your time, this is our hobby :)
Comment by Mauricio Garrido (gama) - Tuesday, 30 August 2011, 23:46 GMT
@Buschel: Here they are, i made both patches already since the effects buffer code was already there ;).

Here is a list of changes:

- replaced common routines (fading, silence detection, ...) with gme6pre track_filter functions,
and removed the respective variables from all emus.
- updated nes_apu and remaining sound chips with latest versions from gme6pre.
- updated hes_cpu, hes_apu and hes_emu with latest versions from gme6pre.
- replaced almost all long variables with integers.
- updated blip_buffer and multi_buffer with latest versions from gme6pre.
- added some multi_buffer calls in all emus to match gme6pre code.
- fixed update_gain call in sgc_emu.
- correctly call check_end in vgm_emu now.
- replaced hes_cpu_io.h with hes_cpu_run.h.

There are two patches, one without the effects buffer and one with them.
Also with some of the above changes the following files are not needed anymore: nes_cpu_io.h, hes_cpu_io.h and gme_types.h.

Well hope you have some time to try them.
Comment by Mauricio Garrido (gama) - Tuesday, 30 August 2011, 23:55 GMT
Forgot, the effects are disabled by default, just comment out the GME_DISABLE_STEREO_DEPTH option in blargg_config.h to enable them ;).
Comment by JoshuaChang (JoshuaChang) - Wednesday, 31 August 2011, 00:20 GMT
i should mention one thing, after svn r30331, most gme codecs's speed were decreased by around 10% on my cowon d2, revert to -O3 optimize switch can fix this issue
Comment by Mauricio Garrido (gama) - Wednesday, 31 August 2011, 00:45 GMT
Yeah, the same thing happens with the sansa fuzev2. I remember i used -O3 because of that ;).
Comment by Andree Buschmann (Buschel) - Wednesday, 31 August 2011, 06:40 GMT
1) The patch does not compile for simulator (win32 cross compilation). I get lots of "undefined reference to `___assert'" and some double defined functions. Please provide a working patch.

2) You have mentioned a fix of "update_gain call in sgc_emu": Is this fix required for the current svn implementation? If so, can you please provide this fix as separate patch?

3) Which codecs in detail slowed down with -O1?
Comment by JoshuaChang (JoshuaChang) - Wednesday, 31 August 2011, 06:54 GMT
1.you should add NDEBUG 1 in config file manually, iirc

3.almost every codecs were slow down when using -O1, maybe this only affect arm926 processor?
Comment by Andree Buschmann (Buschel) - Wednesday, 31 August 2011, 07:14 GMT
> 1.you should add NDEBUG 1 in config file manually, iirc

This *is* defined in blargg_config.h.
Comment by JoshuaChang (JoshuaChang) - Wednesday, 31 August 2011, 07:22 GMT
#define NDEBUG 1 in libgme/blargg_common.h
Comment by Andree Buschmann (Buschel) - Wednesday, 31 August 2011, 09:26 GMT
This does not solve the problem. I will wait for a proper patch now.
Comment by Mauricio Garrido (gama) - Wednesday, 31 August 2011, 14:46 GMT
> 1) The patch does not compile for simulator (win32 cross compilation). I get lots of "undefined reference to `___assert'" and some double defined functions. Please provide a working patch.

Mmm, i'm using the debian-rockbox image with vmware and have no problem compiling both SIMULATOR and real target, so don't know what's wrong ;(.

> 2) You have mentioned a fix of "update_gain call in sgc_emu": Is this fix required for the current svn implementation? If so, can you please provide this fix as separate patch?

Well, it's more an update than a fix. The current codec doesn't handle the update_gain like gme6pre does. They are just a couple of lines actually.
Comment by JoshuaChang (JoshuaChang) - Wednesday, 31 August 2011, 16:16 GMT
i have experienced this simulator compile error before and #define NDEBUG 1 in libgme/blargg_common.h could slove this issue, i'm using win32+mingw, will looking forward to the new patch...
Comment by Andree Buschmann (Buschel) - Wednesday, 31 August 2011, 17:25 GMT
My build environment is Ubuntu 64-bit with SDL 1.2.14. Have you tried to apply your patch to a clean sync'ed rockbox depot?
Comment by Andree Buschmann (Buschel) - Wednesday, 31 August 2011, 18:18 GMT
Got it compilable now, also resync'ed against my latest changes.

Performance measurements with the provided samples show a reasonable speed up (AY +7%, GBS +5%, SGC +13%, VGM (SN76496) +8%, others +/-0%) with this patch.

Edit: Submitted with r30397-30400, plus several fixes for residual compile issues.
Comment by Mauricio Garrido (gama) - Thursday, 01 September 2011, 14:02 GMT
@Buschel: here, the effects patch taken from r30404. I was wrong it does use quite some RAM, 160 KB by default!, but reducing the maximum number of buffers and the echo buffer size it can be only ~64 KB. Anyway hope you can give it a try and let me know what do you think about it.

By the way there is quite some floating point math in the effects buffer but it doesn't affect playback, it is used only once when calculating some values.
Comment by Andree Buschmann (Buschel) - Thursday, 01 September 2011, 18:14 GMT
Hmm, I just tested this and I am not 100% convinced from a sound-wise point of view. I admit that the sound is fuller (which will most likely be a result of the echo/reverb). This is not clearly audible on all of the provided samples. The bad thing is that the stereo image disturbs me in sn76496.vgm where there acoustic centre has moved to left. It does somehow not sound correct and out of balance.
Comment by Mauricio Garrido (gama) - Thursday, 08 September 2011, 21:45 GMT
@Buschel: Here they are, i made two patches, one containing a couple of small bug fixes. And another including the stereo enhancement, which also includes the bug fixes because some of them are needed for the stereo enhancement to work correctly.

Anyway there are the small bug fixes descriptions:

- Converted some remaining floating point stuff to FP in nes_apu.
- Correcly set the error_count in the nes_cpu.
- Fixed some missing call to Blip_set_modified in Nes_vrc6_apu
- Correctly set PSG voices in vgm_emu.
- Correctly reset current_track variable in vgm_emu.
- Correctly call Sound_mute_voices after calling Buffer_set_channel_count in nsf_emu.
Comment by Andree Buschmann (Buschel) - Friday, 09 September 2011, 06:31 GMT
Thanks for the update. A few comments on the bugfix patch:
- in apps/codecs/libgme/nes_apu.c: Apu_enable_nonlinear_() you do not divide the parameter of the first Synth_volume()-call by amp_range anymore. Is this correct?
- wouldn't it be better to comment the whole function as it not used at all?
- what is the new added error_count_ used for? I only see that this is set, but not used.
Comment by Mauricio Garrido (gama) - Friday, 09 September 2011, 13:18 GMT
- in apps/codecs/libgme/nes_apu.c: Apu_enable_nonlinear_() you do not divide the parameter of the first Synth_volume()-call by amp_range anymore. Is this correct?

Yes, that's correct. But i guess we could comment that function, it seems it's an experimental code that is not used by default. I asked blargg
about this one i hope i can get a better explanation from him. I even tested it but not sure about the correct values for sq and tnd.

- what is the new added error_count_ used for? I only see that this is set, but not used.

Ups, i included that one because it is used by the SAP emulator. I wanted to compare the ASAP codec with the one from blargg's gme, but it seems right now the ASAP library does a better job in terms of compatibility, since it supports all types of SAP tracks, and gme only supports three types. But not sure about which one has better emulation and sound quality. So, you can skip that one.
Comment by Andree Buschmann (Buschel) - Friday, 09 September 2011, 17:59 GMT
I submitted your bugfix patch w/o adding the unused error_count-stuff and with commenting out Apu_enable_nonlinear_() with r30490.
Comment by Mauricio Garrido (gama) - Friday, 09 September 2011, 23:19 GMT
Perfect ;). Blargg was kind enough to answer about the Apu_enable_nonlinear, this is what he told me: "Nes_Apu's nonlinear is for use with a special mixer that handles nonlinear mixing. It's very subtle and never was made easy to use", so i guess we can forget about it.
Comment by Andree Buschmann (Buschel) - Saturday, 10 September 2011, 02:08 GMT
Updated your effect patch against latest svn and also migrated the effects code to fixed point.
Comment by Mauricio Garrido (gama) - Saturday, 10 September 2011, 02:47 GMT
That was fast ;). One question, why not use 12 bit precision, as blargg did?.
Comment by Andree Buschmann (Buschel) - Saturday, 10 September 2011, 03:06 GMT
Updated against r30493.

> why not use 12 bit precision, as blargg did?

That is possible of course. Just change FP_ONE_EFFECT to (1LL << 12)
Comment by Andree Buschmann (Buschel) - Sunday, 11 September 2011, 21:17 GMT
- Do not add *any* of the effect buffer code when GME_DISABLE_STEREO_DEPTH is set.
- Add a few comments regarding the size of effects buffers.
Comment by Mauricio Garrido (gama) - Monday, 12 September 2011, 13:41 GMT
> That is possible of course. Just change FP_ONE_EFFECT to (1LL << 12)

Of course, that's easy.

By the way buschel, in case you didn't noticed, in the nsc.c codec file there is an example of how to config each effect individually.
And let me know if there is something else that i can do about the libgme.

I will start working on the PSF plugin again, will try to merge some ARM asm optimizations made by notaz in the SPU code, i hope it works.
Comment by Postolati Maxim (tails_) - Thursday, 29 September 2011, 18:43 GMT
Hi there, some time ago I downloaded SVN version from site and have problem with VRC7 chip, it sounds detuned compared to original. Attached example

P.S. gama, can be older MAME YM2612 emulator used with this codec now?
Comment by Mauricio Garrido (gama) - Thursday, 29 September 2011, 21:24 GMT
Well it is posibble to use it in private builds, cuz it won't make it to the SVN due to the MAME license. I can work on a patch using the MAME versions of ym2413 and ym2612 if you wish.
Comment by Mauricio Garrido (gama) - Thursday, 29 September 2011, 21:27 GMT
@tails_: One question, where can i find the latest MAME versions of those emus?
Comment by MichaelGiacomelli (saratoga) - Thursday, 29 September 2011, 21:28 GMT
>I can work on a patch using the MAME versions of ym2413 and ym2612 if you wish.

Technically you shouldn't even do that since you would violate both the MAME license and GPL, so I'm going to ask that you not discuss this here.
Comment by Mauricio Garrido (gama) - Thursday, 29 September 2011, 23:01 GMT
Ups, OK no more MAME talk then.
Comment by Andree Buschmann (Buschel) - Friday, 30 September 2011, 06:11 GMT
gama, tails_ claims that the current svn implementation (and also the first commit of libgme with r30264) shows this "detuning" effect if compared to the latest patch in  FS#11999 . Do you see a chance to further debug this?
Comment by Mauricio Garrido (gama) - Friday, 30 September 2011, 13:00 GMT
Yes i have confirmed the issue, and i have found the cause. Currently the 2413 emu, used by the VRC7 apu, is running at a different sampling rate than default, and removing the call OPLL_set_rate( &this->opll, (e_uint32)r ) (line 35, nes_vrc7_apu.c) seems to fix the issue.

It seems in the vrc7 apu the 2413 emu must run always at the default rate. The vgm_emu plays fine because it uses a resampler after computing the samples from the 2413 emu, so it can use a different sampling rate.

Comment by Andree Buschmann (Buschel) - Saturday, 01 October 2011, 06:45 GMT
tails_, can you confirm this fixes your issue? I would like to commit the fix.
Comment by Postolati Maxim (tails_) - Saturday, 01 October 2011, 08:12 GMT
@Buschel, works flawlessly
@gama, thanks for fix
Comment by Andree Buschmann (Buschel) - Saturday, 01 October 2011, 08:17 GMT
Submitted with r30623.
Comment by Mauricio Garrido (gama) - Monday, 03 October 2011, 02:46 GMT
Nice, glad to hear it works fine now. Still looking for possible optimizations and enhancements on libgme.
Comment by Mauricio Garrido (gama) - Tuesday, 24 January 2012, 00:52 GMT
Hi, I replaced my fuze v2 for a newer fuze+ a couple of months ago, the player is nice but the rockbox port is still incomplete and has some serious bugs. I was listening to some video game tunes the other day and found that the vgm tunes are not played fine on the fuze+, it sounds like the player can't decode them at realtime,, I'm pretty sure the problem is related to the fuze+ port, but I just wanted to ask here if anyone is having a similar problem with another player.

By the way i noticed that the value of CHUNK_SIZE defined in vgm.c file is (1024*4) but it shoud be (1024*2) like the rest of the gme codecs, i forgot to change it back when i was writting the codec.
Comment by Mauricio Garrido (gama) - Saturday, 27 October 2012, 02:20 GMT
Hi ;), in case anyone is still interested in this, I'm working on some updates for the gme codecs pack. Since kode54 made his changes public I will try to update the current codecs with those changes. I am not very used to GIT commands so I may simply upload the updated files and hope for someone to make a proper patch. You can find kode54 repo here: https://github.com/kode54/Game_Music_Emu.


Well, see you soon.
Comment by Mauricio Garrido (gama) - Friday, 18 January 2013, 15:14 GMT
Hi there, I finished updating the port with the latest kode54 changes. I have attached the updated library and codec files in case anyone is interested. A lot of changes where made to the stable patch so I wont address them all here.

The most important changes are:

- Added support for most of the chips from the vgm format specification version 1.6.0.
- Added support for blargg high quality synthesis in all emus.
- Added support for gme fir resampler in vgm and spc emus.
- Merged the effects buffer support.
- Several updates/fixes to most files.

About the vgm emu new chips, most of them are MAME licensed, so it's obvious they cant make it to an stable release. But there is an option to easily remove them in blargg_config.h. The same for the other two options, they can be enabled or disabled there.

There are two reasons why I updated the library: first, I wanted to make a more complete port from the original sources. And second, I have a very powerfull target, a Samsung YPR0, so I wanted to enjoy the full quality in sound emulation that the library provides. And I can tell you there is a noticeable difference in quality, by disabling the fast synthesis and fast resamplers.

And sorry if I didn't uplad a proper patch file, It's just that since rockobx moved to git, I haven't found the correct way to make a patch ;P.


Comment by Mauricio Garrido (gama) - Friday, 25 January 2013, 15:20 GMT
This update fixes a bug in vgm codec. See ya!
Comment by Human Being (BlackGuyRX) - Tuesday, 09 April 2013, 00:17 GMT
I need help on a few things-

How do i replace the old default codecs with the new ones? Is there a way to change the default playtime of files? some songs are a tad lengthy and will fade-out before they even finish. Also tagged files like Sega VGM files will fade-out to early. Also tagged playlists won't work.

Comment by Michael Sevakis (MikeS) - Tuesday, 09 April 2013, 08:20 GMT
I fixed the VGM codec in the repository for playtime and fade where fades only happen when there are loops and is extended 4 seconds to accomodate the fade. Loopless files don't fade.

To replace codecs, just make a full build and extract to the root directory of the player or copy .codec files to /.rockbox/codecs. Personally I recommend the first way so to ensure compatibility.
Comment by Human Being (BlackGuyRX) - Tuesday, 09 April 2013, 23:09 GMT
>To replace codecs, just make a full build

What is that exactly?
Comment by MichaelGiacomelli (saratoga) - Wednesday, 10 April 2013, 02:46 GMT
>What is that exactly?

Not something you should be emailing everyone on this task. Go ask in the forums if you need help with the instructions.

Loading...