Skip to content

gpac: add ffmpeg support & refactor#424904

Merged
RossComputerGuy merged 4 commits intoNixOS:masterfrom
thesn10:gpac-full-compile
Nov 8, 2025
Merged

gpac: add ffmpeg support & refactor#424904
RossComputerGuy merged 4 commits intoNixOS:masterfrom
thesn10:gpac-full-compile

Conversation

@thesn10
Copy link
Contributor

@thesn10 thesn10 commented Jul 13, 2025

This is my first contribution so i am happy to take advice ^^

  • Adds FFmpeg support to gpac
  • Adds gpac_nightly package that tracks HEAD
  • Adds myself as a maintainer

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • Nixpkgs 25.11 Release Notes (or backporting 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 25.05 NixOS Release notes)
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other contributing documentation in corresponding paths.

Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 12.first-time contribution This PR is the author's first one; please be gentle! 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` labels Jul 13, 2025
@thesn10 thesn10 force-pushed the gpac-full-compile branch from b8316fa to 7b89a93 Compare July 13, 2025 20:19
@nixpkgs-ci nixpkgs-ci bot added the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Jul 13, 2025
@mdaniels5757 mdaniels5757 requested a review from mgdelacroix July 19, 2025 14:03
@nixpkgs-ci nixpkgs-ci bot removed the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Jul 19, 2025
@nixpkgs-ci nixpkgs-ci bot added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Jul 27, 2025
@thesn10
Copy link
Contributor Author

thesn10 commented Oct 26, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 424904 --extra-nixpkgs-config '{ withFullDeps = true; }'
Commit: 12f8f0a79a4990345d3e134b254a5cac44698d3d


x86_64-linux

✅ 1 package built:
  • gpac_nightly

@thesn10
Copy link
Contributor Author

thesn10 commented Oct 26, 2025

@mgdelacroix can you review?

@thesn10
Copy link
Contributor Author

thesn10 commented Nov 2, 2025

@mdaniels5757 can you review?
The original maintainer seems to be inactive and i want to adopt this package

Copy link
Member

@mdaniels5757 mdaniels5757 left a comment

Choose a reason for hiding this comment

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

Welcome, and thanks for this contribution! I'm sorry it took so long for it to be reviewed.

I've left some comments/suggestions inline. Some additional overall things:

  • The merge commits should not be present in this PR. Probably the easiest way to fix it is to run git rebase -i $(git merge-base HEAD upstream/master)
  • The nix fmt commit should be combined with the "add ffmpeg support & refactor" commit. One way to do this would be to leave the nix fmt commit out of the rebase command above, and edit the "add ffmpeg support & refactor" commit in the rebase. Then when editing the commit you can run the formatter.
  • Commits "2.4-unstable-2025-07-11 -> 2.4-unstable-2025-09-07" and "2.4-unstable-2025-09-07 -> 2.4-unstable-2025-10-26" should be combined into one. You can choose to squash the second into the first in the rebase command above.

@thesn10
Copy link
Contributor Author

thesn10 commented Nov 6, 2025

@mdaniels5757 Thanks for your detailed review and for taking time to actually help me implement this and show me how to integrate the update script! It was very helpful for me as a newbie ^^
I implemented your suggestions, can you take a look again?

Copy link
Member

@mdaniels5757 mdaniels5757 left a comment

Choose a reason for hiding this comment

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

Looks good to me! I'm happy you found my suggestions helpful: I had worried they could be sort of overwhelming...

@mdaniels5757
Copy link
Member

One thing I should note, actually: I don't think that this should be updated on a daily basis. Nixpkgs just isn't built to handle that. The unstable build should be advanced on more of a "the update bot triggered, and the changes made are substantial" basis, for the most part (absent e.g. a bug that is fixed in upstream, in which case a manual update would make sense).

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Nov 6, 2025
@thesn10
Copy link
Contributor Author

thesn10 commented Nov 6, 2025

Okay great, but how do i actually get this PR merged?
Do i also need a review from the maintainer? The problem is he is inactive.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/2632

@RossComputerGuy
Copy link
Member

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 424904
Commit: 62d9589af359626a841841e8f991bf9bd57cc1bf


aarch64-linux

✅ 1 package built:
  • gpac-unstable

Copy link
Member

@RossComputerGuy RossComputerGuy left a comment

Choose a reason for hiding this comment

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

Approved automatically following the successful run of nixpkgs-review.

@RossComputerGuy RossComputerGuy added this pull request to the merge queue Nov 8, 2025
@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Nov 8, 2025
Merged via the queue into NixOS:master with commit cce7304 Nov 8, 2025
27 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons. 12.first-time contribution This PR is the author's first one; please be gentle!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants