search

NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
17.88k stars 13.94k forks source link

migrate away from ffmpeg_3 #120705

Closed dotlambda closed 2 years ago

dotlambda commented 3 years ago

ffmpeg_3 has many open vulnerabilities (see #94003 and #120372). There seems to be no effort to add patches for these, so we should drop ffmpeg_3 or at least mark it as insecure. In https://github.com/NixOS/nixpkgs/pull/89264, ffmpeg_3 was made the de facto default by making every package that depends on ffmpeg depend on ffmpeg_3 instead. I think that was a bad idea given that the Ffmpeg packages aren't well maintained. Most packages should build just fine with ffmpeg but someone needs to test them.

~Is there an easy way to obtain a list of packages using ffmpeg_3 and ping their maintainers?~

cc @doronbehar @codyopel

Here's a list of affected packages:

Please remove the list of maintainers from packages that are done because GitHub won't allow me to ping more than a certain number of people.

script that I used to generate this list The manually obtained file `packages` contains one attribute per line. ```bash pkgs=$(cat packages) for pkg in $pkgs; do pings=$(nix eval "(with import ./. { }; lib.concatStringsSep \" \" (map (m: \"@\" + m.github) ($pkg.meta.maintainers or [ ])))" --raw) if [ -z "$pings" ]; then echo "- [ ] $pkg" >> packages-with-maintainers else echo "- [ ] $pkg ($pings)" >> packages-with-maintainers fi done ```
Radvendii commented 3 years ago

Ring daemon is already broken. You're welcome to switch the ffmpeg version, and if I ever get around to fixing that package, I'll make sure it works (or downgrade the ffmpeg version if absolutely necessary)

I can do it myself, but I'm assuming it's better to do one large PR changing as many as possible at once, than a million tiny PRs. Let me know if that's an incorrect assumption, and I should bump the ffmpeg version in ring-daemon

dotlambda commented 3 years ago

Ring daemon is already broken. You're welcome to switch the ffmpeg version, and if I ever get around to fixing that package, I'll make sure it works (or downgrade the ffmpeg version if absolutely necessary)

If a package is broken for a long time, we should consider removing it. But given that you want to fix it at some point, I guess we should keep this one.

I can do it myself, but I'm assuming it's better to do one large PR changing as many as possible at once, than a million tiny PRs. Let me know if that's an incorrect assumption, and I should bump the ffmpeg version in ring-daemon

It's easier to do in small PRs because nobody will be able to test whether all these packages still work after switching to a newer Ffmpeg.

magnetophon commented 3 years ago

Ardour builds and runs with regular ffmpeg. A new release is expected any day now, is it OK if I fix it in the PR for that?

andrewrk commented 3 years ago

FYI I'm working on libgroove upstream to bump the dependency. If we want to remove libgroove from nix and resubmit it later when I've completed that work that would be fine with me. Afaik the only package that uses it is groove basin, which I am also the upstream author of and also in the (long) process of rebooting it. IMO these packages should be removed from nixpkgs for now and I will resubmit them in the future.

AndersonTorres commented 3 years ago

Ardour builds and runs with regular ffmpeg. A new release is expected any day now, is it OK if I fix it in the PR for that?

If it is a security update, then it is better to fix it now and backpropagate to current stable releases.

If upstream Ardour sees no update in one week, update it regardless.

AndersonTorres commented 3 years ago

FYI I'm working on libgroove upstream to bump the dependency. If we want to remove libgroove from nix and resubmit it later when I've completed that work that would be fine with me. Afaik the only package that uses it is groove basin, which I am also the upstream author of and also in the (long) process of rebooting it. IMO these packages should be removed from nixpkgs for now and I will resubmit them in the future.

Is this a distant future? If it is broken but you are notifying us that it will be fixed, therefore I would suggest mark meta.broken = true; followed with a comment upstream notified a new version is coming or something like that.

ScarletHg commented 3 years ago

I cannot submit a PR in a reasonable amount of time due to my current employer, but I can confirm that get_iplayer version 3.27 (SHA-256: 077y31gg020wjpx5pcivqgkqawcjxh5kjnvq97x2gd7i3wwc30qi) builds and smoke tests fine using the ffmpeg package.

If someone would like to submit a PR updating the version and making the ffmpeg change, I would appreciate it greatly. ❤️

minijackson commented 3 years ago

Opened an issue for Carla here: https://github.com/falkTX/Carla/issues/1403

peterhoeg commented 3 years ago

I took the liberty of sorting the list to make it easier to read. I don't suggest we keep doing that - it was simply to get the the ones not done yet in order.

jonringer commented 3 years ago

thanks @dotlambda for organizing this, and tackling a good issue :)

jonringer commented 3 years ago

cc @NixOS/release-engineers this a great issue to address, and would be awesome to have completed before branch off :)

dotlambda commented 3 years ago

@jonringer If this is a blocker for 21.05, so should be https://github.com/NixOS/nixpkgs/pull/120582

jonringer commented 3 years ago

@jonringer If this is a blocker for 21.05, so should be #120582

Agreed, we should avoid users having vulnerable software on their systems.

sheenobu commented 3 years ago

Testing spotify on 20.09 by merging the latest spotify into my .nixpkgs. had to use gnutls but I guess that's just a 20.09 specific problem? Commit is here: https://hg.sr.ht/~sheenobu/config/rev/98765d83a39647270a4f6e13d6f806400558f477

nixpkgs commit incoming, thanks!

Profpatsch commented 3 years ago

Ultrastardx ffmpeg 2.x -> 4.2: https://github.com/NixOS/nixpkgs/pull/123416

dotlambda commented 3 years ago

The remaining packages will be marked as insecure by https://github.com/NixOS/nixpkgs/pull/123496. So let's make a joint effort to bring the number of packages still using FFmpeg 3.4 as close to zero as possible before 21.05 is released!

badmutex commented 3 years ago

Vivaldi can be ticked

lunik1 commented 3 years ago

MEGAcmd will need a patch or upstream fix to build against ffmpeg 4.4, as it accesses fields that were made private between 4.3 and 4.4 (see #122405 and FFmpeg/FFmpeg@108864a).

The problem is in the mega sdk so MEGASync might be affected. Looks like @michojel is the maintainer, can you check if this is the case? ffmpeg 4.4 is available on staging-next.

Both still build against ffmpeg 4.4. Are we targeting 4.3 or 4.4 for 21.05?

dotlambda commented 3 years ago

Both still build against ffmpeg 4.4. Are we targeting 4.3 or 4.4 for 21.05?

staging-next will be merged into master before branch-off, so 21.05 will have FFmpeg 4.4

michojel commented 3 years ago

The problem is in the mega sdk so MEGASync might be affected. Looks like @michojel is the maintainer, can you check if this is the case? ffmpeg 4.4 is available on staging-next.

It does fail on staging-next against ffmpeg 4.4:

mega/src/gfx/qt.cpp:1218:18: error: 'AVStream' {aka 'struct AVStream'} has no member named 'skip_to_keyframe'
 1218 |     videoStream->skip_to_keyframe = true;
      |                  ^~~~~~~~~~~~~~~~

I don't have time to work on a patch until next week. But since ffmpeg is optional, shall I temporarily remove the dependency?

Update: addressed in #123620

lunik1 commented 3 years ago

I'm looking into retroArchCores. Seems the only core that uses ffmpeg is PPSSPP which has already been ported, so shouldn't be too much of a challenge.

lunik1 commented 3 years ago

I'm cursed. PPSSPP is affected by the same issue as the MEGA SDK - build broken by FFmpeg/FFmpeg@108864a.

The good news is that in this case upstream has fixed it, just not in the latest release. The PR fixing it, hrydgard/ppsspp#14176, applies cleanly to to the latest release. I have made PRs addressing this for the retroarch core and PPSSPP standalone at #123842 and #123843.

zaninime commented 3 years ago

I've tried updating nginxModules.vod but fails due to the usage of deprecated functions, an issue is open upstream: https://github.com/kaltura/nginx-vod-module/issues/1262

A similar case is there for thumbextractor (although I didn't contribute it), but it can be fixed by updating to the latest revision on master: https://github.com/NixOS/nixpkgs/pull/123881

magnetophon commented 3 years ago

Ardour fixed in the above PR.

lunik1 commented 3 years ago

retroArchCores is done.

loewenheim commented 3 years ago

pcsxr is currently instantiated with ffmpeg_2, which is also marked as insecure.

jchv commented 3 years ago

Lightspark should be good as of #124330 and #124701.

samuela commented 3 years ago

It looks like PRs for capture and mediatomb have been merged now.

eduardosm commented 3 years ago

dr14_tmeter can be marked as done (https://github.com/NixOS/nixpkgs/pull/125241)

hrdinka commented 3 years ago

attract-mode was fixed as well in https://github.com/NixOS/nixpkgs/pull/126354

flexagoon commented 3 years ago

tvheadend was already fixed in #135662

wackbyte commented 3 years ago

libvdpau-va-gl was fixed in #123757

thiagokokada commented 2 years ago

I went ahead and merged all remaining PRs that were already opened pointing on this issue.

vs49688 commented 2 years ago

Related: https://github.com/NixOS/nixpkgs/pull/155537 https://github.com/NixOS/nixpkgs/pull/155539

ajs124 commented 2 years ago

After #155993 only grass and natron will be left, for both of which exist open PRs (#150286 and #121212 (nice)), but they sadly both seem to be stalled.

jonringer commented 2 years ago

grass has been merged

SuperSandro2000 commented 2 years ago

and we are done here. Thank you everyone!

dotlambda commented 2 years ago

and we are done here

What about natron?

dotlambda commented 2 years ago

Once this is actually done, ffmpeg_3 should be removed from Nixpkgs.

SuperSandro2000 commented 2 years ago

We did that in #163509 I just forgot to tick the box.