SaveAs patch

[yoyz]

Hi Marc,

I've written a small patch which allow to have a "save as" feature.
Could you review it and tell me what's ok and what is wrong in the implementation in order to allow me to fix it.

IMHO, there is issue, because near everything is implemented in "ProjectView.cpp" which is not the right place, but it should be fixable, I don't know where could it be implemented so a review could help.
For now it's more a "copy as" feature than a "save as", but it work on my laptop, don't have try on dingoo/psp/gp2x.

You can review it here :
https://gist.github.com/yoyz/2789e907a9dd5124f4c9

yoyz

[Mdashdotdashn]

Thanks, It looks like it could work. I have to look at it more closely because there are unwanted parts in the diffs. I'll have a peek at it and possibly integrate it in the main line.

[Mdashdotdashn]

So, it looks like there's no cross platform issue. The only possible problem I see is that after doing save as, you are still working on the original project - if you further modify the song and save, it's the old one that will be modified - while most software make the newly save project active. This would mean at least re-defining the project alias but possibly other stuff. Look at the loading process, it should be covered there.

[yoyz]

Yes, it's a really big issue to be in the old project...
I have written the patch which change the project directory using this interface in AppWindow :
AppWindow::CloseProject()
AppWindow::LoadProject()

Seem to be fine, and it work from a "functional point of view" on my debian laptop.

https://gist.github.com/yoyz/2075e9704ce46a7d67ef

Unfortunately I hit a lot of "random but really frequent" crash when the patch change the project.
From what I understand from gdb, it is related to the audiocallback.
I miss something from the audiocallback management...
Here is a log of one of the crash :

$ gdb ./lgpt.deb-exe :
...
...
[MAPPING] Attached /event/a to key:0:a
[MAPPING] Attached /event/b to key:0:s
[MAPPING] Attached /event/left to key:0:left
[MAPPING] Attached /event/right to key:0:right
[MAPPING] Attached /event/up to key:0:up
[MAPPING] Attached /event/down to key:0:down
[MAPPING] Attached /event/lshoulder to key:0:right ctrl
[MAPPING] Attached /event/rshoulder to key:0:left ctrl
[MAPPING] Attached /event/start to key:0:space
project:/home/yoyz/lgpt/bin/../lgpt_@
[New Thread 0x7ffff0763700 (LWP 21624)]
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=2048
[-D-] Out initialized
[New Thread 0x7fffeff62700 (LWP 21625)]
[Thread 0x7ffff0763700 (LWP 21624) exited]
project:../lgpt_E
[New Thread 0x7ffff0763700 (LWP 21626)]
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=2048
[-D-] Out initialized
[New Thread 0x7fffef761700 (LWP 21627)]

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff0763700 (LWP 21626)]
memcpy_sse2 () at ../sysdeps/x86_64/multiarch/../memcpy.S:267
267 ../sysdeps/x86_64/multiarch/../memcpy.S: No such file or directory.
(gdb) bt
#0 memcpy_sse2 () at ../sysdeps/x86_64/multiarch/../memcpy.S:267
#1 0x0000000000469875 in RTAudioDriver::fillBuffer(short, int) ()
#2 0x000000000046999f in callback(void, void, unsigned int, double, unsigned int, void) ()
#3 0x000000000046f7c5 in RtApiAlsa::callbackEvent() ()
#4 0x00000000004708ed in alsaCallbackHandler ()
#5 0x00007ffff7419b50 in start_thread (arg=) at pthread_create.c:304
#6 0x00007ffff69c50ed in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#7 0x0000000000000000 in ?? ()

[yoyz]

Hi Marc,

it seem this issue is related to my debian box, some library, RTAudio ???
Some old 1.3g_04B crash too, the 1.3m_051 is affected too.
So it is an old issue which could appear while changing project, here above, I try to change project near 15 time on an old lgpt build, and in the end it crash.

If you have an idea or you want more info.

$ uname -a
Linux yoyz-laptop 3.2.0-4-amd64 #1 SMP Debian 3.2.57-3 x86_64 GNU/Linux

