-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Issues #631 #632 #634
Issues #631 #632 #634
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer "i for i,e in enumerate(tt[:-1])..." instead of the usage of zip()
@uummoo running into
using your solution. |
…uses exactly n frames for avg. before edge cases where not. improvement for Zulko#634
… now times which expand clip.duration as well. improvement for Zulko#634
@@ -79,7 +79,7 @@ def concatenate_videoclips(clips, method="chain", transition=None, | |||
|
|||
if method == "chain": | |||
def make_frame(t): | |||
i = max([i for i, e in enumerate(tt) if e <= t]) | |||
i = max([i for i, e in zip(range(len(tt) - 1), tt) if e <= t]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is better to use np.searchsorted
i = np.searchsorted(tt, t)
clip = VideoFileClip("media/big_buck_bunny_432_433.webm") | ||
|
||
clip1 = supersample(clip, clip.duration, 1) | ||
assert clip1.duration == clip.duration | ||
clip1.write_videofile(os.path.join(TMP_DIR, "supersample1.webm")) | ||
|
||
clip2 = supersample(clip, clip.duration / 2, 2) | ||
assert clip2.duration == clip.duration | ||
clip2.write_videofile(os.path.join(TMP_DIR, "supersample2.webm")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know, maybe it is just me. But this test passes even without your changes. Which leads to question: what the reason of this PR.
I mean there is definitely a problem has been solve, but tests didn't reflect this fact. In other words there is no tests to reproduce an issue you have tried to solve.
Thank you for your contributions and for reporting issues in this repository. With the release of v2, which introduces significant changes to the codebase and API, we’ve reviewed the backlog of open PRs and issues. Due to the length of the backlog and the likelihood that many of these are either fixed or no longer applicable, we’ve made the decision to close all previous PRs and issues. If you believe that any of these are still relevant to the current version or if you'd like to reopen a related discussion, please feel free to create a new issue or pull request, referencing the old one. Thank you for your understanding and continued support! |
Fixes for #631 and #632.
Still needs to be discussed. I can't oversee all the implications of these changes.