SkeletonUtils retargetClip is always off by 1 frame.

[timbotimbo]

[UPDATE] See my new comment below for an improved example.

Description

Any AnimationClip I use in the SkeletonUtils retargetClip function will result in an output clip with a shorter duration, that is missing exactly 1 frame. The difference is hardly visible in animations with hundreds or thousands of frames, but it is still incorrect.

The issue seems to come from the numFrames calculation. numFrames = Math.round(clip.duration * ( options.fps / 1000 ) * 1000 )

Theoretical example

Lets take a clip with a duration of 3 seconds and 1 fps as example.

• Input: frame times: 0 1 2 3, duration: 3 seconds

numFrames is calculated as 3, which is used in a < numFrames for-loop.

• Output: frames times: 0 1 2, duration: 2 seconds

Solutions

• If we can assume that any AnimationClip input will start at time 0, we can just add a + 1 to the numFrames definition. (This would get an index out of range for a clip with times 1 2 3, but i'm not sure that is possible)

• An alternative is to use numFrames = clip.tracks[0].times.length The function already seems to assume all tracks are the same length.

Testing

There don't seem to be any examples or tests that use this function.

Reproduction steps

1. Load an animationClip and create a skeletonhelper.
2. Make an options object that includes an fps property that matches the framerate of the animation.
3. Call retargetClip using the variables above as input.
4. The output of retargetClip does not match the duration of the loaded clip.

Code

  // Any use of the retargetClip function, where the options fps is set.
  // This excerpt is from within the scope of a bvhloader callback, see the live example

  let options = {
    fps: 1 / result.clip.tracks[0].times[1]
  };

  console.log("original duration: " + result.clip.duration);
  console.log("original track0 frames: " + result.clip.tracks[0].times.length);

  let newClip = retargetClip(skeletonHelper, skeletonHelper, result.clip, options);

  console.log("retarget duration: " + newClip.duration);
  console.log("retarget track0 frames: " + newClip.tracks[0].times.length);

Live example

This is basically the bvhLoader example pasted in jsfiddle, with the above code block pasted into the loader callback. The use of the retarget does not make sense here, as it doesn't load any other model, but this is enough to show the bug.

https://jsfiddle.net/sn58dp40/

Expected console output for the example:

"original duration: 4.924997806549072"
"original track0 frames: 592"
"retarget duration: 4.9166646003723145"
"retarget track0 frames: 591"

Version

r148, dev

Device

Any

Browser

Any

OS

Any

[kate-grant]

Looking at the retarget function, I think the issue is related to keyframe generation, specifically the last keyframe. When the converted tracks are pushed, they currently push without the final keyframe. This is because the times and positions are assigned at every numFrame, but numFrame does not account for a final keyframe with no interpolation afterwards, only keyframes followed by interpolation.

A solution could be: After the main loop, to add the final keyframe calculated a the exact duration timestamp.

I would love to work on a PR for this if it is open to work on.

[timbotimbo]

The fix in #25589 doesn't seem to work as intended. The final frame seems fuly broken now with the model seemingly exploded into a random shape.

I've got a new live example that shows some actual retargeting. This applies the bvh loader pirouette.bvh to the the fbx loader Samba Dancing.fbx.

Reproduction

1. Retarget an animationClip on a model. For simplicity, just run the jsfiddle (or source code) below.
2. You should see the Ybot model dancing a pirouette.
3. Make sure the skeletonUtils import uses r150: "three/addons/": "https://unpkg.com/three@0.150.1/examples/jsm/" Now the animation looks good, but the retageted animation is 1 frame short, as in the original issue above.
4. Now make sure the import uses r151: "three/addons/": "https://unpkg.com/three@0.151.0/examples/jsm/" Now the frame count is correct, but there will sometimes be a weird flash or glitch in the loop of the animation.
5. Uncomment (or add) the following to pause the animation at the last frame.

   // pause on the last frame
   mixer.setTime(newClip.duration*0.99999);
   mixer.timeScale = 0;

6. Check this pausing on both r150 and r151. On r151 you should see something very glitchy.


Example

Live example jsFiddle

jsfiddle source code as backup

```html ``` ```javascript import * as THREE from 'three'; import { OrbitControls } from 'three/addons/controls/OrbitControls.js'; import { BVHLoader } from 'three/addons/loaders/BVHLoader.js'; import { FBXLoader } from 'three/addons/loaders/FBXLoader.js'; import { retargetClip } from 'three/addons/utils/SkeletonUtils.js'; const clock = new THREE.Clock(); let camera, controls, scene, renderer; let mixer, skeletonHelper; let model; let options = { hip: "hip", // bone mapping FBX : BVH names: { "mixamorigHips": "hip", "mixamorigSpine": "abdomen", "mixamorigSpine1": "Chest2", "mixamorigSpine2": "Chest3", "mixamorigNeck": "neck", "mixamorigHead": "head", // "mixamorigHeadTop_End": "", "mixamorigRightShoulder": "rCollar", "mixamorigRightArm": "rShldr", "mixamorigRightForeArm": "rForeArm", "mixamorigRightHand": "rHand", "mixamorigRightHandThumb1": "rThumb1", "mixamorigRightHandThumb2": "rThumb2", // "mixamorigRightHandThumb3": "", "mixamorigRightHandIndex1": "rIndex1", "mixamorigRightHandIndex2": "rIndex2", // "mixamorigRightHandIndex3": "", "mixamorigRightHandMiddle1": "rMid1", "mixamorigRightHandMiddle2": "rMid2", // "mixamorigRightHandMiddle3": "", "mixamorigRightHandRing1": "rRing1", "mixamorigRightHandRing2": "rRing2", // "mixamorigRightHandRing3": "", "mixamorigRightHandPinky1": "rPinky1", "mixamorigRightHandPinky2": "rPinky2", // "mixamorigRightHandPinky3": "", "mixamorigLeftShoulder": "lCollar", "mixamorigLeftArm": "lShldr", "mixamorigLeftForeArm": "lForeArm", "mixamorigLeftHand": "lHand", "mixamorigLeftHandThumb1": "lThumb1", "mixamorigLeftHandThumb2": "lThumb2", // "mixamorigLeftHandThumb3": "", "mixamorigLeftHandIndex1": "lIndex1", "mixamorigLeftHandIndex2": "lIndex2", // "mixamorigLeftHandIndex3": "", "mixamorigLeftHandMiddle1": "lMid1", "mixamorigLeftHandMiddle2": "lMid2", // "mixamorigLeftHandMiddle3": "", "mixamorigLeftHandRing1": "lRing1", "mixamorigLeftHandRing2": "lRing2", // "mixamorigLeftHandRing3": "", "mixamorigLeftHandPinky1": "lPinky1", "mixamorigLeftHandPinky2": "lPinky2", // "mixamorigLeftHandPinky3": "", "mixamorigRightUpLeg": "rThigh", "mixamorigRightLeg": "rShin", "mixamorigRightFoot": "rFoot", // "mixamorigRightToeBase": "", "mixamorigLeftUpLeg": "lThigh", "mixamorigLeftLeg": "lShin", "mixamorigLeftFoot": "lFoot", // "mixamorigLeftToeBase": "", } }; init(); animate(); const fbxLoader = new FBXLoader(); fbxLoader.load('https://rawcdn.githack.com/mrdoob/three.js/996f6c9f5f606d3193b3c5545b88de7eb09cbb55/examples/models/fbx/Samba%20Dancing.fbx', function (fbxImport) { model = fbxImport; model.traverse(function (child) { if (child.isMesh) { child.castShadow = true; child.receiveShadow = true; child.frustumCulled = false } }); if (!model.skeleton) { model.traverse((child) => { if (child.skeleton) { model.skeleton = child.skeleton; } }); } mixer = new THREE.AnimationMixer(model); scene.add(model); const loader = new BVHLoader(); loader.load('https://rawcdn.githack.com/mrdoob/three.js/996f6c9f5f606d3193b3c5545b88de7eb09cbb55/examples/models/bvh/pirouette.bvh', function (result) { skeletonHelper = new THREE.SkeletonHelper(result.skeleton.bones[0]); skeletonHelper.skeleton = result.skeleton; // allow animation mixer to bind to THREE.SkeletonHelper directly options.fps = 1 / result.clip.tracks[0].times[1]; console.log("original duration: " + result.clip.duration); console.log("original track0 frames: " + result.clip.tracks[0].times.length); let newClip = retargetClip(model, skeletonHelper, result.clip, options); console.log("retarget duration: " + newClip.duration); console.log("retarget track0 frames: " + newClip.tracks[0].times.length); let action = mixer.clipAction(newClip); action.play(); // pause on the last frame // mixer.setTime(newClip.duration*0.99999); // mixer.timeScale = 0; }); }); function init() { camera = new THREE.PerspectiveCamera(60, window.innerWidth / window.innerHeight, 1, 1000); camera.position.set(0, 200, 300); scene = new THREE.Scene(); scene.background = new THREE.Color(0xa0a0a0); scene.fog = new THREE.Fog(0xa0a0a0, 200, 1000); const hemiLight = new THREE.HemisphereLight(0xffffff, 0x444444, 1.5); hemiLight.position.set(0, 200, 0); scene.add(hemiLight); const dirLight = new THREE.DirectionalLight(0xffffff, 1.5); dirLight.position.set(0, 200, 100); dirLight.castShadow = true; dirLight.shadow.camera.top = 180; dirLight.shadow.camera.bottom = - 100; dirLight.shadow.camera.left = - 120; dirLight.shadow.camera.right = 120; scene.add(dirLight); // ground const mesh = new THREE.Mesh(new THREE.PlaneGeometry(2000, 2000), new THREE.MeshPhongMaterial({ color: 0x999999, depthWrite: false })); mesh.rotation.x = - Math.PI / 2; mesh.receiveShadow = true; scene.add(mesh); const grid = new THREE.GridHelper(2000, 20, 0x000000, 0x000000); grid.material.opacity = 0.2; grid.material.transparent = true; scene.add(grid); // renderer renderer = new THREE.WebGLRenderer({ antialias: true }); renderer.setPixelRatio(window.devicePixelRatio); renderer.setSize(window.innerWidth, window.innerHeight); document.body.appendChild(renderer.domElement); controls = new OrbitControls(camera, renderer.domElement); controls.minDistance = 300; controls.maxDistance = 700; window.addEventListener('resize', onWindowResize); } function onWindowResize() { camera.aspect = window.innerWidth / window.innerHeight; camera.updateProjectionMatrix(); renderer.setSize(window.innerWidth, window.innerHeight); } function animate() { requestAnimationFrame(animate); const delta = clock.getDelta(); if (mixer) mixer.update(delta); renderer.render(scene, camera); } ```

[AlaricBaraou]

@timbotimbo this new PR should fix your issue.

But in your example you will still have to change the way you get the fps from options.fps = 1 / result.clip.tracks[0].times[1]; to options.fps = result.clip.tracks[ 0 ].times.length / result.clip.duration;

Otherwise you're getting 120.000047 fps instead of 120.20309 fps this small difference is what's causing the issue in the amount of frames being short by 1.

But your issue highlighted other issues while debugging that should be adressed by that PR #27653

[timbotimbo]

Thanks for picking this up.

But in your example you will still have to change the way you get the fps from options.fps = 1 / result.clip.tracks[0].times[1]; to options.fps = result.clip.tracks[ 0 ].times.length / result.clip.duration;

Maybe to to prevent errors like these the function should default to the fps of the clip. Right now it defaults to 30 and you need to explicitly define the correct value if your clip doesn't match 30. I would expect it to take the clip as default and only change it if you explicitly define a different fps.

[AlaricBaraou]

Good point, it would make sense to default to the original FPS. If that's not considered as a breaking change I can add it to the PR.