Tuesday, June 5, 2012

Making things better

On this week, I fixed architecture mistakes in event registrator. When I've enabled registering of all time events (when somebody whant to know about system ticks) there strange mistakes occurred and it didn't work at all.  So, I've enabled registering time events only in some places of engine. But even in comments to my gsoc proposal, LordHoto noticed that need something to do with it, because when different parts of VM see different time, this will cause problems. In addition, direct replacement of g_system->getMillis() by the g_eventRec.getMillis() is bad, untransparent, unflexible, etc. At last, after one of commits, my mentor _sev proposed me to liquidate this dirty hack, by getting rid of all g_eventRec calls and doing registration of all g_system->getMillis() calls.
First problem we have found - events might be recorded before full initialization of registrator. It was fixed by simple  flag adding which shows that the registrator completely initialized.
After I solved this problem, I tried to find another bugs during at about two days. Something went wrong, but I couldn't find what exactly. _sev very helped me when suggested to use backtrace macros which will show who and when call getMillis fucntion. I've begun to write this data to log and analyze it. This helped me to find next two problems:
DefaultEventManager::pollEvent called more often durring the playback. And also it calls if user doing any action (moves mouse, presses keyboard keys). I solve this programm by adding a stop and start registrating feature  to registrator class. Now it just skips time messages from pollEvent function.
Last problem was hard to find, but very easy to solve. To describe a problem I should tell how we've developed wrapper for audio registering.
Original audio subsystem can be represented by this sequence diagramm:

I. e, SDL audio subsystem give mixer manager to know about it's readiness to play next chunk of sound and give buffer which must be filled by sound data. Manager give this event to platform-independent mixer which must to fill this buffer. Mixer check is there any free chanal and give this buffer to it. Chanal fill the buffer by sound data and ask the current time from system. System ask for time from events registrator.

This scheme worked perfectly, but had a little problem: it was very hard to synchronize sound with logic. Wasn't possible to determine how often will be calback called and what will be buffer size. _sev proposed to get rid of the dependence from  SDLInternals and replace SdlMixerManager to own wrapper. Obviously, there was needed to use some external events source which  will set how frequently mixer's callback will called. Also, this source must have possibility of sinchronization with registrator. We decided to use processMillis for this purposes.   Regarding to this idea, sequence acquired the following form:

Very easy to see that this sequence resulted in an infinite loop and stack overflow. We decied to make a bypass to disable events recording while mixer's callback executing 

Now, in case of necessary, registrator taken out from sequence and system time became the result of function. It saved from stack overflow and worked quite well. When I began to register all calls of getMillis, problem has arisen where I did not expect it. There is function Chanal::getElapsedTime, which returns the time elapsed from begining of sound playing. It getting first time period in Chanal::mix function (during it bypass works and time is real from system). Another time it gets from events recorder. So difference between them is undefined and different for each replay. After problem was found it was very easy to fix it and make result of getElapsedTime function positive and predictable.

No comments:

Post a Comment