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

Variable frame rate (FPS 0) is not supported #9

Open
Endilll opened this issue Jul 2, 2020 · 4 comments
Open

Variable frame rate (FPS 0) is not supported #9

Endilll opened this issue Jul 2, 2020 · 4 comments

Comments

@Endilll
Copy link
Owner

Endilll commented Jul 2, 2020

Key points:

  • I haven't decided on one true way of "addressing", so internally VSPreview equally supports frames and timestamps
  • There are probably some parts of code that relies on timestamp addressing, including ones that haven't been written yet
  • Support for frame addressing with VFR clips is technically possible to implement in VSPreview alone
  • VFR makes timestamp addressing quite difficult to implement
  • Support for timestamp addressing with VFR clips requires at least some kind of index.

How a proper solution could look like:

  1. Source filters supply timestamp for each frame in clip's properties. They probably already have them in their own index files
  2. VapourSynth does a great job keeping this metadata intact through all manipulations with the clip, taking care of all corner cases
  3. VSPreview takes advantage of the fact that for VFR clip with metadata seeking by timestamp is O(log n).

For instance, given the following CFR and VFR clips:

0    1     2      3      | 0    1     2
0 ms 50 ms 100 ms 150 ms | 0 ms 25 ms 80 ms

appending the latter to the former would result in:

0    1     2      3      4      5      6
0 ms 50 ms 100 ms 150 ms 200 ms 225 ms 280 ms

See also this comment.

@DJATOM
Copy link
Contributor

DJATOM commented Jul 2, 2020

Source filters supply timestamp for each frame in clip's properties

It does, but per frame basis. Each frame must supply timestamp. If it's CFR clip, timestamp remains the same. As I'm aware of, there's only one source filter that can fire VFR clips - ffms2. Probably it's not that difficult to decompress index and examine it's contents to decide if input clip is VFR. Yet it's not that easy to say, if clip was edited or not after importing by source filter (trims, splices, etc). So as universal solution "indexing" of clip might be required.

@OrangeChannel
Copy link
Contributor

OrangeChannel commented Sep 14, 2020

Using an index is really only possible if the clip is from the source filter itself, because like DJATOM said, any modifications to the clip could affect the timestamps. See my discussion about this for an ordered-chapters automation thing and why using an index file isn't a great solution, and pretty much would not work for my purposes.

I have a working Python implementation on creating my own index so to speak but it is very slow currently. I haven't tested it with the concurrent VideoNode.frames() method that's been merged into the audio support branch, but it will apparently be much faster, just not sure by how much https://github.com/OrangeChannel/acsuite/blob/0ebec0221ce7618b7fcd3c805b18e5ff4e472dc1/acsuite/__init__.py#L286.

"Support" for VFR clips could just be achieved by not allowing seeking by timestamp, or just by AssumeFPS'ing a clip with a zero fps attribute.

Or, we could add a --index-file / --timecodes-file command-line option for VFR clips if the user knows they haven't deleted/added any frames nor changed any frame durations since they imported it with the source filter, but that would probably require checking a bunch of stuff against the clip in question and would become pretty complicated for scripts with multiple outputs.

I've asked if there would be a way of getting frameprops without actually needing to render the frame but I've been told that this is out of scope and a bad idea.

@Endilll
Copy link
Owner Author

Endilll commented Sep 18, 2020

  1. Using external index (e.g. from source filter) imposes too much restrictions, which renders this approach useless to my opinion. It also sounds quite error-prone.
  2. VideoNode.frames() would be "render them all" way to create index, which is way too expensive for almost any filtering.
  3. I'm not eager to invest time into disabling timestamp seeking unless I give up on this feature.
  4. I believe any kind of implicit AssumeFPS is even worse than disabling, because it's misleading. If user would like to go this way, he can do it explicitly right away.
  5. I'm not surprised to hear that filling frameprops and rendering goes hand-in-hand. It would be nice to have cheap way to read frameprops, but even without that I can agree on log(n) frames being rendered per seek, as my solution suggests. (I updated the description with an example)

@Endilll
Copy link
Owner Author

Endilll commented May 30, 2021

Initial implementation of VSEdit-like VFR support is available on unstable branch. I'll push it to master after at least some feedback, because I'm not sure how buggy it is

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

3 participants