(Go: >> BACK << -|- >> HOME <<)

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

Can't change Safari volume #42

Open
choco opened this issue Apr 23, 2016 · 21 comments
Open

Can't change Safari volume #42

choco opened this issue Apr 23, 2016 · 21 comments

Comments

@choco
Copy link
choco commented Apr 23, 2016

Hello, great app! I noticed that the per app volume control doesn't work with Safari, the volume level doesn't change. The audio detection does work because when I start playing a youtube video, the music player stops, just can't control it.

@choco
Copy link
Author
choco commented Apr 30, 2016

I haven't been able to test if that is the issue, but could it be that since Safari uses a different prices to reproduce audio, video, you actually aren't changing the process volume?
I was reading the source and in BGM_Clients::SetClientsRelativeVolumes you only seem to update the client with the application pid if I'm correct?

@kyleneideck
Copy link
Owner

could it be that since Safari uses a different prices to reproduce audio, video, you actually aren't changing the process volume?

Pretty much, yeah. The problem is that Safari runs each tab in its own process and each play their own audio and they don't use the same bundle ID as the main Safari process. BGMDriver matches clients to their volumes by either PID or bundle ID (depending on what BGMApp sends), so usually multi-process apps are fine because the processes can be grouped by bundle ID.

MPlayer OSX Extended is the only other app I've encountered that does this, but I'm sure there are more. Actually, I didn't even realize Safari did until you reported it.

I think that grouping child processes with their parents would be a decent workaround. The problem with that is because of XPC, launchd is now the parent process of a lot of processes that would normally have been child processes of the process that actually owns them. Safari is an example of this. If you set Activity Monitor to "All processes, hierarchically" you can see Safari's tab processes are grouped under the main Safari process. But if you select one and hit cmd+i you can see that it's parent process is launchd.

There's also no approved way of getting the information Activity Monitor uses to group processes because the API for it isn't public. Similarly, there's no approved way to get the URL for each tab the way Activity Monitor does.

I'm still looking into it, but there's at least one unapproved way of getting the real parent process. If you do launchctl procinfo [the PID of a tab process] the output will include "responsible pid = [the PID of the main Safari process]". But that command has to be run as root and man launchctl says

This subcommand is intended for diagnostic purposes only, and its output should not be relied upon in production scenarios.

I think you can also resolve a PID to the PIDs it's responsible for, without root, with launchctl print pid/[parent PID]. Each service the parent has started is listed under "services", but it looks like services are only listed with their PIDs if the parent is responsible for them (as opposed to just using them). launchctl print also has a big warning in the man page about not relying on it as API, but if we don't have a better option I think we should use it, as long as we also have a fallback behaviour.

There's also this example that uses private Launch Services APIs to get the URLs of Safari tabs. I'll have a look at Activity Monitor in Hopper and see if there's anything else we could use.

It's a bit more complicated than you'd expect for a bug like this, but I think that fixing this bug will also help with things like hiding apps that don't play audio from the app volumes list. And we could even make apps like Safari expandable so each tab can be controlled individually.

@choco
Copy link
Author
choco commented Apr 30, 2016

Hey, thanks for the great infos!
I don't know if it'll be useful for you or not, but I reversed launchctl and found the interesting private API. By doing a quick search on google it looks like that it is used in some Apple programs, so I'm pretty sure it won't get deprecated easily. Also it would probably be easier instead of parsing the tool output.
Here's some code:

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <string.h>
#include <unistd.h>
#include <libproc.h>
#include <errno.h>

extern int responsibility_get_responsible_for_pid(pid_t, uint64_t*, int32_t*, size_t*, char*);

int main(int argc, char* argv[]) {
    if (argc < 2) {
        printf("Usage: %s <pid>\n", argv[0]);
        return -1;
    }
    pid_t pid = (pid_t)atoi(argv[1]);

    char path_buf[PROC_PIDPATHINFO_MAXSIZE];
    if (proc_pidpath(pid, path_buf, sizeof(path_buf)) <= 0) {
        printf("Couldn't get pid path\n");
        printf("Error: %s\n", strerror(errno));
        return -1;
    }
    printf("Path for process: %s\n", path_buf);

    int32_t rpid;
    uint64_t urpid;
    char responsible_path_buf[PROC_PIDPATHINFO_MAXSIZE];
    size_t path_len = sizeof(responsible_path_buf);
    if (responsibility_get_responsible_for_pid(pid, &urpid, &rpid, &path_len, responsible_path_buf) != 0) {
        printf("Couldn't get responsibility pid\n");
        printf("Error: %s\n", strerror(errno));
        return -1;
    }
    printf("Path for responsible process: %s\n", responsible_path_buf);
    printf("Responsible PID: %d\n", rpid);
    printf("Responsible unique PID: %llu\n", urpid);

    return 0;
}

Given a PID it should give responsible pid, unique pid and responsible process path. A responsible process is the responsible process of itself.
Example usage:

clang responsible_info.c -o responsible_info
./responsible_info 11419
Path for process: /System/Library/Frameworks/WebKit.framework/Versions/A/XPCServices/com.apple.WebKit.WebContent.xpc/Contents/MacOS/com.apple.WebKit.WebContent
Path for responsible process: /Applications/Safari.app/Contents/MacOS/Safari
Responsible PID: 532
Responsible unique PID: 532

It doesn't require root privileges.

@choco
Copy link
Author
choco commented May 2, 2016

Yesterday I was trying to test an approach with the API above in the BGMDriver, but I was constantly getting an error "Operation not permitted", so I tried debugging what was going on and tried to check what responsibility_get_responsible_for_pid was actually doing.
The symbol is defined in libquarantine.dylib, and there's even a better function, extern pid_t responsibility_get_pid_responsible_for_pid(pid_t); that just returns the interesting pid (if returned pid is < 0 that it failed), plus other that return similar data. All of these are implemented by a call to __sandbox_ms that is a wrapper for calling function from specific policies defined in the kernel. The policy used by these functions is Quarantine and it is defined in the Quarantine.kext. When calling these functions defined by a policy, there's probably some credential checking, I haven't had time to take a look at how this is all implemented, but I think it simply checks for the userid. So since coreaudiod runs with its own user id it is unable to get info about processes started by other user ids. Running as root obviously let us check get these infos about every process.

@kyleneideck
Copy link
Owner

Nice one. It looks like responsibility_get_responsible_for_pid/responsibility_get_pid_responsible_for_pid is exactly what we want. I've tested your sample code and it works on my system.

You probably already found this since it's the only thing that comes up when you google "responsibility_get_responsible_for_pid", but there's also an example of it being used at https://opensource.apple.com/source/Security/Security-57031.10.10/Security/sec/SharedWebCredential/swcagent.m. It's not that interesting, though, because it only uses the path.

About getting EPERM in BGMDriver, I think you're probably right that it boils down to essentially a UID check. I get the error from sudo -u _coreaudiod ./responsible_info [PID of a process running as me] but sudo -u _coreaudiod ./responsible_info [PID of com.apple.audio.DriverHelper] returns coreaudiod's PID as expected. Also, running it with launchctl bsexec [...] or launchctl asuser [...] doesn't change anything.

I don't think it'll be a problem though, because BGMApp can just get the responsible PIDs and send them to BGMDriver. Actually, I've done a small amount of work on a patch that does it the other way around and sends the PIDs and bundle IDs of all the clients from BGMDriver to BGMApp. I figure BGMApp can just send the same app volume for every child process. (Unless we add a feature for controlling them separately.) I think it might also make it easier if we end up adding more ways for BGMApp to group processes.

So far the patch only sends the PIDs and bundle IDs (I assume -- haven't actually tested it) but let me know if you'd find that useful and I'll commit it to a WIP branch.

@choco
Copy link
Author
choco commented May 2, 2016

That sounds good 👍
Adding a feature to control them both independently and as a whole would be really cool, but I'm not sure how it would "scale" from a GUI standpoint.

@kyleneideck
Copy link
Owner

Yeah, the GUI changes would be tricky. We definitely want to avoid clutter as much as we possibly can, especially in this part of the UI. I spent a bit of time thinking about some related changes for #5, actually.

For this one, I've been thinking that the list would only include top-level apps by default. Possibly with subprocesses enabled by default for apps that are known to work well with the feature. Then, if you enable it in the preferences menu, BGMApp would show an expand arrow ("disclosure triangle button") next to apps with multiple subprocesses. And subprocesses would be ignored unless they're registered as clients with BGMDriver. Also, the list of subprocesses would be flat.

I'm not sure exactly how it should decide what to count as a top-level app, but I think it might work fine to consider any app with NSApplicationActivationPolicyRegular as top-level. Maybe also include apps when they have NSApplicationActivationPolicyAccessory if their main process is responsible for itself.

But obviously we'd have to test with a lot of different apps to see how well it would all work in actual use.

@ibrokemypie
Copy link

Hey has there been a fix for this pushed yet? Sorry if this is a stupid question.

@kyleneideck
Copy link
Owner

@PwnArt1st Not yet. I haven't had a chance to work on Background Music at all over the last week or so.

@daninfpj
Copy link

+1

@choco
Copy link
Author
choco commented Jul 27, 2016

@kyleneideck Hey, you mentioned you had a work in progress patch, I may have some time to look at this and help out, so if you want to push it to a branch I could take a shot at this.

kyleneideck added a commit that referenced this issue Aug 1, 2016
The idea is to have BGMApp group all of an app's clients/subprocesses
together (the way Activity Monitor does if you set it to "app processes,
hierarchically") when setting app volumes.

For issue #42. (See that issue for much more detail.)
@kyleneideck
Copy link
Owner

@choco Cool, thanks. I've just pushed what I had to WIP-MultiprocessAppVols. It's pretty messy, but it runs and seems to be working so far.

There are some TODO comments that I had written for myself. I left them in just in case they're useful, but they probably won't be.

Let me know if you have any questions or anything.

@manojfedx
Copy link

Any solution yet for making safari go silent?? I installed this wonderful masterpiece yesterday! Im happy that many are working fine ! But my safari is not pausing!!
PS: I no noshit about coding! I came here from Packal.org

@kyleneideck
Copy link
Owner

I haven't worked on it since I pushed my WIP branch. I think we'd probably be able to add a temporary workaround for Safari if it's important enough to people, since the whole feature is quite a lot of work.

But my safari is not pausing!!

Do you mean pausing when another app starts playing audio and unpausing afterwards? If so, that would be a different feature to the one this issue is about. Feel free to create a new issue for it. (I think it would be a fairly tricky one, though, since there isn't a general way to pause a webpage.)

@manojfedx
Copy link

Yes thats what I meant Kyle! When I'm switching to say Chrome and when audio is played in chrome/Itunes safari, safari is not pausing!
Anyway I'm going to create a new issue!! Hope you Will eventually get to solve it!

@timothybasanov
Copy link

It would be nice to have a special workaround for Safari. A lot of people use this app on Macs.

When viewed in Activity Monitor it looks like all Safari per-tab processes use "Safari Web Content" process group, while Safari itself uses "Safari". Is it possible to match orphaned processes by a prefix to app processes?

@krapgras
Copy link

Is this still worked on?? i mean this app is great as you guys are the only free decent volume mixer out there for os X!! And i do understand that being able to change every single tab/process independently could be usefull for later development. But i do think you could implement a single tab/process change and "For now" bundel them up? This would potentially fix the problem for the users out there now (as mentioned before lots of mac users use safari) and it would also provide for further changes as you can still access the single process independently.

Ps. This is just an idea :)

@kyleneideck
Copy link
Owner

I've committed a partial fix/workaround for this in c907f13. It should work for Safari, Firefox, Firefox Nightly, VMWare Fusion and MPlayer OSX Extended. If anyone knows or finds any other apps I can add to that list, let me know. It's pretty simple to do now.

There's a snapshot release that includes c907f13 at https://github.com/kyleneideck/BackgroundMusic/releases/download/0.2.0-SNAPSHOT-c907f13/BackgroundMusic-0.2.0-SNAPSHOT-c907f13.pkg

@swietlu
Copy link
swietlu commented Apr 16, 2018

Great update, thanks! Could you please add Adobe Premiere Pro CC?

@awebdeveloper
Copy link

I tried reducing volume of every app that says safari. But it didn't work. Is it working for safari

@Kacpre23
Copy link
Kacpre23 commented May 3, 2024

Hi everyone, I tried changing the volume in the latest version of the app and it still didn't work for Safari

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

10 participants