Bug report: Access violation in ResetPivotSetAndConvertAnimation

[lasse.lindqvist]

Hello,

I use FBX SDK 2016.1.2 for importing FBX files into my software. Since my software does not have pivot support, I call ResetPivotSetAndConvertAnimation for the root node of the imported FbxScene.
This has worked fine for a good amount of imported files. However, now I got a file from a user which produces an access violation inside ResetPivotSetAndConvertAnimation.

Here's code for reproducing the problem and the file with this issue is attached inside a zip file. The attached file opens without problems into Maya.

#include <fbxsdk.h>

void main(void)
{
  auto fbxSdkManager = FbxManager::Create();
  auto ios = FbxIOSettings::Create(fbxSdkManager, IOSROOT);
  fbxSdkManager->SetIOSettings(ios);
  auto importer = FbxImporter::Create(fbxSdkManager,"");
  const char* file = "problem.fbx";
  importer->Initialize(file, -1, fbxSdkManager->GetIOSettings());
  auto fbxScene = FbxScene::Create(fbxSdkManager, "MyScene");
  importer->Import(fbxScene);
  auto rootNode = fbxScene->GetRootNode();
  auto framerate = static_cast<float>(FbxTime::GetFrameRate(fbxScene->GetGlobalSettings().GetTimeMode()));
  rootNode->ResetPivotSetAndConvertAnimation(framerate, false, true);
  importer->Destroy();
}

I did not find a better place to post bug report, so here it this.

It would be great to have this fixed in FBX SDK.

Thanks.

[laurimaeki]

Any news on this? We have tried upgrading FBX SDK to version 2017.0.1, but that didn't help.

[laurimaeki]

FBXSDK 2018 did not help. Any updates on this?

[regalir]

The problem is not in the FBX SDK. The file in question defines some animation keys at -48 h and some other animation keys at +200 frame. The thing is that ResetPivotSetAndConvertAnimation has to go through all the animation range and plot keys at every single frame. This, obviously, is going to take a lot of time and memory causing, at some point, the access violation.

The best suggestion I can give you is to check all the animation curves and remove the keys defined at -48 h before you call the ResetPivotSetAndConvertAnimation. The idea is to have a manageable animation range. I believe that you can also call the function on the objects that have the meaningful animation (and skip the objects that have the keys set at -48h)

[lasse.lindqvist]

Sorry, but I have to disagree with you on this. In my opinion it is indeed the problem of FBX SDK if it crashes on malformed input crashing any product using it.

The correct behavior would be to throw an exception that can be caught without corrupting the process memory. It is OK if the input file cannot be processed, but it is not OK to take the whole process down.

In our case we don't decide the input files, but it is done by our end users. We are creating a software product in which the user can choose any files to import. The user expects that our product either imports their files or refuses to import them. Crashing the software, however, is unacceptable.

I hope you can take a further look on this and try to make ResetPivotSetAndConvertAnimation to behave so that it can exit cleanly also in case of there is data it cannot process. 

[laurimaeki]

Thanks for the reply,

Is there a reason why you prefer us to do prefiltering for the 3rd party data to prevent crashes in your software instead of FBX SDK doing that?

In case you want to prevent doing any alterations for the input data I do understand that but then how do you continue from there on? Crash the software without information on the reason?

It would be a lot better if you could handle the problem without crashing FBX SDK and/or by providing us means to see the reason it crashes to be able to provide the error information to the user that has created the invalid data.

Basically you are now just telling us why your software is crashing and claiming that it's not your problem. It doesn't help us much.

BR,

Lauri Mäki

[regalir]

Maybe I did not explained myself clearly. The data is not malformed. A little bit weird to define keys at -48h  but this could be the original intent of the creator of the file!

When I said that the problem is not in the FBX SDK, I meant that the code is performing as expected with the data it receives. It does resample all the "48 hours" for the 40 animation curves. For example, on my machine with 16 Gb of ram, I was able to apply the ResetPivotSetAndConvertAnimation() without problems (well, it did take about 5 minutes to process but it got to the end of it!)

Crashing in the memory allocation functions when running out of memory, was a decision made long time ago because of all the platforms and compilers the SDK was built on not fully supporting exceptions. However, with the next version of the FBX SDK (2019.2, releasing soon) because it is now generated with more recent compilers that support exceptions in a consistent way,  it does throw the "bad alloc" exception.

The reason I suggested you pre-filter the files is because the FBX SDK cannot make any assumptions on either the validity of the data in the scene or what kind of processing is performed on it.  Some checks are performed during the load of the file (for data integrity) but, in this case, everything pass because the data is totally acceptable.

If your application does check the animation range (maybe even calculate an estimate of the required memory) before calling intensive operations such as the ResetPivotSetAndConvertAnimation then it may be able to better inform the user and, if necessary, prompt for actions to take.

Hope this clarifies my previous post!