apoplexy v2.1b released

Windows and Linux editor of PoP1 (for DOS and SNES) and PoP2 (for DOS).
Locked
User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 5743
Joined: April 9th, 2009, 10:58 pm

apoplexy v2.1b released

Post by Norbert »

I've just released version 2.1b of apoplexy.

NOTE: This version contains bugs. Use version 2.2b instead.

The GNU/Linux version is available here.
[Edit: and...] The Windows version can be downloaded here.

Changes since the last version are:
* Migrated from SDL 1.2 to 2.0.
+ Added several additional gcc warning options in the Makefile.
+ The cursor now changes on the loading screen and when hovering over web links.
* Improved the code quite a bit after receiving new gcc warnings (see above) and after using cppcheck and clang for the first time.
David
The Prince of Persia
The Prince of Persia
Posts: 2846
Joined: December 11th, 2008, 9:48 pm
Location: Hungary

Re: apoplexy v2.1b released [= beta, but faster]

Post by David »

I tried to port, but I ran into a runtime error...
When a chomper sound would be played, the editor crashes. (I put a chomper into the first room of level 1.)
I could narrow down the location of the crash to the free() call in PlaySound().

Also, GDB gave some disturbing error messages:

Code: Select all

This GDB was configured as "i686-pc-mingw32"...c:/cygmnt/prj/pkg/src/gdb/mingw32
/gdb/dwarf2read.c:985: gdb-internal-error: read_comp_unit_head: dwarf from non e
lf file

An internal GDB error was detected.  This may make further
debugging unreliable.  Quit this debugging session? (y or n) n

Create a core file containing the current state of GDB? (y or n) n
This is weird, there is no such problem with apoplexy-2.0 (if I recompile it with debugging info).

When running with GDB, the first chomp does not crash (but it has no sound), but the second does:

Code: Select all

(gdb) run
Starting program: E:\Hasznos\apoplexy-2.1b/apoplexy.exe
warning: Heap corruption detected at 00CCA0F0
Then I tried to make the editor play all sounds.
Only two cause crashes: chomper.wav and extras.wav. These have 11000 Hz sample rates, the others have 11025 Hz.
A-ha! According to my debugging printf calls, cvt.len_cvt (the length of the converted sound) is bigger than what you allocate.
But you allocate dlen*cvt.len_mult bytes. Looks like SDL is giving a wrong len_mult value?
dlen=5120, cvt.len_mult=16
malloc is called with 81920.
cvt.len_cvt = 82106 (length after conversion)
Wait... 82106/5120=16.036328125 is not a whole number. Perhaps SDL should round up instead of down?
But why can't SDL just first take the original length, and calculate the new length, instead of calculating multipliers?

Lastly, I inserted the same debugging printf calls into apoplexy-2.0 (that uses SDL 1.2.8), and there cvt.len_cvt = dlen * cvt.len_mult.
(And I changed 22050 to 44100 to match apoplexy-2.1b)
dlen=5120, cvt.len_mult=16
malloc is called with 81920.
cvt.len_cvt = 81920 (length after conversion)
Perhaps this version truncates the converted sound?
User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 5743
Joined: April 9th, 2009, 10:58 pm

Re: apoplexy v2.1b released [= beta, but faster]

Post by Norbert »

Thanks for giving it a try and for reporting your findings.

I'll give some background information here about why I made the changes I made, because I think it may be related to what you're running into.

The SDL 1.2 to 2.0 Migration Guide says:
SDL 1.2 to 2.0 Migration Guide wrote:Audio

The good news for audio is that, with one exception, it's entirely backwards compatible with 1.2. If you want the new features, they're available to you, but you'll probably just compile and run without them.

That one really important exception: The audio callback does NOT start with a fully initialized buffer anymore. You must fully write to the buffer in all cases. If you don't have enough audio, your callback should write silence. If you fail to do this, you'll hear repeated audio, or maybe audio corruption. If you want to restore the old behavior of unconditionally initializing the buffer, just put an SDL_memset(stream, 0, len) at the start of your callback.
As can be seen in this image (mirror), these are all the audio related differences between apoplexy 2.0 and 2.1b:
- Added: SDL_memset (stream, 0, iLen); /*** SDL2 ***/
- Changed "22050" to "44100".
- Changed "Uint32 amount" to "int iAmount".
- Added: if (unused != NULL) { } /*** To prevent warnings. ***/

