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

obs-studio-plugins.obs-backgroundremoval: 0.5.16 -> 1.1.13 #230775

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Zahrun
Copy link
Contributor

@Zahrun Zahrun commented May 8, 2023

Description of changes

occ-ai/obs-backgroundremoval@v0.5.16...1.1.13

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (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.

Requires #228093 (includes tensorrt_provider_factory.h to build)

@Zahrun Zahrun force-pushed the upd/obs-backgroundremoval-2 branch 2 times, most recently from 7c79881 to 1f61c68 Compare May 8, 2023 22:24
@Zahrun Zahrun force-pushed the upd/obs-backgroundremoval-2 branch 2 times, most recently from 044286e to 0e221ce Compare June 9, 2023 05:01
@Zahrun Zahrun changed the title obs-backgroundremoval: 0.5.16 -> 0.5.19 obs-backgroundremoval: 0.5.16 -> 1.0.1 Jun 9, 2023
@spikespaz
Copy link
Contributor

Does this still need to be a draft?

@Zahrun
Copy link
Contributor Author

Zahrun commented Jan 11, 2024

Still pending #228093

@Zahrun
Copy link
Contributor Author

Zahrun commented Jan 11, 2024

#260745 seems promising

@spikespaz
Copy link
Contributor

I had trouble finding the most recent version of the plugin that builds with onnx 1.15.1 (current in unstable). I've been trying for well over an hour.

@Zahrun
Copy link
Contributor Author

Zahrun commented Jan 11, 2024

Maybe we could ask the plugin developers to provide a way to build it in CPU-only mode, which would not require tensorrt_provider_factory.h to build, if that is not already an option.

@Zahrun
Copy link
Contributor Author

Zahrun commented Jan 11, 2024

@wegank wegank changed the title obs-backgroundremoval: 0.5.16 -> 1.1.10 obs-studio-plugins.obs-backgroundremoval: 0.5.16 -> 1.1.10 Jan 23, 2024
@Zahrun Zahrun force-pushed the upd/obs-backgroundremoval-2 branch from e39a7e4 to 136d2f2 Compare January 30, 2024 11:16
@Zahrun
Copy link
Contributor Author

Zahrun commented Jan 30, 2024

onnx seems fine. onnxruntime fails to build though. should be fixed by #258392 according to #281011 (comment)

@Zahrun Zahrun force-pushed the upd/obs-backgroundremoval-2 branch from 136d2f2 to 07063e8 Compare February 8, 2024 05:35
@Zahrun
Copy link
Contributor Author

Zahrun commented Feb 8, 2024

Ok this builds now.
However it does not work yet.
It seems the plugin is not finding the model path.
info: [obs-backgroundremoval] Model file path: (null)

PS: On Ubuntu, that model is located at /usr/share/obs/obs-plugins/obs-backgroundremoval/models/mediapipe.onnx

PSS: also getting
warning: Failed to load 'en-US' text for module: 'obs-backgroundremoval.so'
later
error: [obs-backgroundremoval] Unable to get model filename models/mediapipe.onnx from plugin. error: [obs-backgroundremoval] Failed to create ONNXRuntime session. Error code: 1
and a lot of warning: [obs-backgroundremoval] Background mask is empty. This shouldn't happen. Using previous mask.

@Zahrun
Copy link
Contributor Author

Zahrun commented Feb 8, 2024

@spikespaz I opened an issue on the developers’ repo at occ-ai/obs-backgroundremoval#544.
This is probably more a Nix packaging issue but I’m not sure exactly what’s going on. If you have an idea, please suggest (or anyone else’s help is welcome of course)

@Zahrun Zahrun force-pushed the upd/obs-backgroundremoval-2 branch from 07063e8 to e232059 Compare March 18, 2024 05:24
@Zahrun Zahrun changed the title obs-studio-plugins.obs-backgroundremoval: 0.5.16 -> 1.1.10 obs-studio-plugins.obs-backgroundremoval: 0.5.16 -> 1.1.12 Mar 18, 2024
@Zahrun Zahrun force-pushed the upd/obs-backgroundremoval-2 branch from e232059 to f0ca9e0 Compare March 24, 2024 04:04
@Zahrun Zahrun changed the title obs-studio-plugins.obs-backgroundremoval: 0.5.16 -> 1.1.12 obs-studio-plugins.obs-backgroundremoval: 0.5.16 -> 1.1.13 Mar 24, 2024
@Zahrun Zahrun force-pushed the upd/obs-backgroundremoval-2 branch 2 times, most recently from a21c551 to 4dd8fb2 Compare March 24, 2024 05:15
@Zahrun Zahrun marked this pull request as ready for review March 24, 2024 05:16
@Zahrun
Copy link
Contributor Author

Zahrun commented Mar 24, 2024

Fixed the installation path by removing the extra /usr in the install cmake line.
Also updated to last release version.
Ready for review.

@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-ready-for-review/3032/3681

Comment on lines +37 to 44
buildPhase = ''
cd ..
cmake --build build_x86_64 --parallel
'';

installPhase = ''
cmake --install build_x86_64 --prefix "$out"
'';
Copy link
Member

Choose a reason for hiding this comment

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

Can't we use cmakeFlags and the normal phases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cmakeFlags will apply to the normal buildPhase, right?
How to specify flags for installPhase?
Where is the documentation about this?

@spikespaz
Copy link
Contributor

Is this PR still relevant or do we need a new one?

@Zahrun
Copy link
Contributor Author

Zahrun commented Apr 27, 2024

Is this PR still relevant or do we need a new one?

Sorry I forgot this was pending. In my head this was done and merged. Let me check the review comments.

@Zahrun Zahrun force-pushed the upd/obs-backgroundremoval-2 branch 3 times, most recently from 33a9d03 to 7753a8e Compare April 28, 2024 10:30
@Zahrun Zahrun force-pushed the upd/obs-backgroundremoval-2 branch from 7753a8e to c136052 Compare April 28, 2024 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants