Timing drift caused by BPM changes

[Jumprocks1]

Each BPM change chip (channel 08) causes all chips after it to drift by ~0.5ms. Normally this is negligible, but for charts with hundreds of BPM changes, it is a significant problem.

Here is an example chart that I put together to demonstrate the issue: issue.zip

• good.dtx has no BPM changes (just set to 137 BPM at the start). It stays in sync.
• bad.dtx is the same as good.dtx, except it has 8 BPM changes per bar (all set to 137 BPM, so no real change). By the end of chart, it is off by ~300ms.

This is caused by:

DTXmaniaNX/DTXMania/Code/Score,Song/CDTX.cs

Line 3776 in 6640a0f

chip.nPlaybackTimeMs = ms + ( (int) ( ( ( 0x271 * ( chip.nPlaybackPosition - n発声位置 ) ) * dbBarLength ) / bpm ) );

That line calculates the position (in ms) of each chip. Unfortunately it uses an int cast since it stores the time in CChip.nPlaybackTimeMs. In my example, each BPM chip is 48 ticks apart, so the gap in ms is calculated as:

$$\lfloor 625 * 48 / 137 \rfloor = \lfloor 218.978 \rfloor = 218$$

Normally, the error caused by this cast isn't a problem, but for BPM chips, the casted value gets stored in int ms and is then added to all other chips. This causes the error to accumulate for each BPM chip. In my example, each BPM chip introduces 0.978ms of error. Across 312 BPM chips, the total error ends up being ~305ms.

DTXmania AL has the exact same issue as far as I can tell.

Steps to recreate example charts

• bgm.ogg
  • In Audacity, Generate > Rhythm Track > 137 BPM, 40 bars
  • Export as ogg
• good.dtx
  • In DTXC, add the BGM and hit.ogg
  • Add BGM chip at start of bar 0
  • Set BPM to 137
  • Add 4 snare hits per bar (for 40 bars)
• bad.dtx
  • Copy good.dtx
  • Add 8 BPM changes per bar (all set to 137 BPM)

Recommended solution

The final solution is up to whoever submits the pull request, but my recommendation is to calculate the playback time using a double instead of an int. This calculation can then be cast to an int and stored in chip.nPlaybackTimeMs. This would change the line I referenced above to be:

double playbackTimeMs = ms + 0x271 * ( chip.nPlaybackPosition - n発声位置 ) * dbBarLength / bpm;
chip.nPlaybackTimeMs = (int)Math.Round(playbackTimeMs);

int ms = 0; would become double ms = 0; and whenever assigning to it, you would use playbackTimeMs instead of chip.nPlaybackTimeMs. For example, the case for EChannel.BPMEx would use ms = playbackTimeMs; instead of ms = chip.nPlaybackTimeMs;

Overall I think this would only require changing ~10 lines.

Edit:
It's worth noting that we may not even want to change this. This behavior is definitely unexpected and it doesn't line up with the behavior of external tools, but changing it now would affect the timing for older simfiles. It would also create another difference between NX and AL. Simfiles timed for NX would be slightly off in AL and vice-versa.

The main reason we would want to fix this is for compatibility with other programs. If you time a song in a DAW or a MIDI editor, you would expect that timing to convert perfectly to DTXmania, but right now that is not the case as each BPM change will cause timing drift.

[fisyher]

Thanks @Jumprocks1 for highlighting this issue.
After several more in-depth testing with a handful of real simfiles, I found that not only BPM chips but any change in barLength will also result in the same time drift issue. I have created a sample chart based on the one prepared by @Jumprocks1 , but with alternating 0.5 and 1.5 bar lengths, while putting the same number of snares:
bad2_withBarLengthChangesOnly.zip

The time drift adds up to 0.04 seconds or 40 ms, with only bar length changes every bar, which is around 1 ms per change. Although the drift is quite small in this example, it will certainly add up to significant amount for very long songs with 100 or more bar length/BPM changes.

The following table shows the Song Length shown in the Debug Info during gameplay for both Original and the Improved Compute methods (updated to 3 decimal places in seconds):

Chart	Length in sec (Original Compute Method)	Length in sec (Improved Compute Method)	Drift in sec	Remarks
good.dtx	71.824	71.825	0.001
bad.dtx	71.519	71.825	0.306	312 BPM chips of same BPM of 137
bad2.dtx	71.792	71.825	0.033	39 bar length changes
Obsidian	110.620	110.638	0.018	Some BPM and bar length changes
Long version of Konran shoujo Soflan-chan!! by @approvedtx	542.201	542.334	0.133	9 mins song with numerous BPM and bar length changes, timed using mainly DTXCreator and DTXV
Hartmann Youkai Girl by @fisyher	262.197	262.227	0.03	No BPM changes, only bar length changes
Model DD Ultimates	169.769	169.777	0.008	Some BPM changes and bar length changes

In practical terms, only Soflan-chan Long Version has audible desync near the last quarter of song when switched to using the Improved Compute Method. This is because its BPM and Bar Lengths were manually sync and adjusted using DTXCreator with audio cues from the DTXViewer (@approvedtx please correct me if I am wrong). For songs with well-known BPMs and Time-Signatures, the Improved Computed Method will resolve the de-sync issue and compatibility issue with external tools.

Solution

As mentioned by @Jumprocks1, fixing this issue will cause incompatibility with other DTXMania variants such AL since they all have the same issue and cause existing BPM/Time-signature varying charts timed using DTXViewer to not play accurately. As such, I have to decided to add a new config field in config.ini tentatively named ChipPlayTime Compute Mode, implement both Original and Improved methods of computing playbackTimeMs, and let players choose which mode to use in the Config Option menu.

The implementation can be found in the misc_issues branch. Feedbacks and comments are welcomed.

[Jumprocks1]

Thank you for looking at this @fisyher. I reviewed the changes for CDTX.cs in misc_issues and the implementation looks correct as far as I can tell.

The measurements in your table also look correct. I would however recommend including another decimal place when discussing these changes, as the the difference between 10ms and 1ms is pretty large for audio. As an example, the default judgement window for a perfect is 34ms, so a 10ms drift reduces this window by a noticeable 29%. A 40ms drift completely destroys the window. For most basic simfiles though, the drift is closer to 1ms.

One thing you did not mention in your comment is that the default is set to the new compute mode. I have no opinion on what the default should be, just mentioning that for anyone else who is curious.

[fisyher]

Thanks @Jumprocks1 for the feedback.

I forgot to highlight that the measurements collected are limited to what the existing Debug Info on the UI, which always has the sound length round off to 2 decimal point in seconds. Therefore, the actual drift may not be as high as shown in the table. I will probably update Debug Info to show a few more decimals and update the comparison table to make it more accurate.

As for the default value being set to the new compute mode, it is chosen as a way to encourage players & simfile authors to try out the fix as I believe it should be the way moving forward to improve authoring experience with external audio tools. Opting out of it is made easy as it is placed as the last configurable option in under System: