Set start time to play sound in A_PlaySound

Remember, just because you request it, that doesn't mean you'll get it.

Moderator: GZDoom Developers

Re: Set start time to play sound in A_PlaySound

Postby Nash » Sat Oct 12, 2019 8:35 am

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
Nash
 
 
 
Joined: 27 Oct 2003
Location: Kuala Lumpur, Malaysia
Github ID: nashmuhandes

Re: Set start time to play sound in A_PlaySound

Postby Chris » Sat Oct 12, 2019 1:10 pm

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
Chris
 
Joined: 17 Jul 2003

Re: Set start time to play sound in A_PlaySound

Postby Nash » Sat Oct 12, 2019 2:22 pm

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 allExpand view

            
// 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
Nash
 
 
 
Joined: 27 Oct 2003
Location: Kuala Lumpur, Malaysia
Github ID: nashmuhandes

Re: Set start time to play sound in A_PlaySound

Postby Rachael » Sat Oct 12, 2019 3:08 pm

This definitely seems to be something that should be included in GZDoom's mainline. And probably not just for the radios. :P
User avatar
Rachael
Webmaster
 
Joined: 13 Jan 2004
Discord: Rachael#3767
Twitch ID: madamerachelle
Github ID: madame-rachelle
Graphics Processor: nVidia with Vulkan support

Re: Set start time to play sound in A_PlaySound

Postby Chris » Sat Oct 12, 2019 4:22 pm

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 allExpand view

            
// 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
Chris
 
Joined: 17 Jul 2003

Re: Set start time to play sound in A_PlaySound

Postby Nash » Sun Oct 13, 2019 5:19 am

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
Nash
 
 
 
Joined: 27 Oct 2003
Location: Kuala Lumpur, Malaysia
Github ID: nashmuhandes

Re: Set start time to play sound in A_PlaySound

Postby Chris » Sun Oct 13, 2019 7:54 am

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
Chris
 
Joined: 17 Jul 2003

Re: Set start time to play sound in A_PlaySound

Postby Nash » Mon Oct 14, 2019 7:26 am

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
Nash
 
 
 
Joined: 27 Oct 2003
Location: Kuala Lumpur, Malaysia
Github ID: nashmuhandes

Re: Set start time to play sound in A_PlaySound

Postby Rachael » Mon Oct 14, 2019 7:29 am

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
Rachael
Webmaster
 
Joined: 13 Jan 2004
Discord: Rachael#3767
Twitch ID: madamerachelle
Github ID: madame-rachelle
Graphics Processor: nVidia with Vulkan support

Re: Set start time to play sound in A_PlaySound

Postby Nash » Mon Oct 14, 2019 7:31 am

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
Nash
 
 
 
Joined: 27 Oct 2003
Location: Kuala Lumpur, Malaysia
Github ID: nashmuhandes

Re: Set start time to play sound in A_PlaySound

Postby Graf Zahl » Mon Oct 14, 2019 9:02 am

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
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: Set start time to play sound in A_PlaySound

Postby Chris » Mon Oct 14, 2019 4:00 pm

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
Chris
 
Joined: 17 Jul 2003

Re: Set start time to play sound in A_PlaySound

Postby Rachael » Mon Oct 14, 2019 4:42 pm

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
Rachael
Webmaster
 
Joined: 13 Jan 2004
Discord: Rachael#3767
Twitch ID: madamerachelle
Github ID: madame-rachelle
Graphics Processor: nVidia with Vulkan support

Re: Set start time to play sound in A_PlaySound

Postby Graf Zahl » Mon Oct 14, 2019 5:05 pm

... 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
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: Set start time to play sound in A_PlaySound

Postby Nash » Mon Oct 14, 2019 6:12 pm

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.
User avatar
Nash
 
 
 
Joined: 27 Oct 2003
Location: Kuala Lumpur, Malaysia
Github ID: nashmuhandes

PreviousNext

Return to Feature Suggestions

Who is online

Users browsing this forum: Awario [RSS] and 0 guests