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

Fix get_num_frames_in_video #2090

Merged
merged 9 commits into from
Jun 23, 2023
Merged

Fix get_num_frames_in_video #2090

merged 9 commits into from
Jun 23, 2023

Conversation

namtacs
Copy link
Contributor

@namtacs namtacs commented Jun 17, 2023

Tests have shown that there can be 2 exact same numbers in the output, separated by non-digit symbols. I think it's better to just find the first number.

@tancik
Copy link
Contributor

tancik commented Jun 18, 2023

What are you seeing as the output of ffprobe? I wan't to get a better idea about when it fails.

@namtacs
Copy link
Contributor Author

namtacs commented Jun 19, 2023

Yes, in some cases ffprobe can give results like this: 123123\n\n123123.
For example, the AVCHD codec in MTS format does this.

@tancik
Copy link
Contributor

tancik commented Jun 22, 2023

This doesn't work for me. I ran a test where it should return 635 frames, but it only returned 6.

@namtacs
Copy link
Contributor Author

namtacs commented Jun 22, 2023

There was an error in the code: output[0] was referencing the initial command output, which is not what we want. I changed it to number_match[0]. This means "take the regex match and return the group 0 of it (whole match)".

Copy link
Contributor

@tancik tancik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tancik tancik merged commit ef1350b into nerfstudio-project:main Jun 23, 2023
4 checks passed
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

Successfully merging this pull request may close these issues.

None yet

2 participants