yoyz@yoyz-laptop:~/lgpt/bin/ ./lgpt.deb-exe.2
Trying to log [CONFIG] Got config path=/home/yoyz/lgpt/bin/config.xml before logger is installedTrying to log [CONFIG] No (bad?) config.xml before logger is installed[-D-] Audio
[AUDIO] Audio object initialised with
[AUDIO] Api:
[AUDIO] Device:
[AUDIO] Buffer size:256
[AUDIO] Pre Buffer Count:2
[AUDIO] Current API: Linux ALSA
[AUDIO] Found device hw:HD-Audio Generic,0
[AUDIO] Found device hw:HD-Audio Generic,3
[AUDIO] Selecting: hw:HD-Audio Generic,0
[DISPLAY] Using driver x11. Screen (1366,768) Bpp:32
[DISPLAY] Creating SDL Window (320,240)
[DISPLAY] Preparing fonts
[DISPLAY] Preparing font cache
[DISPLAY] Preparing full font cache
[MIDI] 1 input port(s)
[MIDI] 5 output port(s)
[MAPPING] No (bad?) mapping file (bin:mapping.xml)
[-D-] Mapping config
[MAPPING] Attached /event/a to key:0:a
[MAPPING] Attached /event/b to key:0:s
[MAPPING] Attached /event/left to key:0:left
[MAPPING] Attached /event/right to key:0:right
[MAPPING] Attached /event/up to key:0:up
[MAPPING] Attached /event/down to key:0:down
[MAPPING] Attached /event/lshoulder to key:0:right ctrl
[MAPPING] Attached /event/rshoulder to key:0:left ctrl
[MAPPING] Attached /event/start to key:0:space
AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
[ERROR] Failed to open /home/yoyz/lgpt/bin/../lgpt_DEB/samples/
[ERROR] Failed to open /home/yoyz/lgpt/bin/../lgpt_DEB/samples/
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
[ERROR] Failed to open /home/yoyz/lgpt/bin/../lgpt_DEB/samples/
[ERROR] Failed to open /home/yoyz/lgpt/bin/../lgpt_DEB/samples/
[AUDIO] RTAudio device hw:HD-Audio Generic,0 successfully open - buffer=256
[-D-] Out initialized
Segmentation fault

[yoyz]

SIGSEV seem to be fixed with these line in sources/Adapters/RTAudio/RTAudioDriver.cpp :

//mainBuffer_=(char *)((((int)unalignedMain_)+1)&(0xFFFFFFFC)) ;
mainBuffer_=unalignedMain_;

More info from the gdb output :
https://gist.github.com/yoyz/d8fdd0fc727ff5e015f2

[Mdashdotdashn]

I would say the issue is that you are making a 64 bit build and so possibly in that case the aligned buffer would end up on a 32-bit boundary rather than 64. I'm unsure doing the straight assignment would work all the time but it's possible I guess.

[yoyz]

Yes it's a 64 bit build, there is not a lot of issue. Just this pointer which make sometimes the soft crash when the project is loaded. And some sounfont headers which check the size of short, long which fail at compile time.
I have disable the check but I don't have check if soundfont is working so maybe 64 bit doesn't handle soundfont.
10k and some other compo sound right.
The second patch which close the project and load the new project works fine, I will try to give you a clean patch which is checked on dingoo/psp/debian_x86_64.

I don't know how to handle the mainBuffer_ correctly from a cross platform point of view.
If it was only Linux posix_memaligned could had been the good guess, but everything is not related to Linux and I don't know how to write it the right way.

[Mdashdotdashn]

So, usually when there's potentially api conflicts across plaftforms, I deal with them using adapters.
For example memory allocation goes through SYS_MALLOC that is a macro for

System::GetInstance()->Malloc(size)

System in itself is just an installable instance and, depending on the platform, a different version is used. For linux, it would be the one that is in

https://github.com/Mdashdotdashn/LittleGPTracker/tree/master/sources/Adapters/DEB/System

If you extend the interface

https://github.com/Mdashdotdashn/LittleGPTracker/tree/master/sources/System/System

with an aligned malloc api and implement the linux version in DEBSystem, then porting it to other system will be simply adding this method too. I know Windows has an equivalent and most other platforms are most likely posix complient.

For soundfont, it's probably going to be shitty. This library (and as far as I know the only one available) is from the 90's and they didn't give a fuck back then :D

Cool for the patches, try to send them without too many diffs of header changes and so on.

Thanks!
/M

[yoyz]

Hi Marc,

Ok, I begin to understand how it is done now, this is not a bad idea to split things like this. Not easy to understand first, but not bad at all. It took me days to really understand how thing fit together.
For the Soundfont source code I aggree, it seem to come from an old floppy drive.

I've written the patch, I've check by building them on DEB, PSP, DINGOO.
No other check, I will try GP2X as soon as I have a toolchain ready.
Seem to work fine. I had a lot of pain with the PSP which Freeze because the open/copy/close was broken... it was related to path, and I had no previous experience on how to debug PSP code....
I used the logger which help me a lot.

There are multiple patch you can browse it here : https://gist.github.com/yoyz/af1502c26e9680378926

I've split them to make it easier to read, I will try to explain them below :

patch_saveas_20140529_DEBSystem
handle the "root" and "bin" Alias in the same way it is done on other PLATFORM ( DEB/PSP/DINGOO ) , I don't know if it is the correct way, but DEB and DINGOO doesn't do the same thing with the path and I think it's really weird.

patch_saveas_20140529_PSPFileSystem
this diff is not clean, the import part are the include which is broken "Utils" : s@Application/utils/wildcard.h@Application/Utils/wildcard.h@ if you prefer

patch_saveas_20140529_AppWindow
handle the logic to go to the new project after a saveas

patch_saveas_20140529_ViewEvent
enum needed by AppWindow

patch_saveas_20140529_ProjectView
the logic which implement the copy of the lgptdat.sav, sample to the destination project, it use the FileSystem patch

patch_saveas_20140529_FileSystem.cpp
FS copy function

Hope they are not too dirty.

Johann

[yoyz]

Fuck yes, it now works on gp2x f100 too.
So GP2X/PSP/DINGOO/DEB should be ok ( there is the pointer alignement on DEB but it should only affect me ). Unfortunately, my blue midi adapter on my gp2x f100 firmware 2.0 seem to doesn't work....
I have an old 1.1h_043 which work on it, so it should be on the software side....

The patch is : patch_saveas_20140530_ProjectView
it invalidate the previous patch : patch_saveas_20140529_ProjectView

here it is : https://gist.github.com/yoyz/6bad43a91f194ec4a25c

Without this last patch, there is a SIGSEV linked to the management of the "projectName" in :
SaveAsProjectCallback ->OnSaveAsProject -> ViewEvent ve(VET_SAVEAS_PROJECT,-->>>data<<<<--) ;
This data is lost somewhere when the string are freed....
Now I use a "more permanent" area to store the projectName.
This trick is dirty I agree, but malloc is dirty too and the only way to code it strongly is to use an object somewhere, I don't have an idea about how to do it safe and clean...

Johann

[Mdashdotdashn]

Thanks. I'll have a look at all that when I got some brain space. The definition of root is different on the platform because of the layout of the install files. I don't want to change them right now because it would mean replacing the current executable on people's machine wouldn't work, they would have to relocate it. Call it legacy :D

/M

[yoyz]

Yes, user ( and me too ) don't like to change this, it's painfull to do it on handled.
I let you choose and think about the right way to do it when you have spare time.

On the midi side, I have played with the gp2x today, and I have found a midi version which work fine : release : "1.2a_044"
This one seem to be rock stable, I play it today with a firestarter cable and a nanoloop 2.3 linked with a LSDJMC2. Works fine and rock stable for one hour.
I will try to build a debug version and see if there is error opening the ttySX with the HEAD build.

[yoyz]

Fuck, the last patch which should resolve the memory free issue on gp2x is not resolving anything.
I need a permanent storage for the "projectName" not a memory tricks....

Quoting myself :
" Without this patch, there is a SIGSEV linked to the management of the "projectName" in :
SaveAsProjectCallback ->OnSaveAsProject -> ViewEvent ve(VET_SAVEAS_PROJECT,-->>>data<<<<--) "

I had this dmesg, and the apps freeze after a save as :

lgpt.gpe: unhandled page fault at pc=0x0005b18c, lr=0x0005b184 (bad address=0x00000000, code 7)
pc : [<0005b18c>] lr : [<0005b184>] Tainted: P
sp : bf5ffaec ip : 00000000 fp : 00000000
r10: 00000003 r9 : 00001000 r8 : bf5ffbe0
r7 : bf5ffb20 r6 : 002860f8 r5 : 00285174 r4 : 00285170
r3 : 00000000 r2 : 00000000 r1 : 00000000 r0 : 00000000
Flags: nZCv IRQs on FIQs on Mode USER_32 Segment user
Control: C000317F Table: 01CD0000 DAC: 00000015

So I have not fix anything on the gp2x....

[Mdashdotdashn]

Don't worry too much about it. When I'll integrate your code I'll double check it to see if it is sane and fix it if there's an issue. I've got enough milage to be able to do it without any issue :)