Hi folks,
I have not much experience with memorystuff. I have this game i am working on. I'm trying to create an animated background and it works :) however, after a while my game crashes. Now since the background load correctly, multiple times even but after alot of loaded backgrounds the gamecrashes eventually. Now I think it has something to do with memory usage that eventually is full and then the image data is all messed up because of memory pointers that point to sections that have been altered or something.
Anyway i have four questions:
1.) How can i detect memoryleaks or what are typical errors made?
2.) I use the png library and the graphics.c file from the yelarb tutorialfiles. I load the image using loadImage and when i go to the next background i unload it with freeImage and load the new one again with the loadImage again. After a while this crashes :S Am i forgetting something or should this be correct and should this work?
3.) Is there some code that frees memory that has currently not been used anymore?
4.) If i go from 1 function to another is the first function still in memory waiting that you return to it? if so when i load multiple times a function without closing it with a return can this cause memory leaking?
hope yo uguys can answer some questions
greets ghoti
Memory messed up question
Memory messed up question
My PSP games:
Boxy II: http://www.ghoti.nl/boxyii.php
Elementals: http://www.ghoti.nl/Elementals.php
Boxy II: http://www.ghoti.nl/boxyii.php
Elementals: http://www.ghoti.nl/Elementals.php
Hi,
i have found out that the function fopen is not the most memory friendly function :S
Since i use the functions from the graphics.h files loadImage, lets say around 25 times when i go to a new level, fopen is also called 25 times. I have read here : http://forums.ps2dev.org/viewtopic.php? ... ight=fopen that the fopen function leaks some memory everytime it is opened :( Since i use it alot in my program my memory fills up eventually :( can i do something about this?
Maybe i am not correct but here is wat happened:
i use this function to see howmany free ram i have:
i have made my Sqaure button run the nextlevel code to speed the testing a little up. When i press 1 time i run the fopen 25 times(approx.) when i do this nextlevel thing like four time or so i see a big change in freememory when i start out with 10456789 i then have 8456789 left. The after 4 times or so nextlevel pressed i have 6456789 left. This goes on until my memory is out. :( i have this problem like for ages but i no have figured out that my nextlevel function is the error, when i don't have any memory left the loadImage function returns a Null because my image is null at that time ( i use assert to check this.)
So has anybody any ideas of how to fix this? is there a way of freeing that lost data? or are there better open functions?
all ideas are appriciated :)
greets ghoti
i have found out that the function fopen is not the most memory friendly function :S
Since i use the functions from the graphics.h files loadImage, lets say around 25 times when i go to a new level, fopen is also called 25 times. I have read here : http://forums.ps2dev.org/viewtopic.php? ... ight=fopen that the fopen function leaks some memory everytime it is opened :( Since i use it alot in my program my memory fills up eventually :( can i do something about this?
Maybe i am not correct but here is wat happened:
i use this function to see howmany free ram i have:
Code: Select all
#include <malloc.h>
int __freemem()
{
void *ptrs[480];
int mem, x, i;
for (x = 0; x < 480; x++)
{
void *ptr = malloc(51200);
if (!ptr) break;
ptrs[x] = ptr;
}
mem = x * 51200;
for (i = 0; i < x; i++)
free(ptrs[i]);
return mem;
}So has anybody any ideas of how to fix this? is there a way of freeing that lost data? or are there better open functions?
all ideas are appriciated :)
greets ghoti
My PSP games:
Boxy II: http://www.ghoti.nl/boxyii.php
Elementals: http://www.ghoti.nl/Elementals.php
Boxy II: http://www.ghoti.nl/boxyii.php
Elementals: http://www.ghoti.nl/Elementals.php
Hi,
The code that uses the fopen is in the loadImage function. I have not created it but it is closes everytime int that code:
And my code does only this:
the way i see it there is nothing wrong with this code.
i shall try the fopen/fclose only to see what happens but maybe you can take a look at the code above to see if there is an error?
greets ghoti
The code that uses the fopen is in the loadImage function. I have not created it but it is closes everytime int that code:
Code: Select all
Image* loadImage(const char* filename)
{
png_structp png_ptr;
png_infop info_ptr;
unsigned int sig_read = 0;
png_uint_32 width, height;
int bit_depth, color_type, interlace_type, x, y;
u32* line;
FILE *fp;
Image* image = (Image*) malloc(sizeof(Image));
if (!image) return NULL;
if ((fp = fopen(filename, "rb")) == NULL) return NULL;
png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
if (png_ptr == NULL) {
free(image);
fclose(fp);
return NULL;;
}
png_set_error_fn(png_ptr, (png_voidp) NULL, (png_error_ptr) NULL, user_warning_fn);
info_ptr = png_create_info_struct(png_ptr);
if (info_ptr == NULL) {
free(image);
fclose(fp);
png_destroy_read_struct(&png_ptr, png_infopp_NULL, png_infopp_NULL);
return NULL;
}
png_init_io(png_ptr, fp);
png_set_sig_bytes(png_ptr, sig_read);
png_read_info(png_ptr, info_ptr);
png_get_IHDR(png_ptr, info_ptr, &width, &height, &bit_depth, &color_type, &interlace_type, int_p_NULL, int_p_NULL);
if (width > 512 || height > 512) {
free(image);
fclose(fp);
png_destroy_read_struct(&png_ptr, png_infopp_NULL, png_infopp_NULL);
return NULL;
}
image->imageWidth = width;
image->imageHeight = height;
image->textureWidth = getNextPower2(width);
image->textureHeight = getNextPower2(height);
png_set_strip_16(png_ptr);
png_set_packing(png_ptr);
if (color_type == PNG_COLOR_TYPE_PALETTE) png_set_palette_to_rgb(png_ptr);
if (color_type == PNG_COLOR_TYPE_GRAY && bit_depth < 8) png_set_gray_1_2_4_to_8(png_ptr);
if (png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS)) png_set_tRNS_to_alpha(png_ptr);
png_set_filler(png_ptr, 0xff, PNG_FILLER_AFTER);
image->data = (Color*) memalign(16, image->textureWidth * image->textureHeight * sizeof(Color));
if (!image->data) {
free(image);
fclose(fp);
png_destroy_read_struct(&png_ptr, png_infopp_NULL, png_infopp_NULL);
return NULL;
}
line = (u32*) malloc(width * 4);
if (!line) {
free(image->data);
free(image);
fclose(fp);
png_destroy_read_struct(&png_ptr, png_infopp_NULL, png_infopp_NULL);
return NULL;
}
for (y = 0; y < height; y++) {
png_read_row(png_ptr, (u8*) line, png_bytep_NULL);
for (x = 0; x < width; x++) {
u32 color = line[x];
image->data[x + y * image->textureWidth] = color;
}
}
free(line);
png_read_end(png_ptr, info_ptr);
png_destroy_read_struct(&png_ptr, &info_ptr, png_infopp_NULL);
fclose(fp);
return image;
}Code: Select all
// -----------------------------------------
//
// GetNewTheme
//
// -----------------------------------------
int GetNewTheme(void){
// freeing the images
if (bClearThemes == FALSE) {
freeImage(Blocks[1]);
Blocks[1] = NULL;
freeImage(Blocks[2]);
Blocks[2] = NULL;
freeImage(Blocks[3]);
Blocks[3] = NULL;
freeImage(Blocks[4]);
Blocks[4] = NULL;
freeImage(Blocks[5]);
Blocks[5] = NULL;
freeImage(Blocks[6]);
Blocks[6] = NULL;
freeImage(Blocks[7]);
Blocks[7] = NULL;
freeImage(Message[1]);
Message[1] = NULL;
freeImage(Message[2]);
Message[2] = NULL;
freeImage(Message[3]);
Message[3] = NULL;
freeImage(Message[4]);
Message[4] = NULL;
freeImage(Message[5]);
Message[5] = NULL;
freeImage(Message[6]);
Message[6] = NULL;
freeImage(Message[7]);
Message[7] = NULL;
freeImage(Message[8]);
Message[8] = NULL;
freeImage(Fx[1]);
Fx[1] = NULL;
freeImage(Fx[2]);
Fx[2] = NULL;
freeImage(Fx[3]);
Fx[3] = NULL;
freeImage(Fx[4]);
Fx[4] = NULL;
freeImage(Fx[5]);
Fx[5] = NULL;
freeImage(GameBackground);
GameBackground = NULL;
freeImage(Line);
Line = NULL;
freeImage(Pause);
Pause = NULL;
}
// check if everything has been emptied
assert(Blocks[1] == NULL);
assert(Blocks[2] == NULL);
assert(Blocks[3] == NULL);
assert(Blocks[4] == NULL);
assert(Blocks[5] == NULL);
assert(Blocks[6] == NULL);
assert(Blocks[7] == NULL);
assert(Message[1] == NULL);
assert(Message[2] == NULL);
assert(Message[3] == NULL);
assert(Message[4] == NULL);
assert(Message[5] == NULL);
assert(Message[6] == NULL);
assert(Message[7] == NULL);
assert(Message[8] == NULL);
assert(Fx[1] == NULL);
assert(Fx[2] == NULL);
assert(Fx[3] == NULL);
assert(Fx[4] == NULL);
assert(Fx[5] == NULL);
assert(GameBackground == NULL);
assert(Line == NULL);
assert(Pause == NULL);
int i;
bBackgroundAnimation = FALSE;
iBackgroundAnimationCount = 0;
if (iGameModeRandom == 1) {
//game mode Random so random theme
if (iTotalOfThemes == 0){
// swap the lists
for(i = 0;i<iTotalOfThemesPlayed;i++){
sprintf(sThemeList[i] ,"%s", sThemeListPlayed[i]);
}
iTotalOfThemes = iTotalOfThemesPlayed;
iTotalOfThemesPlayed = 0;
bClearThemes = TRUE;
GetNewTheme();
}
if (iTotalOfThemes == 1) { iCurrentThemes = 0; }
else {
iCurrentThemes = get_randomNew(0,iTotalOfThemes-1);
}
sprintf(sThemeListPlayed[iTotalOfThemesPlayed], "%s", sThemeList[iCurrentThemes]);
iTotalOfThemesPlayed++;
for(i=iCurrentThemes;i<iTotalOfThemes; i++){
sprintf(sThemeList[i] ,"%s", sThemeList[i+1]);
}
iTotalOfThemes--;
}
else {
if (iTotalOfThemes == 0){
// swap the lists
for(i = 0;i<iTotalOfThemesPlayed;i++){
sprintf(sThemeList[i] ,"%s", sThemeListPlayed[i]);
}
iTotalOfThemes = iTotalOfThemesPlayed;
iTotalOfThemesPlayed = 0;
bClearThemes = TRUE;
GetNewTheme();
}
sprintf(sThemeListPlayed[iTotalOfThemesPlayed], "%s", sThemeList[0]);
iTotalOfThemesPlayed++;
for(i=0;i<iTotalOfThemes; i++){
sprintf(sThemeList[i] ,"%s", sThemeList[i+1]);
}
iTotalOfThemes -= 1;
}
sprintf(sBuffer, "%s/Block1.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Blocks[1] = loadImage(sBuffer);
sprintf(sBuffer, "%s/Block2.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Blocks[2] = loadImage(sBuffer);
sprintf(sBuffer, "%s/filledBlock1.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Blocks[3] = loadImage(sBuffer);
sprintf(sBuffer, "%s/filledBlock2.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Blocks[4] = loadImage(sBuffer);
sprintf(sBuffer, "%s/killedBlock.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Blocks[5] = loadImage(sBuffer);
// 6: special 1
sprintf(sBuffer, "%s/Block1Special.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Blocks[6] = loadImage(sBuffer);
// 7: special 2
sprintf(sBuffer, "%s/Block2Special.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Blocks[7] = loadImage(sBuffer);
sprintf(sBuffer, "%s/background.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
GameBackground = loadImage(sBuffer);
sprintf(sBuffer, "%s/pause.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Pause = loadImage(sBuffer);
sprintf(sBuffer, "%s/line.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Line = loadImage(sBuffer);
sprintf(sBuffer, "%s/message1.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Message[1] = loadImage(sBuffer);
sprintf(sBuffer, "%s/message2.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Message[2] = loadImage(sBuffer);
sprintf(sBuffer, "%s/message3.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Message[3] = loadImage(sBuffer);
sprintf(sBuffer, "%s/message4.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Message[4] = loadImage(sBuffer);
sprintf(sBuffer, "%s/message5.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Message[5] = loadImage(sBuffer);
sprintf(sBuffer, "%s/message6.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Message[6] = loadImage(sBuffer);
sprintf(sBuffer, "%s/message7.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Message[7] = loadImage(sBuffer);
sprintf(sBuffer, "%s/message8.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Message[8] = loadImage(sBuffer);
sprintf(sBuffer, "%s/fx1.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Fx[1] = loadImage(sBuffer);
sprintf(sBuffer, "%s/fx2.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Fx[2] = loadImage(sBuffer);
sprintf(sBuffer, "%s/fx3.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Fx[3] = loadImage(sBuffer);
sprintf(sBuffer, "%s/fx4.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Fx[4] = loadImage(sBuffer);
sprintf(sBuffer, "%s/fx5.png", sThemeListPlayed[iTotalOfThemesPlayed-1]);
Fx[5] = loadImage(sBuffer);
bClearThemes = FALSE;
// check if everything has been emptied
assert(Blocks[1] != NULL);
assert(Blocks[2] != NULL);
assert(Blocks[3] != NULL);
assert(Blocks[4] != NULL);
assert(Blocks[5] != NULL);
assert(Blocks[6] != NULL);
assert(Blocks[7] != NULL);
assert(Message[1] != NULL);
assert(Message[2] != NULL);
assert(Message[3] != NULL);
assert(Message[4] != NULL);
assert(Message[5] != NULL);
assert(Message[6] != NULL);
assert(Message[7] != NULL);
assert(Message[8] != NULL);
assert(Fx[1] != NULL);
assert(Fx[2] != NULL);
assert(Fx[3] != NULL);
assert(Fx[4] != NULL);
assert(Fx[5] != NULL);
assert(GameBackground != NULL);
assert(Line != NULL);
assert(Pause != NULL);
return 0;
}i shall try the fopen/fclose only to see what happens but maybe you can take a look at the code above to see if there is an error?
greets ghoti
My PSP games:
Boxy II: http://www.ghoti.nl/boxyii.php
Elementals: http://www.ghoti.nl/Elementals.php
Boxy II: http://www.ghoti.nl/boxyii.php
Elementals: http://www.ghoti.nl/Elementals.php
Re: Memory messed up question
use a memory manager. like -> http://www.paulnettle.com/pub/FluidStud ... anager.zipGhoti wrote:Hi folks,
1.) How can i detect memoryleaks or what are typical errors made?
This is an example of nice structured code. Please don't forget to mention in some credits page that you used this function from Lua Player, because this is the only requirement for the BSD licence of Lua Player, if you want to use source code from it.Ghoti wrote:Code: Select all
Image* loadImage(const char* filename)
Please refactor this code, it's impossible to understand the control flow. Some ideas for refactoring:Ghoti wrote: And my code does only this:
Code: Select all
// ----------------------------------------- // // GetNewTheme // // ----------------------------------------- int GetNewTheme(void){
- you are calling the function GetNewTheme recursivly, which I think is not necessary
- instead of Blocks[1] = NULL; freeImage(Blocks[2]); Blocks[2] = NULL; ... you should use a for loop
- sprintf(foo,"%s", bar); would be better strcpy(foo, bar)
- you don't need all the asserts, if you have a clear control flow (but you should test the value returned from loadImage and display an error message instead of an assert, because you don't want to crash your program, if the user ejects the memory stick)
- you should extract functions, if it is too long, e.g. "loadImage" and "releaseImage", which you can test independently of the rest of the program
- are you sure that with your firmware and installed drivers or loaders the assert function didn't crash your program?
Hi,
@shine: Thanks for the great feedback :) as for your points:
the lua code: I did not know it was lua code, i got it from the Yelarb tutorials so i gave him credit in my credits. You sure it is from lua and not his own? then i will rectify the fault.
recursivly: hmm i shall look to make it not recursive maybe that is the problem. can it be because after you have entered the function it returns again and then loads up the images again by creating them and then the images are lost of which the initial pointer points to and so my memory fills up?
loop: I had that in a loop, however to be certain it did not load any index or so that has not been initialize i wrote it out this way (so it was just for checking)
asserts: Also put them in to check everything because of the memleak/badcode/crash so it also did not crash my code since it crashed before i ever had it in.
(BTW i'm using firmware 1.5)
I'll use the strcpy function instead? is this just nicer code or does it do it saver, better, quicker or some other optimization?
@weak: Thanks for the memleak checker, i'm going to look into it :)
greets ghoti
@shine: Thanks for the great feedback :) as for your points:
the lua code: I did not know it was lua code, i got it from the Yelarb tutorials so i gave him credit in my credits. You sure it is from lua and not his own? then i will rectify the fault.
recursivly: hmm i shall look to make it not recursive maybe that is the problem. can it be because after you have entered the function it returns again and then loads up the images again by creating them and then the images are lost of which the initial pointer points to and so my memory fills up?
loop: I had that in a loop, however to be certain it did not load any index or so that has not been initialize i wrote it out this way (so it was just for checking)
asserts: Also put them in to check everything because of the memleak/badcode/crash so it also did not crash my code since it crashed before i ever had it in.
(BTW i'm using firmware 1.5)
I'll use the strcpy function instead? is this just nicer code or does it do it saver, better, quicker or some other optimization?
@weak: Thanks for the memleak checker, i'm going to look into it :)
greets ghoti
My PSP games:
Boxy II: http://www.ghoti.nl/boxyii.php
Elementals: http://www.ghoti.nl/Elementals.php
Boxy II: http://www.ghoti.nl/boxyii.php
Elementals: http://www.ghoti.nl/Elementals.php
Yes, it's a verbatim copy of an older version of the function used in Lua Player. The current version has added support for loading it from memory (e.g. if you bind your images to your EBOOT file). See graphics.cpp in SVN repository.Ghoti wrote:the lua code: I did not know it was lua code, i got it from the Yelarb tutorials so i gave him credit in my credits. You sure it is from lua and not his own? then i will rectify the fault.
I don't know. You should really clean it up and extract small functions, which does exactly one thing.Ghoti wrote: recursivly: hmm i shall look to make it not recursive maybe that is the problem. can it be because after you have entered the function it returns again and then loads up the images again by creating them and then the images are lost of which the initial pointer points to and so my memory fills up?
It's faster, but this is not important: It makes your code more readable, because if you read "strcpy" you don't need to parse the format string when trying to understand the code.Ghoti wrote: I'll use the strcpy function instead? is this just nicer code or does it do it saver, better, quicker or some other optimization?
@shine: Thank you very much again, I have solved the problem, it was indeed the not returning with the recursive. I have fixed it and now it works fine! i even have found out a problem i was having with my mp3player and now that has been corrected also !!! i'm so happy :D:D:D
anyway you learned me a thing or two so thank you!
(have always coded in visual basic, so i have not much exp. with c and c++)
how do i properly thank the guys at lua? just say in my credits thanks to lua-team?
greets ghoti
anyway you learned me a thing or two so thank you!
(have always coded in visual basic, so i have not much exp. with c and c++)
how do i properly thank the guys at lua? just say in my credits thanks to lua-team?
greets ghoti
My PSP games:
Boxy II: http://www.ghoti.nl/boxyii.php
Elementals: http://www.ghoti.nl/Elementals.php
Boxy II: http://www.ghoti.nl/boxyii.php
Elementals: http://www.ghoti.nl/Elementals.php
I wrote this part of the code of Lua Player, but you can add something like this: "some functions copied from Lua Player (http://www.luaplayer.org)"Ghoti wrote:how do i properly thank the guys at lua? just say in my credits thanks to lua-team?
Maybe I'll get some money from Google-Ads, if your program gets famous :-)