The last two changes are to prevent warnings during compilation. (After suggestions from clang/newly added gcc warning options.)
The first I added because the migration guide told me to do so. Without it, all sounds are repeated/corrupted audio.
After I had made that first change, two samples were still distorted, chomper.wav and extras.wav, the two WAVs that you also mentioned.
I too figured out it was probably related to their sample rates, because I tested:
console wrote:norbert@ren ~/Desktop/apoplexy-2.1b/wav $ mplayer *|grep '1ch\|wav'
mplayer: could not connect to socket
mplayer: No such file or directory
Failed to open LIRC support. You will not be able to use your remote control.
Playing check_box.wav.
AO: [pulse] 11025Hz 1ch u8 (1 bytes per sample)
Playing chomper.wav.
AO: [pulse] 11000Hz 1ch u8 (1 bytes per sample)
Playing cross.wav.
AO: [pulse] 11025Hz 1ch u8 (1 bytes per sample)
Playing dosbox.wav.
AO: [pulse] 11025Hz 1ch u8 (1 bytes per sample)
Playing extras.wav.
AO: [pulse] 11000Hz 1ch u8 (1 bytes per sample)
Playing hum_adj.wav.
AO: [pulse] 11025Hz 1ch u8 (1 bytes per sample)
Playing level_change.wav.
AO: [pulse] 11025Hz 1ch u8 (1 bytes per sample)
Playing move_room.wav.
AO: [pulse] 11025Hz 1ch u8 (1 bytes per sample)
Playing ok_close.wav.
AO: [pulse] 11025Hz 1ch u8 (1 bytes per sample)
Playing plus_minus.wav.
AO: [pulse] 11025Hz 1ch u8 (1 bytes per sample)
Playing popup_close.wav.
AO: [pulse] 11025Hz 1ch u8 (1 bytes per sample)
Playing popup.wav.
AO: [pulse] 11025Hz 1ch u8 (1 bytes per sample)
Playing popup_yn.wav.
AO: [pulse] 11025Hz 1ch u8 (1 bytes per sample)
Playing save.wav.
AO: [pulse] 11025Hz 1ch u8 (1 bytes per sample)
Playing screen2or3.wav.
AO: [pulse] 11025Hz 1ch u8 (1 bytes per sample)
Playing scroll.wav.
AO: [pulse] 11025Hz 1ch u8 (1 bytes per sample)
norbert@ren ~/Desktop/apoplexy-2.1b/wav $
I decided: I'll try various sampling rates other than 22050 and if nothing works, I'll file a bug report.
Since 44100 fixed the problem (on Linux), I didn't do the latter.

Virtually all code in the MixAudio and PlaySound functions is from sample code on the old SDL website.
I managed to find the Internet Archive version of that page.
Ye goode olde days.

Apparently, somewhere along the way I also changed...

Code: Select all

cvt.buf = malloc (dlen*cvt.len_mult);
...to...

Code: Select all

cvt.buf = (Uint8 *)malloc (dlen*cvt.len_mult);
Suggestions on how to proceed are most welcome.
Maybe I should point people from the SDL mailing list (or IRC channel) to this thread; maybe a bug report is in order?
David
The Prince of Persia
The Prince of Persia
Posts: 2846
Joined: December 11th, 2008, 9:48 pm
Location: Hungary

Re: apoplexy v2.1b released [= beta, but faster]

Post by David »

Norbert wrote:Suggestions on how to proceed are most welcome.
The simplest workaround would be to replace "dlen*cvt.len_mult" with "dlen*(cvt.len_mult+1)".
I tried this and it works.

By the way, I see that most wavs were exported with an old version of PR that doesn't know about the header of wave resources (and thus treats the header as part of the wave data). It can be heard as a pop at the beginning of sound. This is the most hearable in save.wav.
Most waves should have 11000 Hz samplerates, except popup.wav, which should be 14000 Hz.
Norbert wrote:Maybe I should point people from the SDL mailing list (or IRC channel) to this thread; maybe a bug report is in order?
Yes, this seems to be an SDL bug, see below.
It looks like it was fixed in a newer version, see the end of my post.

I just downloaded the sources of SDL 1.2 and 2.0.
I found some differences between the audio conversion in the two versions: (SDL_audiocvt.c)
1.2 seems to do only "hi_rate = lo_rate*2^x" conversions. (It has a "SDL_RateSLOW" converter, but the only call to it is disabled with an #if.)
If this is not the case, the new rate is rounded to the a rate that satisfies the above equation. (That is, new = old * some power of 2.)
2.0 has a "SDL_BuildAudioResampleCVT" to handle arbitrary rate conversions.
The part that calculates len_mult is this:

Code: Select all

        if (src_rate < dst_rate) {
            const double mult = ((double) dst_rate) / ((double) src_rate);
            cvt->len_mult *= (int) SDL_ceil(mult);
            cvt->len_ratio *= mult;
        } else {
            cvt->len_ratio /= ((double) src_rate) / ((double) dst_rate);
        }
And SDL_ceil is defined as... (in SDL_stdlib.c)

Code: Select all

double
SDL_ceil(double x)
{
#ifdef HAVE_CEIL
    return ceil(x);
#else
    return (double)(int)((x)+0.5);
#endif /* HAVE_CEIL */
}
... and HAVE_CEIL is not defined, because HAVE_LIBC is not defined.
But the custom re-implementation rounds up only from 0.5! Ouch!
Perhaps they assumed that the (int) typecast rounds to the nearest integer, but in fact it rounds (truncates) toward zero.
Even the C standard says that: (in 6.3.1.4)
When a finite value of real floating type is converted to an integer type other than _Bool, the fractional part is discarded (i.e., the value is truncated toward zero).
It seems that the whole "custom math library" was introduced in 2.0, it's not in 1.2.

Finally, it looks like this bug was fixed in a newer version:
(more precisely, it was fixed in the repository, meaning that the next version won't have this bug)
https://bugzilla.libsdl.org/show_bug.cgi?id=2274
http://lists.libsdl.org/pipermail/commi ... 07679.html
http://hg.libsdl.org/SDL/rev/8181c3a4a055
The SDL_ceil function is implemented incorrectly when HAVE_CEIL is not defined (HAVE_LIBC not defined).
[...]
This functions is used in the SDL_BuildAudioResampleCVT function of the audio subsystem (SDL_audiocvt.c), and causes a bug in that function.
Norbert wrote:After I had made that first change, two samples were still distorted, chomper.wav and extras.wav, the two WAVs that you also mentioned.
[...]
I decided: I'll try various sampling rates other than 22050 and if nothing works, I'll file a bug report.
Since 44100 fixed the problem (on Linux), I didn't do the latter.
I just tried it: with 22050 Hz, they are also distorted under Windows.
The length of the converted chomper sound is 41053, which can't be correct, because the target of conversion is a 16-bit format. Plus the data is preceded by an extra byte, effectively swapping the upper and lower bytes of the samples.

The actual conversion is done by SDL_Upsample_U16LSB_1c (in SDL_audiotypecvt.c).
It goes from the end of the buffer to the beginning (because the input and output buffer are the same).
And the calculation of the new size ignores the fact that each sample is 2 byte.

Code: Select all

    const int dstsize = (int) (((double)cvt->len_cvt) * cvt->rate_incr);
So this is another SDL2 bug.

Some other people have noticed this:
http://forums.libsdl.org/viewtopic.php? ... b868fd3415
https://bitbucket.org/bgK/sdl_psl1ght/c ... 5c1aacfa6d
http://forums.libsdl.org/viewtopic.php?t=7975
Another consequence of this bug:
https://bugzilla.libsdl.org/show_bug.cgi?id=1014 ...but that "fix" doesn't fix this bug.
User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 5743
Joined: April 9th, 2009, 10:58 pm

Re: apoplexy v2.1b released [= beta, but faster]

Post by Norbert »

David wrote:The simplest workaround would be to replace "dlen*cvt.len_mult" with "dlen*(cvt.len_mult+1)".
Sounds good to me. ;)
David
The Prince of Persia
The Prince of Persia
Posts: 2846
Joined: December 11th, 2008, 9:48 pm
Location: Hungary

Re: apoplexy v2.1b released [= beta, but faster]

Post by David »

I attached the port to this post.
To make the zip smaller, I deleted the png folder, and didn't include the games.
The DLLs are included.

Plus I made a simple testcase that demonstrates the resampling bug.
It is the one that results in distorted sound; not the other that results in heap corruption.
The latter was already fixed in the repository, the former is not even reported in the bugzilla (or so it seems).
Howerer, the following may be related to it: https://bugzilla.libsdl.org/show_bug.cgi?id=2094
Attachments
sdl_resample_bug.zip
(1.57 KiB) Downloaded 91 times
apoplexy-2.1b-win32-no-images.zip
(1.45 MiB) Downloaded 81 times
User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 5743
Joined: April 9th, 2009, 10:58 pm

Re: apoplexy v2.1b released [= beta, but faster]

Post by Norbert »

David wrote:I attached the port to this post.
Thank you.
I've also updated the first post of this thread to include a download link for the Windows port.
David wrote:[...], the former is not even reported in the bugzilla [...]
Will you report it, including your sample code? Or do you prefer me to do so?

By the way, now I'm curious why I didn't run into the heap corruption problem on Linux, since I didn't use a bleeding edge SDL2 either; any ideas?
David
The Prince of Persia
The Prince of Persia
Posts: 2846
Joined: December 11th, 2008, 9:48 pm
Location: Hungary

Re: apoplexy v2.1b released [= beta, but faster]

Post by David »

Norbert wrote:Will you report it, including your sample code?
Done: https://bugzilla.libsdl.org/show_bug.cgi?id=2389
David
The Prince of Persia
The Prince of Persia
Posts: 2846
Joined: December 11th, 2008, 9:48 pm
Location: Hungary

Re: apoplexy v2.1b released [= beta, but faster]

Post by David »

Norbert wrote:By the way, now I'm curious why I didn't run into the heap corruption problem on Linux, since I didn't use a bleeding edge SDL2 either; any ideas?
Most probably HAVE_LIBC is defined there, which causes HAVE_CEIL to be defined, and therefore SDL_ceil() uses the standard ceil().
Looks like nobody answered to my bug report.
User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 5743
Joined: April 9th, 2009, 10:58 pm

Re: apoplexy v2.1b released [= beta, but faster]

Post by Norbert »

Hm... when PoP2 editing (maybe also PoP1) under Linux (maybe also Windows), v2.1b apparently has (major) memory leakage, especially when a zoomed (2x) view is used.
It also messes up PoP2 images in zoomed mode, for example in the starting room of level 11, but basically all over the place.
Strange that I didn't run into either of those things before.
The memory problem makes sense, since I never really looked into making sure I free up stuff, but I did work quite hard to make the images show up properly. :|
Anyways, it's a beta release and I've moved on to other things.
David
The Prince of Persia
The Prince of Persia
Posts: 2846
Joined: December 11th, 2008, 9:48 pm
Location: Hungary

Re: apoplexy v2.1b released [= beta, but faster]

Post by David »

Norbert wrote:Hm... when PoP2 editing (maybe also PoP1) under Linux (maybe also Windows), v2.1b apparently has (major) memory leakage, especially when a zoomed (2x) view is used.
The worst part is that memory usage increases even when I am not doing anything. And sooner or later it allocates all available memory.

I think I found what causes this:
Text surfaces and textures are not freed.
After each call to CustomRenderCopy (messaget, ...) , I inserted the following:

Code: Select all

	SDL_DestroyTexture(messaget);
	SDL_FreeSurface(message);
This seems to fix the bug.
Norbert wrote:It also messes up PoP2 images in zoomed mode, for example in the starting room of level 11, but basically all over the place.
Seems like the bug affects ruins/temple backgrounds that are bigger than one tile.

The problem is that you multiply source coordinates with iScale in ShowBackground.
If I take those out then it fixes the bug.
Norbert wrote:The memory problem makes sense, since I never really looked into making sure I free up stuff
Is there some debugging tool that, after the program quits, shows memory blocks that were allocated but not freed?
If possible with backtraces showing where were they allocated.
(Something like heaptrc except that heaptrc is for Free Pascal.)
User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 5743
Joined: April 9th, 2009, 10:58 pm

Re: apoplexy v2.1b released [= beta, but faster]

Post by Norbert »

Hey David, it's kind of you to look into this. :)
I'm currently too busy to work on the program, but a month from now I may dive into it again and also fix other stuff I ran into.

Also, I'm still demotivated by Mechner's - what I would describe as - antisocial behavior. People - not just me - take the time to contact him about PoP and he (virtually) never gets back to them. Why would he not reply to something like this or this. He has 'only' 17.2K followers on Twitter, for instance, so it's not like he's being bombarded by questions from fans. Even if I contact him anonymously via different e-mail addresses about different things in a different manner; he just doesn't appear to give a fuck. Pardon my French. I've contacted him no more than a couple of times a year, the last seven years or so, and always with normal remarks and questions. If I remember correctly, the only reply I've ever gotten was this. Not even an acknowledgment of receipt for anything else. I've been thinking about recreating PoP1 with SDL2 for a while now, but Mechner's attitude is what's holding me back. Maybe I'm interpreting his silence as a sign that he wants the (his) old PoP to die. Although he was happy to find his old ProDOS disks. He doesn't strike me as a person with a self-centered personality. It'll be interesting to see what he's got planned for the 25th anniversary on October 3rd - if anything.
Locked