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

Init gerbera package and reuse mediatomb service #82429

Closed
wants to merge 2 commits into from
Closed

Conversation

@edwtjo
Copy link
Member

@edwtjo edwtjo commented Mar 12, 2020

Motivation for this change

Mediatomb replacement package which is actively maintained. For now just uses the old mediatomb service since the packages are, from a process level viewpoint, interchangeable.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 (usually in ./result/bin/)
  • 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.
edwtjo added 2 commits Mar 12, 2020
The duplication of the interface xml tag is needed for
the daemon to respect the setting.
path = [ pkgs.mediatomb ];
serviceConfig.ExecStart = "${pkgs.mediatomb}/bin/mediatomb -p ${toString cfg.port} ${if cfg.interface!="" then "-e ${cfg.interface}" else ""} ${if cfg.customCfg then "" else "-c ${mtConf}"} -m ${cfg.dataDir}";
path = [ pkg ];
serviceConfig.ExecStart = "${pkg}/bin/${name} -p ${toString cfg.port} ${if cfg.interface!="" then "-e ${cfg.interface}" else ""} ${if cfg.customCfg then "" else "-c ${mtConf}"} -m ${cfg.dataDir}";
Copy link
Member

@aanderse aanderse Mar 12, 2020

if then else "" -> optionalString would be nice.

@@ -257,12 +270,12 @@ in {
###### implementation

config = mkIf cfg.enable {
systemd.services.mediatomb = {
description = "MediaTomb media Server";
systemd.services."${name}"= {
Copy link
Member

@aanderse aanderse Mar 12, 2020

Given the module is called mediatomb this might cause some confusion. It might make more sense to continue to call the systemd service mediatomb and then add a gerbera alias.

@@ -206,20 +219,20 @@ in {

dataDir = mkOption {
type = types.path;
default = "/var/lib/mediatomb";
default = "/var/lib/" + name;
Copy link
Member

@aanderse aanderse Mar 12, 2020

This will cause existing installations to mysteriously break. Either leaving the default to mediatomb or explicitly mentioning this in release notes is a good idea.

Copy link
Member Author

@edwtjo edwtjo Mar 12, 2020

Yes, well this was the intention since it has to be done sooner or later. I'll add it to the release notes to ease the transition.

after = [ "network.target" ];
wantedBy = [ "multi-user.target" ];
path = [ pkgs.mediatomb ];
serviceConfig.ExecStart = "${pkgs.mediatomb}/bin/mediatomb -p ${toString cfg.port} ${if cfg.interface!="" then "-e ${cfg.interface}" else ""} ${if cfg.customCfg then "" else "-c ${mtConf}"} -m ${cfg.dataDir}";
path = [ pkg ];
Copy link
Member

@aanderse aanderse Mar 12, 2020

Is this line actually required?

Copy link
Contributor

@ardumont ardumont Jul 18, 2020

I gather it's not since the command line specifies the full path to the binary...
We'll see ;)

Copy link
Contributor

@ardumont ardumont Jul 18, 2020

As expected, it works without it ;)

pkgs/servers/gerbera/default.nix Show resolved Hide resolved
description = ''
How to identify the server on the network.
'';
};

package = mkOption {
type = types.package;
example = literalExample "pkgs.mediatomb";
Copy link
Member

@aanderse aanderse Mar 12, 2020

Given the wide range of options available it might be more valuable to show an example with an override.

@aanderse aanderse mentioned this pull request Jul 16, 2020
10 tasks
@ardumont
Copy link
Contributor

@ardumont ardumont commented Jul 17, 2020

Hello,

Do you intend to continue and finish this? Or shall I pick it up?

I started a similar work taking another approach. I did not reuse the existing mediatomb service as I don't really know what's common between gerbera and mediatomb. I generated a new conf out of the latest gerbera 1.5.0 and built from this [1]

Now I realize it's not drastically different than current mediatomb service though, main difference is I opened:

  • a mediaDirectories option to allow the media directories to be inlined in the configuration
  • the transcoding option stops hard-coding values to something unresolvable in nix/nixos
  • if transcoding option to no, then the configuration about it is mostly empty
  • if transcoding option to yes, well that pulls the necessary dependencies and declare those so it's actually runnable
  • prefer long flag option instead of short ones in the systemd command so we can understand immediately what the command does (-m -> --home, -e -> --interface...) when looking into systemctl status
  • etc...

I gather this could be integrated in the original mediatomb service as you did here.

I also intend to extend the currently limited import js instructions.
So users can declare their own in their service configuration (as a next step).
It's my understand that to allow better display of media directories within gerbera client (tv for example), one has to do some javascript (<- ikr?).

[1] https://gitlab.com/ardumont/nixos/-/blob/master/odroid/gerbera/service.nix

Cheers,

@ardumont
Copy link
Contributor

@ardumont ardumont commented Oct 8, 2020

Heads up, this can be closed now as #93450 which supersedes it has been merged.

Again, I reused some of the work here, thanks!
I amended the commit which bootstraps the mediatomb service improvment to allow the use of gerbera.
I improved according to the improvments suggested here. And then I iterated over my proposed improvments
as detailed in the previous comment.

Cheers,

@timokau timokau closed this Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants