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

Add a null filter to re-enable frame count #107

Merged
merged 3 commits into from
May 23, 2024

Conversation

tvercaut
Copy link
Contributor

@tvercaut tvercaut commented Mar 7, 2024

This aims at addressing #99

Copy link

@mgorny mgorny left a comment

Choose a reason for hiding this comment

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

Also confirmed that this patch works well for us. Thanks!

gentoo-bot pushed a commit to gentoo/gentoo that referenced this pull request Mar 7, 2024
@almarklein
Copy link
Member

CI fails on one specific test:

def test_read_nframes():
nframes, nsecs = imageio_ffmpeg.count_frames_and_secs(test_file1)
assert nframes == 280
assert 13.80 < nsecs < 13.99

The result is 14.0, so it fails. I confirmed (in #108) that without the change in this PR, this failure does not happen. So apparently ffmpeg produces a slightly different value, depending on these output args.

Nonetheless, I think its safe to just update theupper bound to <= 14.0 in this test.

@almarklein almarklein linked an issue Mar 8, 2024 that may be closed by this pull request
@kkoomen
Copy link

kkoomen commented May 21, 2024

I can confirm this fix works for M1/M2 macbooks as well.

@almarklein
Copy link
Member

@tvercaut could you update the test I referenced in my earlier comment? That should make the tests pass, so we can merge this 🚀

@kkoomen
Copy link

kkoomen commented May 22, 2024

@almarklein can't you just update the tests? Looks like OP isn't that active.

@tvercaut
Copy link
Contributor Author

I just pushed the requested test relaxation change for the PR.

@almarklein almarklein merged commit 44c81e6 into imageio:master May 23, 2024
10 checks passed
@kkoomen
Copy link

kkoomen commented May 31, 2024

@almarklein can you publish a new version pypi version?

@almarklein
Copy link
Member

done!

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.

RuntimeError: Could not get number of frames (with ffmpeg-6.1?)
4 participants