Skip to content

fix(utils): fixes non monotonous dts in concatenate_video_files - #680

Merged
jeertmans merged 6 commits into
jeertmans:mainfrom
wuerfelfreak:fix_concatenation_frame_timeing
Sep 3, 2026
Merged

jeertmans merged 6 commits into
jeertmans:mainfrom
wuerfelfreak:fix_concatenation_frame_timeing

Conversation

@wuerfelfreak

Copy link
Copy Markdown
Contributor

Fixes Issue

Closes #540

Description

Adds a check for non monotonous dts in concatenate_video_files in utils.py and adjust the dts to be monotonous.

Check List

  • I understand that my contributions needs to pass the checks;
  • The title of my pull request is a short description of the requested changes.

Note to reviewers

I don't know much about PyAV and video encoding so this might not be a good solution. But it works for now.

@github-actions github-actions Bot added waiting review Waiting review (or response) by a maintainer waiting triage This issue or PR needs to be labeled accordingly. labels Aug 28, 2026
@codacy-production

codacy-production Bot commented Aug 28, 2026 •

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 13 high · 2 minor

Alerts:
⚠ 15 issues (≤ 0 issues of at least minor severity)

Results:
15 new issues

Category Results
UnusedCode 1 minor
ErrorProne 12 high
Security 1 high
CodeStyle 1 minor

View in Codacy

🟢 Metrics 0 duplication

Metric Results
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@codecov

codecov Bot commented Aug 28, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.02%. Comparing base (c6c134b) to head (ce65ad7).

Files with missing lines Patch % Lines
manim_slides/utils.py 50.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #680      +/-   ##
==========================================
- Coverage   79.10%   79.02%   -0.08%     
==========================================
  Files          23       23              
  Lines        2178     2184       +6     
==========================================
+ Hits         1723     1726       +3     
- Misses        455      458       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jeertmans

Copy link
Copy Markdown
Owner

Hi @wuerfelfreak, thanks for your contribution! Could you add:

  • some tests that would show that a previously failing rendering now succeeds;
  • an entry to the changelog documenting your contribution?

@wuerfelfreak

Copy link
Copy Markdown
Contributor Author

I've not written many tests before. I think there are two options for testing concatenate_video_files.
Either by rendering the full minimal working example by @erooke in #540 or by adding the two video files mentioned in my comment in #540 to the repository under test/data and just pass these two to concatenate_video_files.
Which of the two would be the preferred option?

Thanks for the guidance.

@jeertmans

Copy link
Copy Markdown
Owner

I would go for the first option. Write a test that fails, raising the target error, and see if it is fixed by your patch :-)

@jeertmans jeertmans left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks @wuerfelfreak!

@jeertmans
jeertmans merged commit 7034edc into jeertmans:main Sep 3, 2026
48 of 54 checks passed
@github-actions github-actions Bot removed waiting triage This issue or PR needs to be labeled accordingly. waiting review Waiting review (or response) by a maintainer labels Sep 3, 2026
Rodbourn added a commit to Rodbourn/manim-slides that referenced this pull request Sep 6, 2026
Rodbourn added a commit to Rodbourn/manim-slides that referenced this pull request Sep 6, 2026
Drop the unused `tmp_dir` binding in `test_issue540` (F841) and move the
DTS clamp out of `concatenate_video_files` into a small helper so the
function is back under the complexity limit (C901). No behavior change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ALmkPCFPECUkL8SLB5EQS
Rodbourn added a commit to Rodbourn/manim-slides that referenced this pull request Sep 6, 2026
Drop the unused `tmp_dir` binding in `test_issue540` (F841) and move the
DTS clamp out of `concatenate_video_files` into a small helper so the
function is back under the complexity limit (C901). No behavior change.
Rodbourn added a commit to Rodbourn/manim-slides that referenced this pull request Sep 7, 2026
This reverts commit 4db3028.

Reverted per review feedback: this PR is scoped to moving the `ty` hook
off pre-commit.ci, and the lint fixes belong in their own PR.
jeertmans pushed a commit that referenced this pull request Sep 9, 2026
* ci: run ty outside pre-commit.ci

* chore(changelog): add entry for running ty outside pre-commit.ci

* chore(lint): fix ruff errors introduced by #680

Drop the unused `tmp_dir` binding in `test_issue540` (F841) and move the
DTS clamp out of `concatenate_video_files` into a small helper so the
function is back under the complexity limit (C901). No behavior change.

* chore(changelog): wrap #682 entry to 80 columns

* Revert "chore(lint): fix ruff errors introduced by #680"

This reverts commit 4db3028.

Reverted per review feedback: this PR is scoped to moving the `ty` hook
off pre-commit.ci, and the lint fixes belong in their own PR.

* fix(ci): install cairo and pango headers for type checks
ajlaanayan-crypto added a commit to ajlaanayan-crypto/manim-slides that referenced this pull request Sep 29, 2026
…tmans#680)

* fix concatenation frame timeing

* Add test reproducing Issue 540 using example by https://github.com/erooke

* Changelog

* chore(fmt): auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: wuerfelfreak <>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Jérome Eertmans <jeertmans@icloud.com>
ajlaanayan-crypto added a commit to ajlaanayan-crypto/manim-slides that referenced this pull request Sep 29, 2026
* ci: run ty outside pre-commit.ci

* chore(changelog): add entry for running ty outside pre-commit.ci

* chore(lint): fix ruff errors introduced by jeertmans#680

Drop the unused `tmp_dir` binding in `test_issue540` (F841) and move the
DTS clamp out of `concatenate_video_files` into a small helper so the
function is back under the complexity limit (C901). No behavior change.

* chore(changelog): wrap jeertmans#682 entry to 80 columns

* Revert "chore(lint): fix ruff errors introduced by jeertmans#680"

This reverts commit 4db3028.

Reverted per review feedback: this PR is scoped to moving the `ty` hook
off pre-commit.ci, and the lint fixes belong in their own PR.

* fix(ci): install cairo and pango headers for type checks
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.

[BUG] [Errno 22] Invalid argument when rendering a large number of animations

2 participants