Set start time to play sound in A_PlaySound

Moderator: GZDoom Developers

User avatar
Nash
 
 
Posts: 17434
Joined: Mon Oct 27, 2003 12:07 am
Location: Kuala Lumpur, Malaysia
Contact:

Re: Set start time to play sound in A_PlaySound

Post by Nash »

Hey Chris. If it's not too much trouble, can you care to comment if what I did here is okay, or did I miss anything vital? Since I am working on a standalone game, where the engine needed to be forked anyway for a bunch of other game-specific engine features, I went ahead and did this while deliberately ignoring the modded sound issue for the time being. Barring the modded sound issue, how did I do here?

Here's my fork's branch: https://github.com/nashmuhandes/LAD/com ... 7078365ba0

I have tested this for a bit and it seems to work without hitches, including serialization and handling of looped sounds and all that. :) Here's a quick demo of what it's used for:



The start time parameter is used to make sure songs will continue to play "globally" even between maps. In addition to the radio channels, I also used the sound start time to randomize the starting position of the long ambient loops in the background (like the sound of the birds, the wind, etc) - incidentally, THE original motivation for this entire suggestion back when I made this thread. The sound ambience thing is less obvious in the video I linked to but this makes a huge difference in an in-game sound design context - not having the birds chirp the exact same way every time a map is entered makes all the difference. :P
User avatar
Chris
Posts: 2940
Joined: Thu Jul 17, 2003 12:07 am
Graphics Processor: ATI/AMD with Vulkan/Metal Support

Re: Set start time to play sound in A_PlaySound

Post by Chris »

It looks more or less alright. There's just a couple things to be aware of:

