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

alembicReader.cpp uses fixed frame rate of 24 FPS #940

Closed
sybrenstuvel opened this issue Aug 16, 2019 · 13 comments
Closed

alembicReader.cpp uses fixed frame rate of 24 FPS #940

sybrenstuvel opened this issue Aug 16, 2019 · 13 comments

Comments

@sybrenstuvel
Copy link

Description of Issue

When loading an Alembic file via the Alembic-USD plugin it is assumed to be 24 FPS.

This can be seen in alembicReader.cpp lines 898 and 961. The Alembic file contains timing information, and this should be used instead of hard-coding the frame rate.

Note that this limitation is not documented in the Known Limitations section of the Alembic-USD plugin documentation.

Steps to Reproduce

Here is a zipped Alembic file: bug_rotation_scale_alembic.zip. This file contains 20 frames at a frame rate of 30 FPS. Use abcls -t bug_rotation_scale_alembic.abc to show the timing information. There are two time sample series in there, one for geometry (there is one sample, as the geometry itself is not animated), and one for transforms (20 samples @ 30 FPS):

% abcls -t bug_rotation_scale_alembic.abc 

Time Samplings: 
0 Uniform Sampling. Start time: 0 Time per cycle: 1
Max Num Samples: 1
1 Uniform Sampling. Start time: 0.0333333 Time per cycle: 0.0333333
Max Num Samples: 20

Reproduction method 1

  1. Run usdview bug_rotation_scale_alembic.abc
  2. Play the animation
  3. See the disc change scale. I suspect this happens because of incorrect linear per-component interpolating of transformation matrices.

Reproduction method 2

  1. Run usdcat bug_rotation_scale_alembic.abc
  2. See that it contains samples for frames 0.8 to 16, which is a factor of 24/30 off.

Also, usdcat mentions endTimeCode = 456 in the USD metadata, even though the Alembic file only contains 20 frames. I'm not sure whether this is caused by the same bug or not. What I do know is that this number increases when more than one time sample per frame is written to Alembic.

System Information (OS, Hardware)

Linux, Kubuntu 19.04, Intel Core i7 64-bit.

Package Versions

USD 19.07, self-built from the Git repository v19.07 tag.

Build Flags

Built using:

python2 build_scripts/build_usd.py /opt/usd-with-python --generator Ninja --alembic

@spiffmon
Copy link
Member

spiffmon commented Aug 16, 2019 via email

@sybrenstuvel
Copy link
Author

Hi Sybren! This behavior is not documented as a abcReader limitation because it is an Alembic limitation. I confirmed awhile back on the Alembic forum that Alembic has no official means to encode an overall samples/timeCodes/frames per-second.

Sure, Alembic can store samples at arbitrary timecodes. This is not a compelling reason to resort to hard-coding 24 FPS, though. With a scene in a fixed frame rate, there is a clear mapping from frame number to timecode, and Alembic provides a well-defined way to obtain samples given that timecode. We do this all the time in Blender, and we also support sub-frame sampling. I don't see why this can't be done with USD.

how is abcReader supposed to figure out which of the three timeSamplings it should use to recover timeCodesPerSecond?

All the geometry schemas in Alembic have a getTimeSampling() function that you can use to determine the time sampling.

Maybe the heuristic you’re relying on covers a high percentage of cases?

We're not relying on any heuristic, we're just using those getTimeSampling() functions. See the get_matrix() function in Blender's Alembic code for an example.

@spiffmon
Copy link
Member

spiffmon commented Aug 16, 2019 via email

@jilliene
Copy link

Filed as internal issue #USD-5497

@sybrenstuvel
Copy link
Author

sybrenstuvel commented Aug 20, 2019

I think there are a few things going on at the same time:

  1. USD files declare a fixed frame rate and ABC files do not, so when converting ABC → USD this needs to be guestimated. I understand that this is hard given the difference in time encoding in these formats.
  2. The Alembic-USD plugin assumes the ABC file is a hard-coded 24 FPS. This is not documented in the "Known Limitations" of the Alembic-USD plugin because it is considered to be a known limitation of Alembic (and not the plugin). However, the hard-coding happens in that plugin, so it's a limitation of that plugin. Alembic files are by no means limited to this frame rate.
  3. Because of the assumption that the file is 24 FPS, samples in ABC don't align with frames in USD. This in turn requires interpolation between samples, which, for transformation matrices, is done with component-wise LERP. It is well known that LERPing such matrices produces erroneous results.
  4. When converting an ABC file with inter-frame samples to USD, the reported endTimeCode in the USD file is incorrect. Regardless of the above, I think you'll agree that this is a bug. After all, even when the last frame is written as 16 (a factor of 24/30 off), the endTimeCode should be 16.

So should we interpret that as 60fps with samples on 1’s, or as 30fps with samples on 0.5’s?

I think both are an improvement over the hard-coded 24 FPS, as either interpretation will have a sample in ABC exactly on a frame in USD. This works around point 3 from the list above. It also improves playback performance as no interpolation needs to be performed.

The root cause point 1 above is, I suspect, that Alembic files refer to scene time, whereas the conversion to USD happens without any reference to any existing scene. Maybe the USD and Alembic developers+community can agree on some metadata fields in Alembic that expose the scene frame rate at time of exporting?

@spiffmon
Copy link
Member

spiffmon commented Aug 20, 2019 via email

@sybrenstuvel
Copy link
Author

but would you be willing to pose your closing question to the alembic google group, referencing this issue?

Done: https://groups.google.com/forum/#!topic/alembic-discussion/f0P3bKNe_qQ

@Narann
Copy link

Narann commented Aug 20, 2019

I realize all I said in my answer has already been discussed here, sorry for this. 🙏

@spiffmon
Copy link
Member

spiffmon commented Oct 5, 2019

Hi @sybrenstuvel ,
It doesn't seem like the query on the Alembic forum went anywhere? I'd like to suggest an alternative, which is to start using the timeCodesPerSecond metadata that our Alembic reader/writer already uses to be able to roundtrip USD files through Alembic. The Alembic files Blender produces will then just have this extra metadata that only Blender and USD will understand, but that may be a pretty good starting point.

Cheers,
@spiffmon

@sybrenstuvel
Copy link
Author

@spiffmon It actually was implemented and went into the 1.7.12 release, it just wasn't communicated to the Github issue tracker. I'm now modifying Blender to link against the new Alembic release and write this new metadata field.

@spiffmon
Copy link
Member

I saw it in the release notes, @sybrenstuvel ! I created Issue #1023 with details on how usdAbc might take advantage of it. We’d be thrilled if you were able to tackle that project.

@boberfly
Copy link
Contributor

Hi, I think this issue can now be closed as of #2944 right? I was shocked to run into this but noticed it has been fixed.

@spiffmon
Copy link
Member

Indeed - thanks, @boberfly !

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

No branches or pull requests

5 participants