Skip to content

Use ffms2/bestsource instead of ffprobe#14

Merged
moi15moi merged 8 commits intomainfrom
Use-bestsource
Oct 26, 2025
Merged

Use ffms2/bestsource instead of ffprobe#14
moi15moi merged 8 commits intomainfrom
Use-bestsource

Conversation

@moi15moi
Copy link
Owner

@moi15moi moi15moi commented Jun 15, 2025

This PR replace ffprobe with bestsource and ffms2. It should be better than using ffprobe + it won't require the user to install any external dependency (previously, we were asking the user to install ffprobe + adding it to the PATH)

TODO

  • Understand if I need to report the file used for test_get_pts_file_without_pts (the test currently fail)
  • Rework the publish_to_pypi.yml by using cibuildwheel

@moi15moi
Copy link
Owner Author

moi15moi commented Jun 17, 2025

bestsource is too slow.
I tested the get_pts function with a mkv and a mp4 of 24 min both.
For the mkv, it took 265.66 seconds.
For the mp4, it took 23.46 seconds.
Note that i get similar result with aegisub.

That's much much much slower then ffprobe that took less then 1 second for both video :/

I'm not sure I will replace bestsource with ffprobe because of that.

@moi15moi
Copy link
Owner Author

I should maybe try to add ffms2: https://github.com/FFMS/ffms2

ffms2 use autotools. To build it, we should look how to "call" autotools in meson: https://mesonbuild.com/External-Project-module.html

@moi15moi moi15moi force-pushed the Use-bestsource branch 3 times, most recently from bf28343 to 52a0a28 Compare June 26, 2025 12:25
@codecov-commenter
Copy link

codecov-commenter commented Jun 26, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 98.13084% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.45%. Comparing base (fd3db32) to head (0c9a731).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
tests/video_provider/test_video_provider.py 98.92% 1 Missing ⚠️
video_timestamps/video_timestamps.py 80.00% 1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #14      +/-   ##
==========================================
- Coverage   97.77%   97.45%   -0.32%     
==========================================
  Files          18       17       -1     
  Lines        1574     1614      +40     
==========================================
+ Hits         1539     1573      +34     
- Misses         35       41       +6     

☔ View full report in Codecov by Sentry.
📢 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.

@moi15moi moi15moi force-pushed the Use-bestsource branch 14 times, most recently from cbd9bf4 to f378abe Compare July 5, 2025 02:20
@moi15moi moi15moi force-pushed the Use-bestsource branch 6 times, most recently from c0960bf to 6daf3a2 Compare July 13, 2025 21:04
@moi15moi
Copy link
Owner Author

moi15moi commented Jul 13, 2025

pyproject.toml Outdated
[build-system]
requires = ["setuptools"]
build-backend = "setuptools.build_meta"
requires = ["meson @ git+https://github.com/moi15moi/meson.git@fix", "meson-python", "pybind11==2.13.6"]
Copy link
Owner Author

@moi15moi moi15moi Jul 13, 2025

Choose a reason for hiding this comment

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

I'm waiting for meson to fix this problem: mesonbuild/meson#13916
Currently, I set a branch that has the fix.

I'm also waiting pybind11 to fix this issue with the version 3.0.0: pybind/pybind11#5750

Note: If None, it will try to guess it from the PTS and fps.
use_ffprobe_to_guess_fps (bool): If True, use ffprobe to guess the video fps.
If False, the fps will be approximate from the first and last frame PTS.
use_video_provider_to_guess_fps (bool): If True, use the video_provider to guess the video fps.

Choose a reason for hiding this comment

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

Is there a reason why this option could be needed? I see you're anyway computing them using the video provider.

If the concern was on performance optimization maybe this param should be passed to video_provider.get_pts

Copy link
Owner Author

Choose a reason for hiding this comment

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

use_video_provider_to_guess_fps is needed because the fps from the provider is, in 99% of case, better then the one we approximate with the first and last pts. For CFR video, setting use_video_provider_to_guess_fps to true is always a good choice. For VFR video, it depends, but in general, it is better to set use_video_provider_to_guess_fps to false.

The fps will be used to approximate the timestamps after the video duration. This is a case that pretty much no one use, so you probably don't really care about this option.

@moi15moi moi15moi force-pushed the Use-bestsource branch 4 times, most recently from 06ea10f to a54e29b Compare September 7, 2025 15:50
@moi15moi moi15moi force-pushed the Use-bestsource branch 4 times, most recently from 495bba5 to e910304 Compare October 18, 2025 16:39
@moi15moi moi15moi force-pushed the Use-bestsource branch 3 times, most recently from 6fdd1a6 to e6b8ad6 Compare October 25, 2025 23:57
This was needed because the user doesn't always have ffprobe installed and using ffms2/bestsource is more reliable.

There are some minor breaks:
- Removed __version__  of _version.py
- Removed video_timestamps/ffprobe/*
- Renamed use_ffprobe_to_guess_fps to use_video_provider_to_guess_fps param of VideoTimestamps.from_video_file
- Added video_provider param to VideoTimestamps.from_video_file
It adapt the CI to properly build the wheel and test them.
@moi15moi moi15moi force-pushed the Use-bestsource branch 2 times, most recently from 0c9a731 to 0df3ef8 Compare October 26, 2025 01:13
moi15moi and others added 5 commits October 25, 2025 21:16
…project module with ffms2

gstreamer ffmpeg isn't always up to date. For example, ffmpeg 8.0 have been released 2025-08-22, but as of 2025-10-20, gstreamer still only use the version 7.1.
Also, I don't like the fact that gstreamer doesn't use the official build system. There might be difference, so let's not take any chance and use the official build system.

I wanted to use the ffmpeg build system with meson External Project module https://mesonbuild.com/External-Project-module.html, but it just doesn't work. It seems that subproject (in this case bestsource) can't depend on a external project module because they don't use -uninstalled.pc, but i'm not sure.
Also, the external project module doesn't use the .pc file to retrieve the dependencies, so I need to maintain it manually which sucks: mesonbuild/meson#14767
Finally, the external project module doesn't work on msys2. There is a PR that have been opened almost a year ago and work fine, but for a reason, it's not getting merge: mesonbuild/meson#13916

So, for all these reason, let's directly use autotools for ffms2 and the official ffmpeg build system by building them "manually".
nanobind allow to use the limited api which will help to have less wheel.
Currently, I have this issue: mesonbuild/meson#15172
When it is fixed, we will be able to use the limited api.
@moi15moi moi15moi marked this pull request as ready for review October 26, 2025 02:01
@moi15moi
Copy link
Owner Author

The PR is not perfect, but it's been open for a while and it does work, so I will merge it.
Those point will need to be adressed one day:

  • nanobind doesn't support the limited API with meson. When it will, I will need to add the option
  • I don't use the stubgen from nanobind. I tried it and i was getting error. I didn't try to report them.

@moi15moi moi15moi merged commit 1c40c5f into main Oct 26, 2025
0 of 31 checks passed
@moi15moi moi15moi deleted the Use-bestsource branch October 26, 2025 02:13
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.

3 participants