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

Jump to bottom

vlc: Add chromecast support; libmicrodns: Init at 0.0.10 #58588

Merged
merged 2 commits into from Apr 1, 2019
Merged

vlc: Add chromecast support; libmicrodns: Init at 0.0.10 #58588

merged 2 commits into from Apr 1, 2019

Conversation

shazow
Copy link
Contributor

@shazow shazow commented Mar 30, 2019 •
edited

Motivation for this change

VLC normally ships with Chromecast support enabled by default, in the official binaries as well as in other distros (arch, ubuntu, possibly others).

Chromecast support for VLC has been requested on several occasions (#35124 (comment), #58365 from me, and a bunch of times on IRC).

This PR adds libmicrodns which is a library written by the VLC team that is required for their Chromecast streaming to function.

It also adds Chromecast support to the VLC build enabled by default, but can be overridden.

Fixes #58365.

This is my first nixpkgs contribution, please be gentle. 😊

Some notes/questions:

  • I'm not sure if this is a strict requirement, but enabling hardware acceleration is important for transcoding especially: https://nixos.wiki/wiki/Accelerated_Video_Playback - I needed to add intel-media-driver to get libva info: va_openDriver() returns -1 errors to stop. Not sure what's a more generalized solution. Is there a good place to document this? (Otherwise VLC spits out a bunch of errors, might fall back or not)
  • Port 8010 must be open (streaming works by creating a temporary HTTP server on port 8010 which streams the transcoded video, then points the Chromecast to open that local URL). Any suggestions for documenting/enforcing this?
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-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 nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@shazow shazow marked this pull request as ready for review March 30, 2019 17:48
@infinisil
Copy link
Member

Also related: #53184

@shazow
Copy link
Contributor Author

shazow commented Mar 30, 2019

@infinisil Good call, this PR should obsolete #35184, and it also makes scanning/autodetecting work properly (needed libmicrodns).

@flokli
Copy link
Contributor

flokli commented Mar 30, 2019

Port 8010 must be open (streaming works by creating a temporary HTTP server on port 8010 which streams the transcoded video, then points the Chromecast to open that local URL). Any suggestions for documenting/enforcing this?

I'd drop a short comment somewhere in the vlc derivation, and add a longer <para> inside nixos/doc/manual/release-notes/rl-1909.xml, "Other Notable Changes", noting VLC gained Chromecast support, suggesting to set networking.firewall.allowedTCPPorts = [8010].

Does VLC document that port somehwere obvious inside their documentation?

@flokli flokli mentioned this pull request Mar 30, 2019
Closed
10 tasks
@flokli
Copy link
Contributor

flokli commented Mar 30, 2019 •
edited

@shazow you might want to add protoc to both buildInputs and nativeBuildInputs, see #53184 (comment).

I looked around - protoc is used during build to generate cpp code and headers out of the modules/stream_out/chromecast/cast_channel.proto, but later on, is dynamically linked against libprotobuf-lite.so.17 - which would need to be the one for whatever destination architecture this runs on.

protobuf seems to have a --with-protoc= configure option that allows to specify a native protoc binary when cross-compiling, and it looks like since #48450 we already make use of that.

So my guess would be that we only need protobuf in buildInputs, but I'd be glad to get some feedback from somebody a bit more familiar with that.

/cc @ryan4729, @dtzWill @Mic92.

@shazow
libmicrodns: init at 0.0.10
98e24eb 
Required for Chromecast support by VLC.
@shazow
Copy link
Contributor Author

shazow commented Mar 31, 2019 •
edited

Pushed squashed changes.

What's the easiest way to verify that the release notes are rendered properly? (make inside the nix-shell of the docs dir passes OK.)

@flokli
Copy link
Contributor

flokli commented Mar 31, 2019

@shazow from your checkout:

⇒  nix-build nixos/release.nix -A manual.x86_64-linux
⇒  cd result
⇒  python3 -m http.server

Access http://localhost:8000/share/doc/nixos/release-notes.html#sec-release-19.09-notable-changes in your browser

@shazow
Copy link
Contributor Author

shazow commented Mar 31, 2019

@flokli Ah thank you!

Alright, pushed some more minor edits, should be good for another look:

image

flokli
flokli approved these changes Apr 1, 2019
View reviewed changes
Copy link
Contributor

@flokli flokli left a comment

Choose a reason for hiding this comment

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

minor nitpick, apart from that, LGTM!

@ryantm
Copy link
Member

ryantm commented Apr 1, 2019

@GrahamcOfBorg build vlc

@shazow
vlc: add chromecastSupport option
c37aa79 
Enables Chromecast support by default in VLC.

Fixes NixOS#58365.

Includes release note.
@flokli flokli merged commit 8313a5d into NixOS:master Apr 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryantm ryantm ryantm requested changes

@flokli flokli flokli approved these changes

Assignees
No one assigned
Labels
6.topic: nixos 8.has: changelog 8.has: documentation 8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 11-100 11.by: package-maintainer 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

vlc: Add chromecast support
6 participants
@shazow @infinisil @flokli @ryantm @GrahamcOfBorg @Janik-Haag