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

wrong return value for SourceAndConverterHelper.getMaxTimepoint #251

Open
tischi opened this issue Jun 3, 2022 · 3 comments
Open

wrong return value for SourceAndConverterHelper.getMaxTimepoint #251

tischi opened this issue Jun 3, 2022 · 3 comments
Labels
bug Something isn't working

Comments

@tischi
Copy link
Collaborator

tischi commented Jun 3, 2022

I think I raised this before, but I cannot find it now:
SourceAndConverterHelper.getMaxTimepoint returns the wrong results because the time points are 0 based. I think we should: return nFrames-1.

@tischi tischi added the bug Something isn't working label Jun 3, 2022
@tischi
Copy link
Collaborator Author

tischi commented Jun 7, 2022

Another questions is why the code inside SourceAndConverterHelper.getMaxTimepoint needs to be so complex? Why not a simple for-loop to check whether a timepoint exists?

@NicoKiaru
Copy link
Collaborator

Hi Tischi, I/you can add more functions that would perform better. Because I am using this function everywhere, I'd suggest to let it exist and put a Deprecated annotation. This will let me the time to update its use in other repos.

Regarding the loop: I wanted the function to work with procedural sources which may return true whatever the timepoint input. If I search for a false return linearly, I'll end up with billions of calls. There may be alternatives (directly looking at Integer.MAX), but the linear search with a hard coded max did not look elegant.

@tischi
Copy link
Collaborator Author

tischi commented Jun 7, 2022

OK, makes sense!
If I find the time I will add an alternative function and add @Deprecated to this one.
As mention in the mail, for yet unknown reasons, this functions takes a really long time for me in some specific context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants