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

[Feature Request] print absolute path to file #64

Open
amineden opened this issue Jan 20, 2021 · 10 comments
Open

[Feature Request] print absolute path to file #64

amineden opened this issue Jan 20, 2021 · 10 comments

Comments

@amineden
Copy link

Using the %file% argument for the --format option prints out the path of the music file relative to the mpd music directory, as a user I'd like an option to print the absolute path of the music file.
example ~/D/Music/Alice In Chains/[1992] Dirt (Original Version)/13 - Would.mp3 instead of Alice In Chains/[1992] Dirt (Original Version)/13 - Would.mp3

@bogglez
Copy link
Contributor

bogglez commented Jul 25, 2022

What would it take to implement this? I see that struct mpd_song has a uri member which is read by mpc status using mpd_song_get_uri().

I assume that the URI may be http:// or refer to a file on another server.

  1. How would one reliably determine that the URI refers to the local filesystem?
  2. Does mpc know the music directory, e.g. ~/D/Music/?
  3. Related to 1, what to display if the URI refers to a file on another machine?

@MaxKellermann
Copy link
Member

One can use the config or listmounts command to learn the music directory or the mounted directory.

@bogglez
Copy link
Contributor

bogglez commented Jul 25, 2022

One can use the config or listmounts command to learn the music directory or the mounted directory.

According to the listmounts documentation, there may be multiple mounts:

     listmounts
     mount:
     storage: /home/foo/music
     mount: foo
     storage: nfs://192.168.1.4/export/mp3

According to the find documentation, there is "the" (i.e. 1) "music directory":

- ``(file == 'VALUE')``: match the full song URI
  (relative to the music directory)

Does this mean that mpd can only have one music directory and if the URI doesn't contain :// then it is a relative filepath and it is therefore valid to prepend music_directory?

@MaxKellermann
Copy link
Member

That find text predates the existence of mounts. The relative path is relative to MPD's composite storage, which defaults to just the music directory, but may be enriched with various mounts. To find the absolute path, check which mount URI matches the song URI and take that mount's storage.

@bogglez
Copy link
Contributor

bogglez commented Jul 25, 2022

Turns out the config command only works when connected via a UNIX socket.

mpd has the following check:

	/**
	 * Is this client running on the same machine, connected with
	 * a local (UNIX domain) socket?
	 */
	bool IsLocal() const noexcept {
		return uid >= 0;
	}

Maybe this check should be loosened to allow localhost? Otherwise it's impossible to implement absolute filepaths on Windows and Unix clients that don't use Unix sockets.

@MaxKellermann
Copy link
Member

Clients should prefer local sockets on all operating systems where they are available. It's the default for all clients using libmpdclient (unless they override libmpdclient's defaults).

@bogglez
Copy link
Contributor

bogglez commented Jul 31, 2022

You say prefer, but it seems like unix sockets are a hard requirement instead. The loopback device is considered to be local as well by other projects (e.g. authorization in postgresql is handled differently in that case).
The only difference to me seems performance (half latency and triple bandwidth on unix sockets), but that shouldn't matter for software such as mpd. Can you elaborate this requirement?

I created a patch for mpc to support unix sockets under Windows, since they've been supported for a while now. But I can't even get mpd to compile under Fedora and therefore cannot test it, so I benched it.

@MaxKellermann
Copy link
Member

authorization in postgresql is handled differently in that case

Not true, I think you misunderstood how that works. PostgreSQL actually behaves very similar to MPD: connections on local sockets (UNIX domani sockets, not loopback TCP) have different authorization default settings, because on local sockets, the client user id can be determined, but that cannot be done on loopback TCP sockets.

@bogglez
Copy link
Contributor

bogglez commented Jul 31, 2022

See the trust in the default config? That's what I'm referring to.

# The same using local loopback TCP/IP connections.
#
# TYPE  DATABASE        USER            ADDRESS                 METHOD
host    all             all             127.0.0.1/32            trust

It seems like there's no interest in supporting this, so I'm dropping this feature.

@MaxKellermann
Copy link
Member

Wow, you're right, initdb indeed defaults to trust (upstream, but not on Debian, which is why I never saw this) if you omit the command-line option. But also emits a warning both on stderr and in the generated pg_hba.conf.
https://github.com/postgres/postgres/blob/bfac42ea0284f4608d1c37477db14374f4ac9e87/src/bin/initdb/initdb.c#L189-L193
https://github.com/postgres/postgres/blob/bfac42ea0284f4608d1c37477db14374f4ac9e87/src/bin/initdb/initdb.c#L3116-L3122
That's a very dangerous default, and I'm really disappointed by this bad choice of default value, and the warning doesn't make this choice less bad. Fortunately, Debian overrides it for me, but still you're right about upstream PostgreSQL.

Anyway, PostgreSQL is not setting a good example here. It's just bad. "Look at PostgreSQL" is not a convincing argument.

On the other hand, MPD is not a database server and the data transferred over its protocol is less sensitive. Certainly an option could be added that allows users to make MPD trust clients on loopback TCP connections (or on certain custom IP ranges).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants