Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix playback audio sync issues #1326

Merged
merged 1 commit into from Apr 5, 2020

Conversation

scribblemaniac
Copy link
Member

@scribblemaniac scribblemaniac commented Apr 5, 2020

This PR loads and keeps audio files in memory, and also fixes two small coding errors which were also causing audio delays. I am perhaps overly optimistic that this will completely fix #1179, #1227, #1298, and the many user complaints we've had everywhere else. This code still needs to be tested on Windows where the issue seems to be worst.

Assuming this works as I expect it to, the only remaining known issue with playback audio synchronization will occur when Pencil2D is not scrubbing at the target frame rate (either because it's set too high or the computer cannot process the frames fast enough).

These changes will hopefully completely fix the audio-sync issues
during in-app playback. The first change was to keep all the audio
in memory all the time, not exactly efficient, but eliminates the
issues associated with hard drive seeks etc. The second change was
rearranging when playbackmanager plays the sound. There was a 1
frame offset because it was playing the sound of the frame right
before switch to the next frame, and there was a 1 frame of silence
at the beginning of playback because playsound was not called until
the first scrub.
@scribblemaniac scribblemaniac added Bug Sound 🔹 Minor PR (only one reviewer required) labels Apr 5, 2020
@scribblemaniac scribblemaniac added this to the 0.6.5 milestone Apr 5, 2020
@MrStevns
Copy link
Member

MrStevns commented Apr 5, 2020

Exciting! I'll review this.

Copy link
Member

@MrStevns MrStevns left a comment

LGTM, nice job.

@MrStevns MrStevns merged commit 9785b5e into pencil2d:master Apr 5, 2020
@scribblemaniac
Copy link
Member Author

scribblemaniac commented Apr 8, 2020

A follow up on this PR: I have tested this on Windows and it does improve audio playback. Unfortunately, there are still two issues (both exclusive to Windows):

  1. There still a ~400-800 ms delay for me upon playing a clip for the first time. I have no idea what could be causing this because the resource manager clearly shows that the sound is already in memory.
  2. There is a subtle fade-in when playing a sound clip, whether from the start or from the middle of the clip.

@davidlamhauge
Copy link
Contributor

davidlamhauge commented Apr 9, 2020

Two things:

  1. How do you measure the delay to be 400-800 ms, or is it guesswork?
  2. I found out that your solution has the same flaw as my limited sound scrub feature.
    I have a scene with two sounds on the sound layer. One covers frame 8-57 and the other frame 17-122. If I start playback when on frame 1-16, it plays both sounds, but if I start playback from frame 17 and up, it just plays the last sound, and skips what remains of the first.
    Tested on Linux (Ubuntu 18.04) and Win 10.

@scribblemaniac
Copy link
Member Author

scribblemaniac commented Apr 9, 2020

  1. It was just an educated guess based on how long it appeared to be.
  2. The issue is that we use getKeyFrameWhichCovers in PlaybackManager, which returns at most one keyframe. This issue was "hidden" prior to this PR by mListOfActiveSoundFrames which acted as a cache so that if you were playing multiple sound and then paused, it would remember the sounds that were previously playing when starting to play again. However this was a bad solution because if you started playing at frame 17 and up (in your example), without first playing a frame in the range 8-16, it would not play the first sound. The simplest solution would be to iterate over all previous keyframes checking for ones which are active over the current keyframe. A more complicated solution may be necessary for performance reasons, depending on how often we need to do a full check like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🔹 Minor PR (only one reviewer required) Sound
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sound Layer Audio Delay
3 participants