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.

Sign up for GitHub

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

Jump to bottom

nheko: build with VoIP support #110595

Merged
merged 2 commits into from Feb 23, 2021
Merged

nheko: build with VoIP support #110595

merged 2 commits into from Feb 23, 2021

Conversation

rnhmjoj
Copy link
Contributor

@rnhmjoj rnhmjoj commented Jan 23, 2021 •
edited

Motivation for this change

Making calls (and fix the embedded media player).

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

notes:

  1. The media player works but I still can't get calls to work. In particular is not finding any microphone.
    I tested with both alsa and pulseaudio on NixOS to no avail. On the other hand, the webcam is working.

  2. For a brief moment, after having switched to pulseaudio, it could detect the microphone and I managed to start a call, but nheko crashed as soon as I pressed the bottom green bar.

  3. It seems the problem with the audio sources detection can be traced to the gstreamer monitor: running this

    nix-shell --pure -p gst_all_1.gst-plugins-base --run 'gst-device-monitor-1.0'

fails with Failed to start device monitor!. I would be good if someone else were to confirm this or put the blame on my setup. In theory it should just work and list all the audio and video sources.

cc: @pstn

@ofborg ofborg bot requested a review from fpletz January 23, 2021 13:53
Ekleog
Ekleog approved these changes Jan 23, 2021
View reviewed changes
Copy link
Member

@Ekleog Ekleog left a comment •
edited

Choose a reason for hiding this comment

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

I'm no longer a nheko user myself so other people are probably more qualified than me to review, but as I was asked for review FWIW this looks good to me, though I didn't test it :)

@pstn
Copy link
Contributor

pstn commented Jan 23, 2021 •
edited

I tested voice calling and it worked great.
Video support doesn't work because gst-plugins-good doesn't support qtqml, I added a PR #110608 to fix that.

@pstn pstn mentioned this pull request Jan 23, 2021
Merged
10 tasks
@symphorien
Copy link
Member

Audio call was by default in monitor mode in pavucontrol
image

but if you switch that for non monitor microphone, it works.

@pstn
Copy link
Contributor

pstn commented Jan 24, 2021

I rebased this onto #110608 with the additonal changeset

diff --git a/pkgs/applications/networking/instant-messengers/nheko/default.nix b/pkgs/applications/networking/instant-messengers/nheko/default.nix
index e4473049ddc..3c08d9bae51 100644
--- a/pkgs/applications/networking/instant-messengers/nheko/default.nix
+++ b/pkgs/applications/networking/instant-messengers/nheko/default.nix
@@ -32,7 +32,9 @@ let
   voipDeps = with gst_all_1; [
     gstreamer
     gst-plugins-base
-    gst-plugins-good
+    (gst-plugins-good.override {
+      qt5Support = true;
+    })
     gst-plugins-bad
     libnice
   ];

to enable qt5support in the gst plugin and successfully tested a video call.

@rnhmjoj rnhmjoj marked this pull request as ready for review January 26, 2021 18:53
@ofborg ofborg bot requested a review from Ekleog January 26, 2021 19:00
@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 110595 run on x86_64-darwin 1

1 package marked as broken and skipped:
  • nheko

@ofborg ofborg bot requested a review from matthewbauer February 23, 2021 10:21
@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Feb 23, 2021

After rebasing I noticed the build of gst-plugins-good broke due to a change in master. I should have fixed it, now.

@pstn
Copy link
Contributor

pstn commented Feb 23, 2021

Checked out the PR on top of current master and was able to do a voice call, so lgtm.

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Feb 23, 2021

Good, thank you for testing!

@rnhmjoj rnhmjoj merged commit eb27b8e into NixOS:master Feb 23, 2021
@rnhmjoj rnhmjoj deleted the nheko branch July 10, 2023 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bachp bachp bachp left review comments

@pstn pstn pstn left review comments

@SuperSandro2000 SuperSandro2000 SuperSandro2000 requested changes

@rawkode rawkode Awaiting requested review from rawkode

@symphorien symphorien Awaiting requested review from symphorien

@fpletz fpletz Awaiting requested review from fpletz

@Ekleog Ekleog Awaiting requested review from Ekleog

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

Assignees
No one assigned
Labels
10.rebuild-darwin: 11-100 10.rebuild-linux: 101-500
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@rnhmjoj @pstn @symphorien @SuperSandro2000 @bachp @Ekleog