Time-index conversion isues

[ferdnyc]

This is a work-in-progress first-attempt PR, which I believe is complete as to the changes needed for the Marker/Keyframe positioning issue, but is certainly incomplete in terms of addressing all timing/frame-sync issues. (See below for considerable further discussion/developments, but long-and-short I think this fixes the problem of Markers being positioned off by 1 frame, and Keyframe tickmarks being drawn one frame off from where they actually are.)

I'm posting where I'm at so that others might have a chance to test the changes so far, confirm that things seem to be moving in the right direction, and identify remaining issues stemming from time-index off-by-one errors.

The issues that have been reported with marker/keyframe positioning, clip boundaries, and etc. seem to mostly be manifestations of two issues:

Conversion between frame# and time index is performed inconsistently in the code.

Because time-indexes originate at 0 seconds, but frames are counted from 1, every conversion from time-based position to linear (counted) position, in either direction, has to account for this. And the code contains dozens of separate places where this conversion is performed, ad hoc.

1. In the python code:
   • When going forwards,

     # If frames-per-second is expressed as the fraction
     fps_float = float(fps["num"]) / float(fps["den"])
     # Anytime a time index is converted to a frame#, the formula is:
     frame = (time_pos * fps_float) + 1

     ...this appears to be done consistently, so time indexes are always converted to the correct frame.
   • But when going backwards,

     # Anytime a frame# is converted to a time index, the formula is reversed:
     time_pos = (frame - 1 / fps_float)

     ...multiple locations were failing to subtract 1 from the frame number, causing time indexes to be computed and stored one frame too high (once they were properly converted back to frame numbers).
2. In the Timeline HTML:
   • Several places compute the location of display elements in terms of pixelsPerSecond, after computing that based on the zoom level. When the math is being done from a time index, or uses a time duration, frames-per-second is uninvolved and you get math such as:

     <div style="width:{{(clip.end - clip.start) * pixelsPerSecond}}px;
      left:{{clip.position * pixelsPerSecond}}px;">

   • But when the position is frame-based, as it is with keyframe parameters, it's instead e.g.:

     <img style="position: absolute;
      left: {{(((point - 1) /
                (project.fps.num / project.fps.den) - clip.start)
               * pixelsPerSecond) - keyframePointOffset}}px;"
      class="point_icon" ng-src="media/images/keyframes/point.png"/>

     ...again, the necessary subtraction (point - 1) was not being performed.

This PR thus contains, so far, one commit which patches two files, attempting to ensure that the code will "Always compute time as (frame# - 1) / fps".

Interpolated parameters store keyframe positions as frame numbers

This issue is the source of much of the above complexity. While it's in some senses easier to store raw frame numbers, when positioning timeline elements, the problem is that frame number is dependent on the frame rate of the video, so altering the frames-per-second value changes the position of every frame except the first. Most users working with video want frame accuracy in operations, but they want positioning to be independent of frame rate so that, if they place a timeline element at the 1-second mark, that's automatically adjusted to frame 25 @ 24fps, frame 30 @ 29.97 fps, frame 31 @ 30fps, and frame 61 @ 60fps.

This PR does not address, or even attempt to address, this more-fundamental issue. (See bug #1034, and others, for reports of problems related to frame-based positioning.)

Other, possibly-related, issues

There are other time-indexing issues still present in the code.

1. @peanutbutterandcrackers reported Split Clip: Off by a frame #1369 where the Split Clip tool seems to be off by a frame when bounding clips created from project files.
2. Using the "Slice: Keep both sides" timeline tool doubles a frame. (I'm not sure whether this issue has been reported, as such.) The last frame in the left clip will be the same frame as the first frame in the right clip.
   This is because the Start parameter of the right clip is set to the same time-index as the left clip's End parameter, when right.Start should be left.End + (1 / fps) (or both left.End and left.Duration should be decreased by 1/fps). My changes here don't address this, though with the keyframe-positioning updates I believe they may make the incorrect slice positioning more obvious.

[DylanC]

@ferdnyc - This is really great thanks!

[ferdnyc]

Using the "Slice: Keep both sides" timeline tool doubles a frame. (I'm not sure whether this issue has been reported, as such.) The last frame in the left clip will be the same frame as the first frame in the right clip.

After some further experimentation, I now believe I was wrong about this, at least for the "ideal" case where the frame-rate of the clip matches the project profile frame-rate, so that all operations are being done on a clip frame boundary.

For the case where framerates don't match, it does seem that slicing can at times double a frame, but I'm not sure that's really a completely solvable problem. Frame-rate mismatches are a tricky thing to deal with at the best of times.

This applies only to the Slice tool's "keep both sides" function, not to the Split Clip tool frame-shift issues that @peanutbutterandcrackers reported in #1369. I'm still seeing that — in my testing, the resulting clip both starts and ends one frame before the frames chosen.

[ferdnyc]

Oof, hokay. This is a tricky one. Yes, OpenShot can double a frame on slicing, even if the frame rates match between the clip and the project. It can also drop a frame. And the Split Clip tool will sometimes start with the frame before the selected one, when added to the timeline.

But OpenShot isn't doing anything wrong, math-wise. The issue seems to have something to do with certain unpredictabilities in the way frame boundaries work, when decoding the video.

Say I have a 25fps video, in a 25fps project. Each frame is therefore 0.04 seconds long (1/25 of a second). So, theoretically, frame 2 should start at 0.04s, frame 3 at 0.08s, and so on. That's the math OpenShot uses.

But sometimes, when setting the Start of a clip to certain frame positions, the previous frame will "sneak in". For instance, if I set the Start position to 0.80s, which is supposed to be frame 20, the clip might actually start with frame 19, same as if I'd set Start=0.76. But if I set Start=0.84s, it's frame 21! The only way to get frame 20 to be the first frame, in those cases, is by setting Start to a value between 0.77 and 0.79 — but then you've misaligned the clip timing with the project/playhead time and frame boundaries, which can cause a whole host of other problems either immediately or down the road.

And worst of all, there doesn't seem to be any completely-reliable way to predict which frames might be slightly "late", in order to automatically correct for it.

For the "Slice: Keep both sides" functionality, it might be possible to have the resulting two clips overlapped by a half-frame on either side, or something. That may allow them to stay aligned to frame boundaries on the timeline, but still better protect OpenShot against late-frame issues. But for the "Split Clip" tool, I really don't know how to deal with this, except by disclaiming that frame-accuracy is a hard problem to solve and occasionally things may slip a frame here or there.

But it doesn't appear that OpenShot is doing anything wrong with the math, for these instances. It's using all of the right numbers, it's just that sometimes reality doesn't exactly conform to the math.

[jonoomph]

Yeah, this is a tricky issue indeed. Many things in OpenShot use a double precision floating point for time/position. And because that precision is far more precise than 0.04 seconds, it's possible to create some crazy rounding issues. And even rounding to the nearest 0.04 frame position would not help in cases where you change the framerate, breaking everything again. I'll continue to think about this one. =)

[ferdnyc]

Many things in OpenShot use a double precision floating point for time/position. And because that precision is far more precise than 0.04 seconds, it's possible to create some crazy rounding issues. And even rounding to the nearest 0.04 frame position would not help in cases where you change the framerate, breaking everything again.

I don't doubt that floating-point math can foul precision like crazy, especially when dealing with some of the crazy FPS fractions out in the wild. I mean, hell, "23.976" frames/sec isn't even actually 23.976 — it's 24000/1001, which equals 23.976023976023976... (infinitely repeating). But I tried to control for those factors, and still there were problems. Which makes me suspect it's OpenShot doing nothing wrong (though maybe there's more it can do to work around the issue), because the decoder sometimes just doesn't quite output frames exactly when it's supposed to.

Here's the methodology I used, when conducting frame-accuracy experiments:

1. I used a combination of Inkscape and ImageMagick to generate 25 different full-frame 1080p HD images. Twenty-four contain the frame number in black, centered on a colored background. ("LightCoral", specifically.) The 25th is the same but colored blue-on-LemonChiffon, to stand out.
   
   (Hideous, right? I am really regretting that I didn't make each frame a different color, both to give the encoder more work and to make the frame progression, and any ordering issues, more immediately obvious. I may still do that — the numbers are transparent SVG templates, and I composite them over the background color using scripted ImageMagick commands. It'd be a simple tweak to add a continuous lightness or hue progression, when generating the background fill.)

2. I encoded the set of those 25 images, repeated three times, into an MP4 file testframes.mp4 using... well, whatever encoder ImageMagick uses. (According to the metadata, that'd be ffmpeg's "Lavf57.71.100", via the "x264 core 148 r2795" library.) I chose a rate of 25fps specifically because it divides each second cleanly into precise chunks: The frame rate is precisely 25000/1000, each frame is exactly 0.04 seconds long, and the video as encoded has a duration of 3 seconds, 0 milliseconds precisely.

   (One advantage to the samey background color: The entire 75-frame full-HD video, as encoded, is only 112,639 bytes!)

3. I launched OpenShot, selected the "HD 1080p 25 fps (1920x1080)" profile, and imported the video. Then, I tossed it on the timeline and started slicing, splitting, shifting, and otherwise playing with its time-boundaries.

And the results were as I noted above. Even if I manually adjusted the Start parameter of a clip, and explicitly typed in a Start value of "0.80" seconds exactly, the clip would start with frame 19 when it should be 20. "0.76" would also get me frame 19, and "0.84" would get me frame 21. Consistently, every time. I could get it to show me frame 20 by typing in "0.81" or "0.82" or "0.83", but for that particular video, never "0.80".

(If I started the video from frame 1, then at 00:00:00:20 on the timeline sure enough it would show me frame 20. Ditto if I started it from 0.76s and moved one frame forward. It was only when the starting point was exactly where frame 20 was supposed to be, that it showed me the earlier frame. ...And frame 20 is only an example, BTW. This happened with some other frames as well, but so far I haven't been able to detect a pattern.)

So, that's where I'm at with that. It seems like the frames coming in from the decoder are just sometimes not where they should be. Which... maybe it is, maybe that's the nature of the beast. I mean, ffmpeg itself makes no bones about it. In its manpage, the start-position flag -ss is documented thusly (emphasis mine):

When used as an input option (before "-i"), seeks in this input file to position. Note that in most formats it is not possible to seek exactly, so ffmpeg will seek to the closest seek point before position. When transcoding and -accurate_seek is enabled (the default), this extra segment between the seek point and position will be decoded and discarded. When doing stream copy or when -noaccurate_seek is used, it will be preserved.

I still think there's promise in the idea of having the Slice tool create clips that start/end, and even overlap, at half-frame boundaries. Sort of a "pre-roll" for non-first-frame clip starts. My biggest worry is, like I said, ending up with clips positioned off-boundary on the timeline.

Perhaps the pre-roll leader should be both automatic and hidden, so that anytime a clip starts at frame N>1, its actual Start parameter is set to (frame - 1.5 / fps_float) instead of (frame - 1 / fps_float) seconds, and its position on the timeline is automatically shifted left by 0.5 / fps_float seconds. But the Position parameter, and therefore the left edge of the clip (for purposes of repositioning or "resizing", aka adjusting the Start parameter) would automatically be placed a half-frame farther forward, at the frame boundary, so only the decoding is "pre-rolled" and the clip still appears to be aligned to the fps "grid".

If I get really ambitious, I might play with that idea. But probably not... well, not this month, at least.

(On that testframes.mp4 file — I do intend to submit that for inclusion in the source tree, BTW, since as I said it's only 112K in size. Maybe larger, if I vary the backgrounds. But since it's completely my work, I can place it in the public domain and offer it up free and clear. But it's not quite there yet. I have to work out why, even though it plays completely through in the FFmpeg reference player ffplay, in both VLC and OpenShot it only gets to frame 20 in the third loop. VLC actually ends playback prematurely at 2.84 seconds, even though the duration shows as 3 seconds. And in OpenShot, the video plays through all 75 frames / 3 seconds, but the final 5 frames are all frame 20. And none of the tools appear to report any errors regarding the missing end frames.)

[ferdnyc]

Oh, BTW: Given that the remaining issues appear to be more complex than what I've tackled so far, and I'm now more confident that this commit does sort out the Marker and Keyframe positioning issues, it's probably good to go for merging (if it checks out with you, of course). I expect anything else to be done will take a large enough change that it wouldn't make sense to lump those commit(s) in here.

[DylanC]

@jonoomph - Can you test this when you have a chance? Possibly good for merging.

[jonoomph]

Thanks for the PR, looks awesome!

[jonoomph]

Merged

[ferdnyc]

@jonoomph : Do you have any thoughts on the 0- vs. 1-based frame indexing question? (See #1426, probably others.)

The fact that every conversion from time index to frame# involves adding or subtracting a 1 has made me sympathetic to the idea that 0-based is more natural, so I've been toying with the idea of doing the work necessary to go through all of the code and convert to 0-based frame numbers throughout. There are really only two things that give me pause:

1. Keyframe parameters are stored as an [x,y] tuple of [$value, $frame], so they'd have to be able to deal with 0-based. But that shouldn't be a problem, there's no reason that storing [$value, 0] would be any different from storing [$value, 1] in terms of the project file JSON, right?
2. Because this change would thus alter the meaning of that y value, in existing project files, it'd be necessary to version the format so that OpenShot can determine that it needs to subtract 1 from the stored frame numbers when reading older files. (I can't recall whether there's versioning already present in the code/format, which would simplify things, but at the very least it'd necessitate a version increment and the corresponding conversion logic.)