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

Segfault with wav file containing some initial music before spoken words #25

Closed
besmaller opened this issue Nov 17, 2017 · 25 comments
Closed
Labels

Comments

@besmaller
Copy link
Contributor

besmaller commented Nov 17, 2017

I have been working to have Rhubarb help me convert an audio podcast into lip sync animation data for a podcast animation project. These podcasts all start with about 10 seconds of audio, and although some run through Rhubarb ok, most end up with a crash. I was able to isolate the problem after reviewing some debug logs to the first part of the podcast, where the music is located before the speaking begins. If I remove the first 10 seconds from the ~10 minute podcast, Rhubard works fine, and the output works very well. So, I was able to create a small testcase by clipping out the first 20 seconds of audio into a wave file which exhibits the crash. I am using Rhubarb version 1.6.0-win32. Attached is a tarball with the wav file, and the log. I am running Rhubarb like this:

./tools/rhubarb-lip-sync-1.6.0-win32/rhubarb.exe podcast3_first20s.wav --logFile podcast3_first20s.log --logLevel Debug

rhubarb_first20_crash.tar.gz

@DanielSWolf
Copy link
Owner

Thanks for your detailed error report and the data you attached!

I'm having trouble reproducing the error. On my machine, it works just fine. I've tried it with a local Debug build, a local Release build, and I even downloaded the official 1.6.0 binary to make sure there weren't any subtle differences. Rhubarb ran to completion and printed results every time.

I looked at the log file you attached. The weird thing is that all the "problematic" stuff -- segmentation, speech recognition, and voice alignment -- seem to have worked on your system, too. That's where I would have suspected any error to happen. According to the log file, the error must have happened during animation, which is rather uncommon.

Since I can't reproduce the problem, we'll have to try and narrow it down. I've created a special build with excessive logging in the area of the code where the crash appears to happen. Here's what I'd like you to do:

  • Download rhubarb.zip and extract it into your Rhubarb directory, replacing the official .exe file
  • Run Rhubarb again with the same arguments, but specify "Trace" as log level instead of "Debug"
  • Let me know whether you can still reproduce the crash. If so, please send me the log file. I should then be able to pinpoint the problem to a single function.

@besmaller
Copy link
Contributor Author

besmaller commented Nov 17, 2017 via email

@besmaller
Copy link
Contributor Author

OK, I was able to get it to crash with the new version of rhubarb.exe you sent me, and trace logging enabled, but I needed to run with the full podcast. I've atached the log, but here is the end of it, which seems telling, it seems to be processing the same segment more than once, and I don't think I've seen that happen anywhere earlier in the log. Some thread handling issue?
lip_sync_3.log

end of log...

[2017-11-17 16:10:07] 1 [Debug] ##segment[5.07-7.81]: B C D
[2017-11-17 16:10:07] 1 [Trace] optimizeTiming -- end
[2017-11-17 16:10:07] 1 [Trace] animatePauses -- start
[2017-11-17 16:10:07] 1 [Trace] animatePauses -- end
[2017-11-17 16:10:07] 1 [Trace] insertTweens -- start
[2017-11-17 16:10:07] 1 [Trace] insertTweens -- end
[2017-11-17 16:10:07] 1 [Trace] convertToTargetShapeSet -- start
[2017-11-17 16:10:07] 1 [Trace] convertToTargetShapeSet -- end
[2017-11-17 16:10:07] 1 [Trace] animateRough -- start
[2017-11-17 16:10:07] 1 [Trace] animateRough -- end
[2017-11-17 16:10:07] 1 [Trace] optimizeTiming -- start
[2017-11-17 16:10:07] 1 [Debug] ##segment[5.07-7.81]: B C D
[2017-11-17 16:10:07] 1 [Trace] optimizeTiming -- end
[2017-11-17 16:10:07] 1 [Trace] animatePauses -- start
[2017-11-17 16:10:07] 1 [Trace] animatePauses -- end
[2017-11-17 16:10:07] 1 [Trace] insertTweens -- start
[2017-11-17 16:10:07] 1 [Trace] insertTweens -- end
[2017-11-17 16:10:07] 1 [Trace] convertToTargetShapeSet -- start
[2017-11-17 16:10:07] 1 [Trace] convertToTargetShapeSet -- end

@besmaller
Copy link
Contributor Author

besmaller commented Nov 18, 2017

I'm looping through all 20 hours of podcasts now (66 files). I have three that failed (including that one), if you want additional log data to review. (out of the 10 processed so far).

My machine, by the way, is not a 'typical' windows platform, in that it has 2 Xeon X5450 CPUs w/ 4 cores each. I don't know if that might be related. I can try to replicate on a different (i7) machine to see if I can get one to fail there as well.

@DanielSWolf
Copy link
Owner

I've looked at the new log file. I'm afraid the problem isn't as clear as you were hoping: It's perfectly normal for Rhubarb to process the same segment more than once because it's trying different versions in order to find the best one.

I'll analyze the code some more and get back to you. This might take a while, though; I don't have much time at the moment.

@besmaller
Copy link
Contributor Author

Thanks. I have confirmed the crashes occur on a different, i7 machine as well as my Dual-Xeon home machine, but they appear to be somewhat random...

@besmaller
Copy link
Contributor Author

besmaller commented Nov 18, 2017

When I review the Trace logs (I looked at a dozen or so failures, different machines, different wav files, the log always end at the same point:

[2017-11-18 15:24:27] 1 [Trace] convertToTargetShapeSet -- end
[Ends on this step if a crash]

[Proceeds to this step when not crashing]
[2017-11-18 15:24:27] 1 [Trace] animateRough -- start

You may have already noticed this, but I wanted to see if I could help. I'm trying to see if I can come up with a small .wav file replicate-able test for you.

@besmaller
Copy link
Contributor Author

This 10 second wav file seems to fail consistently. (10 times in a row, on two different machines)

podcast_t10_13.zip

Here's a failing trace log.
lip_sync_t10_p13_1.log

@DanielSWolf
Copy link
Owner

Thanks for the new audio file. Unfortunately, I still cannot reproduce any error.

I, too, noticed the spot where the problematic logs end. But I couldn't find anything in that area that would explain the segfault.

It will probably be some days until I find the time to really dig into that crash. I'll let you know as soon as I find something!

@besmaller
Copy link
Contributor Author

OK - bummer you can't replicate. I ran with --extendedShapes "" --logLevel Trace --logFile xxx.log, if that might matter. You might try running it multiple times in a loop, I think there might be some randomness, like how the threading turns out, that could be involved. I'm not a C++ expert by any means, but I'm trying to install Visual Studio w/ cmake to see if I can build locally and maybe do some additional debug on my end.

@DanielSWolf
Copy link
Owner

Sounds great! Let me know if you need any help.

@besmaller
Copy link
Contributor Author

Yes, I'm having trouble with Boost. Is there a particular version I need to install? I installed the latest, and pointed BOOST_ROOT to c:\local\boost_1_65_1, but it's telling me it can't find the libraries.

@DanielSWolf
Copy link
Owner

Rhubarb can work with a wide range of Boost versions. I'm personally using Boost 1.61.0, but later versions should be compatible. You certainly shouldn't get an error about missing libraries.

Make sure you didn't just download the source code, but also built the libraries for the exact version of Visual Studio you are using.

Also, there are about a thousand different configurations for building Boost, even for a given compiler: static vs dynamic library, shared vs static C++ runtime and so on. Every configuration has its own directory and suffix. So when you get an error telling you a certain library couldn't be found, compare the exact path and filename of that library to the libraries you do have installed. Maybe you didn't build the correct configuration.

@besmaller
Copy link
Contributor Author

Thanks - yes, I just realized it's looking for the 32-bit libraries, and I have the 64-bit versions. (I downloaded the pre-built windows libraries).

@besmaller
Copy link
Contributor Author

OK, I have been able to get Rubarb to crash in the debugger. I got this message:
image

@besmaller
Copy link
Contributor Author

It seems to be crashing on this line:

image

@besmaller
Copy link
Contributor Author

I'm not really experienced using this debugger, but let me know if I can send you further details.

@besmaller
Copy link
Contributor Author

besmaller commented Nov 19, 2017

Well, I know enough to be dangerous, so I thought I'd take a stab at this. I think this might be a fix for this issue, but I don't fully understand your classes or how next_combination works. but when I make this change, I am able to run the test case without an error. What do you think? (changed maxReplacementCount to an int vs a const int, and overwrote it if the size of possibleRuleChanges was less than the default (3))

image

@DanielSWolf
Copy link
Owner

Wow, great work! Now everything makes sense.

The purpose of this function is to fix segments of animation where the mouth remains static for an extended amount of time. In normal speech, this typically occurs in sentences like "He seized his keys" where multiple EE-vowels occur with certain consonants in between. Rhubarb then tries to replace some of these mouth shapes with similar mouth shapes to add more variety to the animation. And in normal speech, there are typically many possible changes (that's what possibleRuleChanges refers to).

In your case, the music is interpreted as speech, leading to erratic combinations of sounds being detected that don't have the same characteristics as the sounds of normal speech. And suddenly it's possible that there are only few (or no) possible changes, and we're performing an out-of-bounds read in currentRuleChanges.

As you discovered, I forgot to check the length of possibleRuleChanges. Your fix does exactly the right thing, although I would have worked it into the for loop, like this:

#include <algorithm>

...

	for (
		int replacementCount = 1;
		bestScenario.getStaticSegmentCount() > 0 && replacementCount <= std::min(possibleRuleChanges.size(), maxReplacementCount);
		++replacementCount
	) {

If you'd like to do the honors, I'd be happy to accept a pull request from you.

@besmaller
Copy link
Contributor Author

Sure. This will be my first github pull request though. Is there a branch name syntax you would like me to use for the branch related to this issue? (issue_25_fix or something like that?)

@DanielSWolf
Copy link
Owner

"bugfix/#25-segfault" would make a nice name. I'm looking forward to your PR! 😄

@besmaller
Copy link
Contributor Author

besmaller commented Nov 22, 2017

Do you need to set me up with permission to push a branch into this repo?

git push origin bugfix/#25-segfault
remote: Permission to DanielSWolf/rhubarb-lip-sync.git denied to besmaller.
fatal: unable to access 'https://github.com/DanielSWolf/rhubarb-lip-sync.git/': The requested URL returned error: 403

@besmaller
Copy link
Contributor Author

github newbie... I think I need to fork your repo to my account and make the pull request across forks. Working on that.

@besmaller
Copy link
Contributor Author

OK. I think I figured it out. Let me know if it looks good to you.

DanielSWolf added a commit that referenced this issue Nov 22, 2017
Bug fix for segfault in fixStaticSegmentRules
@DanielSWolf
Copy link
Owner

Thanks for all your help, @besmaller! I really appreciate the effort you put into this (and into converting the Thimbleweed Park podcast 😃 )!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants