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

fix: handle git installation of ffmpeg #5917

Merged
merged 1 commit into from Aug 18, 2023

Conversation

SethFalco
Copy link
Contributor

@SethFalco SethFalco commented Aug 2, 2023

Description

Resolves the error that occurred when using an unconventional distribution of FFmpeg with a non-standard version string. For example, compiled from source, or a third-party fork.

  • #getFFmpegVersion returns the full version string of the FFmpeg now, regardless of format.
  • #parseSemVersion returns null if the string is not a semantic version string.
  • #parseSemVersion handles normalization, such as if the patch segment is missing.
  • If the FFmpeg version does not follow semver (so #parseSemVersion returns null), then we log a warning that we can't check compatibility.

Related issues

Has this been tested?

  • 👍 yes, I added tests to the test suite
  • 💭 no, because this PR is a draft and still needs work
  • 🙅 no, because this PR does not update server code
  • 🙋 no, because I need help


// Fix ffmpeg version that does not include patch version (4.4 for example)
let version = parsed[1]
Copy link
Contributor Author

@SethFalco SethFalco Aug 2, 2023

Choose a reason for hiding this comment

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

Just a heads-up, I don't think this check was ever working properly.
The promise here wasn't being resolved, so the caller was probably just awaiting forever when the version was parsed successfully.

It just looked like everything was running smoothly, since the #checkFFmpegVersion method was a non-blocking call.

})

it('Should parse FFmpeg version from GitHub dev release', function () {
const actual = parseSemVersion('5.1.git')
Copy link
Contributor Author

@SethFalco SethFalco Aug 2, 2023

Choose a reason for hiding this comment

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

I actually compiled and installed n6.1-dev, which showed this version string.
Not sure if that's a typo or what, but a bit odd that the tag and version name don't match up. 🤔

@SethFalco
Copy link
Contributor Author

SethFalco commented Aug 3, 2023

Hmm, seems to me the failing test isn't related to this PR.
Even locally, this test fails on a build from a clean develop branch.

From my local machine:

mocha -c --timeout 30000 --exit --bail ./dist/server/tests/api/search/search-index.js
  Test index search
    Default search
      ✔ Should make a local videos search by default (138ms)
      ✔ Should make a local channels search by default
      ✔ Should make an index videos search by default (363ms)
      ✔ Should make an index channels search by default (165ms)
    Videos search
      ✔ Should make a simple search and not have results (420ms)
      ✔ Should make a simple search and have results (243ms)
      1) Should make a simple search


  6 passing (7s)
  1 failing

  1) Test index search
       Videos search
         Should make a simple search:

      AssertionError: expected 2 to equal 1
      + expected - actual

      -2
      +1
      
      at /home/seth/Documents/repos/contributor/Chocobozzz/PeerTube/dist/server/tests/api/search/search-index.js:82:55
      at Generator.next (<anonymous>)
      at fulfilled (node_modules/tslib/tslib.js:164:62)
      at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

@Chocobozzz Chocobozzz merged commit 2055962 into Chocobozzz:develop Aug 18, 2023
13 checks passed
@Chocobozzz
Copy link
Owner

Thanks!

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.

Cannot check ffmpeg version
2 participants