If you specify a start time that's out of range of the sound being played, it'll be ignored and start from the beginning (it won't wrap).
MarkStartTime seems to ignore the start time (IIRC, that function's only used when a looping sound tries to start and fails, so when it tries to start again later it plays from roughly the correct point).
You won't be able to exactly synchronize multiple sounds this way, given differences in the timing clocks and that get + set/start calls can have an update in between. It sounds like this happens in your video, with how the 3D radio seems to create a small echo with the "local" radio. You would normally also need to consider doppler effects speeding up or slowing down some sound sources relative to others, but GZDoom disables doppler.
User avatar
Nash
 
 
Posts: 17434
Joined: Mon Oct 27, 2003 12:07 am
Location: Kuala Lumpur, Malaysia
Contact:

Re: Set start time to play sound in A_PlaySound

Post by Nash »

Chris wrote: If you specify a start time that's out of range of the sound being played, it'll be ignored and start from the beginning (it won't wrap).
Ah oops, forgot about that.
Chris wrote: MarkStartTime seems to ignore the start time (IIRC, that function's only used when a looping sound tries to start and fails, so when it tries to start again later it plays from roughly the correct point).
Right. Although if I actually mark the start time using the custom start time value, wouldn't this mean that an in-progress song would loop back to wherever position it was set to when the A_PlaySound is called? That means if I entered a level with a song that's played halfway, when it loops, it'll loop back to that halfway point, instead of the start of the song?
Chris wrote: You won't be able to exactly synchronize multiple sounds this way, given differences in the timing clocks and that get + set/start calls can have an update in between. It sounds like this happens in your video, with how the 3D radio seems to create a small echo with the "local" radio. You would normally also need to consider doppler effects speeding up or slowing down some sound sources relative to others, but GZDoom disables doppler.
While you are right with the synchronization issue, the echoing from the 3D radio and the local radio was actually done deliberately in the ZScript, because I noticed this is what happens in the Fallout games (whether intentionally or not :P).

Code: Select all

            // world radios play a little bit later
            double timeOffset = !isPlayerRadio ? 0.07 : 0.f;

            A_PlaySound(rc.currentTrack, CHAN_VOICE, RADIO_VOLUME_OFF, looping, attn,
                startTime: (rc.currentTrackPos % S_GetLength(rc.currentTrack)) + timeOffset);
I thought it was a good idea because if the local radio and 3D radio are perfectly in phase, the perceived volume would be louder, and it becomes hard to differentiate the 3D audio from the local one.

For my project's specific use-case, I can live without the perfect clock syncing, I don't know if this is acceptable if I wanted to turn my PR into something for mainline GZDoom though. And I also need to figure out what to do with the MarkStartTime issue... and lastly, of course, there's the modded sound issue, which I'm not sure if anything can be done about that TBH, or if it's something modders will just have to accept as a limitation. :shrug:
User avatar
Rachael
Posts: 13532
Joined: Tue Jan 13, 2004 1:31 pm
Preferred Pronouns: She/Her
Contact:

Re: Set start time to play sound in A_PlaySound

Post by Rachael »

This definitely seems to be something that should be included in GZDoom's mainline. And probably not just for the radios. :P
User avatar
Chris
Posts: 2940
Joined: Thu Jul 17, 2003 12:07 am
Graphics Processor: ATI/AMD with Vulkan/Metal Support

Re: Set start time to play sound in A_PlaySound

Post by Chris »

Nash wrote:
Chris wrote: MarkStartTime seems to ignore the start time (IIRC, that function's only used when a looping sound tries to start and fails, so when it tries to start again later it plays from roughly the correct point).
Right. Although if I actually mark the start time using the custom start time value, wouldn't this mean that an in-progress song would loop back to wherever position it was set to when the A_PlaySound is called? That means if I entered a level with a song that's played halfway, when it loops, it'll loop back to that halfway point, instead of the start of the song?
The MarkStartTime function stores the current time (the absolute system time it should've started) in the FISoundChannel struct, so when the same struct is later given to StartSound3D, it takes the difference between the now current time and the recorded time to tell how long it should've been playing, and set the offset accordingly. The easiest thing to do here would be to subtract the given startTime offset from the calculated system time that gets stored in the struct. Then when StartSound3D tries to restart the sound using the stored time, it inherently includes the starting offset.
Nash wrote: While you are right with the synchronization issue, the echoing from the 3D radio and the local radio was actually done deliberately in the ZScript, because I noticed this is what happens in the Fallout games (whether intentionally or not :P).

Code: Select all

            // world radios play a little bit later
            double timeOffset = !isPlayerRadio ? 0.07 : 0.f;

            A_PlaySound(rc.currentTrack, CHAN_VOICE, RADIO_VOLUME_OFF, looping, attn,
                startTime: (rc.currentTrackPos % S_GetLength(rc.currentTrack)) + timeOffset);
 
I thought it was a good idea because if the local radio and 3D radio are perfectly in phase, the perceived volume would be louder, and it becomes hard to differentiate the 3D audio from the local one.
True enough. I suppose realistically it would be unlikely two separate sources would be perfectly in sync anyway, even if they themselves are fed from the same signal. Different equipment would process radio signals slightly differently, one possibly with slightly more latency than the other, plus the difference in distance also affecting the exact time to each source is heard at. Enforcing a slight offset would help ensure they don't end up unrealistically synchronized, and make it more apparent that there's two sound sources.
User avatar
Nash
 
 
Posts: 17434
Joined: Mon Oct 27, 2003 12:07 am
Location: Kuala Lumpur, Malaysia
Contact:

Re: Set start time to play sound in A_PlaySound

Post by Nash »

Attempted to fix the out-of-range wrapping and MarkStartTime issues, this time on a clean fork of GZDoom (not on my game-specific branch):

https://github.com/nashmuhandes/gzdoom/ ... 934be9038e

Would this PR be considered for merging into mainline, or is it unacceptable still?
User avatar
Chris
Posts: 2940
Joined: Thu Jul 17, 2003 12:07 am
Graphics Processor: ATI/AMD with Vulkan/Metal Support

Re: Set start time to play sound in A_PlaySound

Post by Chris »

The calls to fmod() should take into account whether the sound is looping or not. A non-looping sound should clamp the offset, while a looping sound should wrap. The wrapping should also be aware of the sound's loop points. Alternatively, leave it to the caller to ensure the offset is appropriate for what it's doing (your script, for example, applies the wrap when calling A_PlaySound, ensuring it's in range; although timeOffset should be applied before the modulo). I'm not sure which would be preferable.

More stylistically, the way you apply the default parameter value for startTime in the sound classes don't look right. It should be applied to the base declaration, what the calling code will "see". It's pretty useless on the implementation definition since callers won't be aware of it when calling the function. I think some compilers will actually warn about this.

Other than that, as far as GZDoom itself is concerned I'm still not sure how to deal with the fact that sounds can be modded and their length/timing changed, throwing off any kind of scripted offset. At best it could just apply a relative offset to a previously retrieved one, like your radio example, but beyond that it starts falling apart. Other people who are more acquainted with modding these kinds of things would have to speak on how that is usually handled.
User avatar
Nash
 
 
Posts: 17434
Joined: Mon Oct 27, 2003 12:07 am
Location: Kuala Lumpur, Malaysia
Contact:

Re: Set start time to play sound in A_PlaySound

Post by Nash »

Hmmm, after some thinking, in my opinion, the wrapping should be handled by the scripter. Doesn't seem like the engine's business to be dealing with these problems. You can get a sound's length in ZScript with S_GetLength, the user has everything they need to ensure their scripts behave well.
More stylistically, the way you apply the default parameter value for startTime in the sound classes don't look right. It should be applied to the base declaration, what the calling code will "see". It's pretty useless on the implementation definition since callers won't be aware of it when calling the function. I think some compilers will actually warn about this.
I'm lost here. I don't really know what I'm doing here, if I'm being completely honest. :lol: Can you be very specific? I don't know which one is the base function. Also the fact that these sound functions are overloaded all over the place made it even more confusing for someone inexperienced like me.
Other than that, as far as GZDoom itself is concerned I'm still not sure how to deal with the fact that sounds can be modded and their length/timing changed, throwing off any kind of scripted offset. At best it could just apply a relative offset to a previously retrieved one, like your radio example, but beyond that it starts falling apart. Other people who are more acquainted with modding these kinds of things would have to speak on how that is usually handled.
I think this should be handled by the modder. This isn't the only case where scripting can get data from assets - remember, you can get texture and sprite dimensions too, and this is all freely accessible in scripting. Those textures can be easily replaced and stuff starts to get unpredictable. Whatever will happen here won't be something new to the engine. In my opinion, I think it's fine to leave this issue as-is...
User avatar
Rachael
Posts: 13532
Joined: Tue Jan 13, 2004 1:31 pm
Preferred Pronouns: She/Her
Contact:

Re: Set start time to play sound in A_PlaySound

Post by Rachael »

Nash wrote:Hmmm, after some thinking, in my opinion, the wrapping should be handled by the scripter. Doesn't seem like the engine's business to be dealing with these problems. You can get a sound's length in ZScript with S_GetLength, the user has everything they need to ensure their scripts behave well.
In most cases I'd normally agree with you, but what happens if we implement an alternative sound backend to OpenAL? The results would then be unpredictable in this case, and history has taught us here that if someone discovers a hack doing things one way, they will abuse it, and then when the affected code gets changed or upgraded their hack breaks.

I'm not saying you're right or wrong, here, I am just throwing out a little caveat to consider.
User avatar
Nash
 
 
Posts: 17434
Joined: Mon Oct 27, 2003 12:07 am
Location: Kuala Lumpur, Malaysia
Contact:

Re: Set start time to play sound in A_PlaySound

Post by Nash »

Well okay, sure, but in this very specific context, all that's happening here is if the "time" given to A_PlaySound is beyond range, the sound simply plays at the beginning. I'm not sure that's harmful at all, or affects anything significant in the long run... at best, I'd just apply a clamp rather than deal with the wrapping. With the clamp, the sound position will at least be safely reset to 0.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: Set start time to play sound in A_PlaySound

Post by Graf Zahl »

Never EVER operate on unvalidated data. If you got out of range offsets there's only two options:

1) return an error
2) clamp the value to a reasonable range or revert to the default.

If there is even some remote chance that a bogus value produces side effects it needs to be dealt with, otherwise people WILL abuse it for whatever stupid reason they may come up with.
User avatar
Chris
Posts: 2940
Joined: Thu Jul 17, 2003 12:07 am
Graphics Processor: ATI/AMD with Vulkan/Metal Support

Re: Set start time to play sound in A_PlaySound

Post by Chris »

Rachael wrote:
Nash wrote:Hmmm, after some thinking, in my opinion, the wrapping should be handled by the scripter. Doesn't seem like the engine's business to be dealing with these problems. You can get a sound's length in ZScript with S_GetLength, the user has everything they need to ensure their scripts behave well.
In most cases I'd normally agree with you, but what happens if we implement an alternative sound backend to OpenAL? The results would then be unpredictable in this case
How so? If there's an alternative sound backend, it would need to be able to report the length of a sound it loaded, meaning scripts still have the same info it needs to wrap. If a new backend isn't able to report a sound's length, all scripts relying on S_GetLength would fail.
User avatar
Rachael
Posts: 13532
Joined: Tue Jan 13, 2004 1:31 pm
Preferred Pronouns: She/Her
Contact:

Re: Set start time to play sound in A_PlaySound

Post by Rachael »

Because there's no guarantee that the alternate backend would do the same thing as OpenAL would. It's like when we got Vulkan - things are just .. different ... and things are also handled differently, even if ultimately the end-user never sees it.

The same thing might happen if OpenAL went through, for instance, a major refactor where the code was rewritten from scratch. Just as an example.

Basically the point I am trying to drive home is - when coding, it is important to check inputs and make sure they match what is actually supported by the library, in order to avoid mishaps in the future. Not doing input validation leads to unpredictable results, and more often than not, issues.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: Set start time to play sound in A_PlaySound

Post by Graf Zahl »

... and if the unvalidated input leads to exploits of whatever kind, the shit really hits the fan.
"The caller should do it" is the worst thing to do here, especially when designing a scripting interface for public use.
User avatar
Nash
 
 
Posts: 17434
Joined: Mon Oct 27, 2003 12:07 am
Location: Kuala Lumpur, Malaysia
Contact:

Re: Set start time to play sound in A_PlaySound

Post by Nash »

Okay, point taken, this discussion should have never happened in the first place, you can see in my branch that I did not leave the value unchecked (it wraps arounds). :D I just had my doubts because I was given a suggestion that perhaps it should be left to the caller.

Anyway, can I get some help on this?
More stylistically, the way you apply the default parameter value for startTime in the sound classes don't look right. It should be applied to the base declaration, what the calling code will "see". It's pretty useless on the implementation definition since callers won't be aware of it when calling the function. I think some compilers will actually warn about this.
Post Reply

Return to “Closed Feature Suggestions [GZDoom]”