[gpz]

dennisat, on 26 January 2015 - 08:18 AM, said:

I'd modified soundprocess.cs to update every sound source every cycle instead of the one update in four cycles that applied to most. In addition, if the sound response time started to exceed 50ms, I dynamically reduced the sound thread sleeptime. The one in four cycle with a 50ms sleeptime means that most sound sources only get updated every 200ms at best which is probably why considerable stuttering could sometimes be heard when the sound load was heavy. I probably get away with my patches because I have a four core processor so loading up the sound thread doesn't affect other threads too much. A two core processor might perform badly using my patches.

This part is not really clear to me, why you insist on removing the NeedsFrequentUpdate flag. Those sound sources not marked with this really not needed to be updated more frequently. (It shouldn't lead to stuttering, because the sound process is not the sound renderer process, it is just for "preparation".) Of course, apart from position update of the moving sources and moving camera. Those properties need to be updated without any thread sleep. Not even with reduced thread sleep, but without any. Unfortunately I don't have too much experience with thread programming, so I don't know what the real solution would be...

[James Ross]

gpz, on 26 January 2015 - 09:00 AM, said:

Unfortunately I don't have too much experience with thread programming, so I don't know what the real solution would be...

So, I think there are two kinds of solution here:

• We could adjust the code so that changes to sound source lists are updated in a thread-safe way, which will be more complex for removal but still possible, and continue setting the position/velocity in Updater process and leaving the other updates to the Sound process.
  
• We could have the Updater process save the minimum data needed to a shared location to update the position/velocity in the Sound process, which will need updating to do this constantly while also performing its other sound duties.

[dennisat]

James Ross, on 26 January 2015 - 01:19 PM, said:

[*]We could have the Updater process save the minimum data needed to a shared location to update the position/velocity in the Sound process, which will need updating to do this constantly while also performing its other sound duties.
[/list]

In the thread safe version this data is already available and used to update the sound position and velocity. At the moment, however, the sound process only cycles every 50ms and this does not update every sound source. All sound sources are updated approximately every 200ms. The sound/position update code is very simple and on the surface it appears it could be formed into a method which is cycled through at a much faster rate while maintaining the slow cycle for the major updates.

My personal mods adopt the brute force technique of updating everything every 50ms but this is expensive on resources and is probably not a good solution for lower powered systems.

Dennis

[gpz]

James Ross, on 26 January 2015 - 01:19 PM, said:

So, I think there are two kinds of solution here:

• We could adjust the code so that changes to sound source lists are updated in a thread-safe way, which will be more complex for removal but still possible, and continue setting the position/velocity in Updater process and leaving the other updates to the Sound process.
  
• We could have the Updater process save the minimum data needed to a shared location to update the position/velocity in the Sound process, which will need updating to do this constantly while also performing its other sound duties.

I think any of these methods would be sufficient. It is no problem updating the positions/velocities from the Sound process, provided it is done fast enough.

[Csantucci]

Today I at last had time to have a comparison check (with earphones) of the actual OR version and of one with Dennis' patch applied.
I found one difference: if you are in view 4 and a train at 150 Km/h passes by with the horn pressed, you have a nice Doppler effect with smoothly descending sound frequency with the actual OR version, while in Dennis' version you hear a stepwise sound frequency reduction: not a tragic problem, but a problem, that MSTS does not have. This probably arises from the fact that the position and speed (in this case I think position is what counts) calculation is done max 20 times per second in case of Dennis' version, while it is done FPS times per second in the actual OR version.
I am wondering if there is a more satisfying solution (suggestions possibly not requiring a master degree in computer science are welcome). I thought about something that is not a lock, but a skip mechanism: here we have sound process and updater process concurring; when neeeded each of them declares itself as needing to use the common resource; after that it tests if the other process is already using the common resource; if not it performs the sound calculations, if yes it skips the sound calculations (it will perform them at next update); in both cases it frees then the common resource. I will have some test.

[gpz]

I also hear the wrong doppler effect for electric locomotives motor sound at lower speeds too. Your described solution is the same as James' 1st. The problem with Dennis' patch is that it is incomplete. The balancing of updates was removed, just because it is easier to code, and that computer is fast enough, which is fine for a single person, but not a general solution. Probably the faster update time for positions could be achieved by James' 2nd solution.

[dennisat]

Something you might like to try, that I have implemented, is to mark mobile sound sources as "NeedsFrequentUpdate" when they exceed 15m/sec. I think this improves things a lot, certainly on a 4 core processor.

Dennis

[gpz]

The drawbacks of this is, that
1. Even NeedsFrequentUpdate-flagged sound sources' positions are updated too slowly for a fast moving sound,
2. A full update on sounds sources is more than just the needed simple position update. This is just for different purposes.

[Csantucci]

This is what I have prepared.

This is the patch:

There is a hypothesis under this patch: the crashes were due because some SoundSourceID's were deleted by the sound process while still being accessed by the updater process. Only uploading and having people running this will show if this is the only case that can cause crashes.

and these are the two runactivities:

I have put them here to allow checking if the sound quality has remained substantially the same.

After (hopefully) getting comments on patch and runactivities I will decide if the patch is worth uploading.

Files removed because new release available.

[dennisat]

Isn't there likely to be an exposure around the new flag "updaterWorking"?

Let's say that the Sound process has reached the position where it checks "updaterWorking". Let's say "updaterWorking" is set false. The loop to update the sound streams then starts. At this point, the Updater process could reach the local copy of "SoundSourceIDs" and then set flag "updaterWorking" to true. The Updater process then starts trying to update "SoundSourceIDs", via its local copy, while the Sound process is still likely to be removing some of them.

If I read this right, "updaterWorking" should be protected by Interlocked Compare/Exchange.

Dennis

Edit:
I think there is a bigger potential problem here. It is highly unlikely that the Sound source being processed by the Sound process actually relates to the Car being processed by the Updater.