telegramdesktop / tdesktop

Telegram Desktop messaging app
https://desktop.telegram.org/
Other
26.44k stars 5.25k forks source link

[Feature Request] Move notification audio handling to notification center on Mac #25211

Open ShadowJonathan opened 2 years ago

ShadowJonathan commented 2 years ago

Is your feature request related to a problem?

This is related to https://github.com/telegramdesktop/tdesktop/issues/9857, when investigating why notifications would always play audio on my mac while notifications were silenced, I saw that audio for notifications is handled separately to the notification popup itself.

This is not how notifications are handled on Mac.

When trying to rectify this, I wasn't able to navigate the codebase to figure out how to extract the relevant audio for the notification and put it into a mac notification center notification.

Describe the solution you'd like

The above; Putting the notification sound for a MacOS telegram notification in the notification object itself, not to play it seperately, ignoring DnD and detailed focus settings, which are dynamic and should be prioritised above what Telegram thinks the DnD state is for the system.

Describe alternatives you've considered

Narrowing down the queryDoNotDisturbState function, however, due to the way Mac handles focus mode on newer OSes, this could become difficult to keep up, especially as mac might hide the specific focus to applications, only applying it to filters on the notifications themselves.

Additional context

No response

john-preston commented 2 years ago

@ShadowJonathan I'd like to stick to current approach if possible. For example, right now you can disable toasts and still have the sound about the new messages. Is this possible in native notification sound? Right now in case toasts are disabled the notification queue isn't even maintained.

ilya-fedin commented 2 years ago

I also wonder whether macOS would read tdesktop's notification sounds as they're in mp3 format and, moreover, the ability to select random mp3 tracks is given to the users via UI.

ilya-fedin commented 2 years ago

When trying to rectify this, I wasn't able to navigate the codebase to figure out how to extract the relevant audio for the notification and put it into a mac notification center notification.

Core::App().settings().getSoundPath(qsl("msg_incoming")). For the default ringtone, this will give a path in the Qt resource system.

ilya-fedin commented 2 years ago

Narrowing down the queryDoNotDisturbState function, however, due to the way Mac handles focus mode on newer OSes, this could become difficult to keep up, especially as mac might hide the specific focus to applications, only applying it to filters on the notifications themselves.

Are you completely sure there's no simple API to detect the setting for application/notification? For instance, Windows provides such API and tdesktop uses it: https://github.com/telegramdesktop/tdesktop/blob/7b5781b84573ba02cf968b093fa37f23c6b4cb3f/Telegram/SourceFiles/platform/win/notifications_manager_win.cpp#L260-L321

ShadowJonathan commented 2 years ago

For example, right now you can disable toasts and still have the sound about the new messages.

@john-preston This is not the problem, if I silence notifications from telegram in a focus preference, MacOS dutifully filters the notifications from appearing.

However, it does not prevent the sound from playing, this is because telegram plays this sound independently from if the notification is allowed through via the notification center.

This issue is made to fix that, as I'm pretty sure it is possible to give Mac the right sound clip to play with a notification, and then leave it up to the system to decide to play it or not at that time, not to have tdesktop do this regardless of that decision.

john-preston commented 2 years ago

@ShadowJonathan I'm not sure you understood my problem.

For example, right now you can disable toasts and still have the sound about the new messages.

I mean that right now in the app Settings you can disable Desktop Notifications but still have the new message sound playing. I was wondering if I can do that by playing the sound through the system notifications code (without showing any notifications on the screen).

ShadowJonathan commented 2 years ago

Uhm... I don't think that is possible, no, sorry.

If that is a requirement of this approach, I'd say to have tdesktop itself handle the audio for that one, as effectively there is no "notification" to be seen, only a sound.

For all other cases (where a native notification is used), the system notification system should be used for notification sounds.

ilya-fedin commented 2 years ago

How to solve the bug for users using only sound notifications though?

VitalyEmelyanov commented 2 years ago

The linux version has a toggle "Use system notifications". Can we add the same for macOS, to make it possible to use standard macos notifications behavior?

ilya-fedin commented 2 years ago

The Linux version behaves just like macOS when this toggle is active (system tooltip, custom sound). When it's disabled, custom tooltip is used.

ShadowJonathan commented 2 years ago

"Use system notifications" should mean that telegram switches entirely to system notifications, if the system for whatever reason decides to filter a notification, telegram should honour that in full.

ilya-fedin commented 2 years ago

So for macOS it's just the toggle is always active and hidden

ilya-fedin commented 2 years ago

"Use system notifications" should mean that telegram switches entirely to system notifications, if the system for whatever reason decides to filter a notification, telegram should honour that.

Well, tdesktop tries to honor that, it's just the current code doesn't work for some reason.

ShadowJonathan commented 2 years ago

Well, tdesktop tries to honor that, it's just the current code doesn't work for some reason.

No, it doesn't honour this, as it still plays its own sound, instead of letting the system also handle that aspect of notifications.

VitalyEmelyanov commented 2 years ago

I'd expect 'Use system notifications' must really use system notifications, no 'custom sounds' etc. When it's disabled, then yes, it should allow custom sounds and other changes to notifications behavior

ilya-fedin commented 2 years ago

I said it tries to honor, it's just there's some bug in the DnD query no one knows how to fix.

ShadowJonathan commented 2 years ago

no one knows how to fix.

This is not the case, it is just that MacOS uses a different (more mobile-aspected) approach to notification handling now, where the notifications are filtered inside notification center, instead of an application being able to query the DnD status themselves, and filter it that way.

This means that telegram has to switch to using the notification api for all its notification business, as there is no other way to honour focus preferences.

john-preston commented 2 years ago

@ShadowJonathan

image

Local reencoding of sounds required. Also, this API is from 10.14, I wonder if it can be used in an app supporting 10.12 and above (with a fallback to current implementation).

ilya-fedin commented 2 years ago

@john-preston does macOS support overriding notification sound per-app in system settings? If yes, maybe just use the default sound and hide notification sound settings in the application?

ShadowJonathan commented 2 years ago

Local reencoding of sounds required.

I wonder if this can be done on-demand, and cache the sound somewhere, but yes, it seems a little bit more work than I previously thought.

ilya-fedin commented 2 years ago

That's how I wanted to handle Linux notifications initially... Unless it's appeared there's a design flaw in Linux notification API and there's no way to be sure notification daemon supports system-provided sounds.

ShadowJonathan commented 1 year ago

After another month of usage, I've come to the conclusion that I really really would like this to be in tdesktop, even if it means that the custom sounds will play outside of the macos notification system (for a time), while only the default sound gets played through it.

I think that, for now, that would be a good solution, as honestly by the day I'm getting more and more annoyed by telegram ignoring my Focus status, while this issue is blocked on custom sounds, which I don't even use, nor know anyone else who uses them.

ilya-fedin commented 1 year ago

This is blocked on someone to write the transcoder in tdesktop

maddocnc commented 1 year ago

One more point for standard Mac OS system.

In Mac OS you can also choose separate audio out for notifications, so for example main external speakers could be on hight volume while you are working and all notifications goes with another volume from MacBook speakers.

Disrespecting Mac OS notification workflow not only gives us problems with focus, but also telegram desktop notifications comes too loud on a wrong audio output.

ilya-fedin commented 1 year ago

It's very nice you folks enumerate advantages, but the reality is this needs someone to do the work, not enumerating advantages... And given that implementing a transcoder is a very non-trivial task, it's unlikely to implemented any time soon (if ever).

VitalyEmelyanov commented 1 year ago

Does anyone really need the feature to have custom sounds instead of having a simple system notification with default sound that behaves as expected, respecting Focus and other features? Maybe it’s really better to have it in KISS principle?

ilya-fedin commented 1 year ago

It's a platform-wide feature to be able to specify per-chat sounds and it's something tdesktop must respect as an official application. In an unofficial fork, one can do whatever they want.

ShadowJonathan commented 1 year ago

I don't care about custom sounds, let it be a bug that they don't use the notification system, but please, make the default one go through Notification Center, we can do that by transcoding the sound to the right format, shipping it, and then just use that on Mac, so we don't need to transcode dynamically for the default one.

rollcat commented 7 months ago

Well, Telegram on Mac woke me up yesterday while I was taking a nap after a very difficult day - I've set sleep mode on the phone, which was synced to the Mac, but Telegram disrespected that setting and allowed a call through anyway, blasting the ringtone through the speakers at full volume.

On iPhone, I could use Shortcuts + automations to run a shortcut (e.g. to quit an app) whenever entering sleep focus; but this is not possible on a Mac (unless I'd use SSH from the phone, which sounds even more hacky).

It seems like there are only two reliable solutions to the immediate problem of receiving an unwanted notification at a wrong time: disable notifications (incl. all calls?) in Telegram entirely, or removing the app (I might forget to quit it otherwise). Neither is great.

ilya-fedin commented 7 months ago

@rollcat perhaps you're using Telegram from mac store, you can switch to the build from the website (https://desktop.telegram.org) which has solved this long time ago.

john-preston commented 7 months ago

@rollcat Well, I'm afraid calls sounds don't follow those settings :( This should be considered a bug, I guess.

ShadowJonathan commented 7 months ago

@john-preston is notification sound handling currently passed off through the notification center?

ilya-fedin commented 7 months ago

@ShadowJonathan no, client checks for system settings manually. But it works now (outside of app store sandbox).

ShadowJonathan commented 7 months ago

https://github.com/telegramdesktop/tdesktop/blob/01bfa467295684d6ca0943024ca848bddf7a1484/Telegram/SourceFiles/platform/mac/notifications_manager_mac.mm#L617

I just saw this line, yeah, it checks for a focus state for notification sounds.

john-preston commented 7 months ago

@ShadowJonathan It should work fine in the version from the website for the notifications sound, but not for incoming calls, calls just play the sound :(

ilya-fedin commented 7 months ago

This should be considered a bug, I guess.

I guess different users could have different expectactions, just like when inheriting Windows settings was fixed and people started doing bug reports that notifications inherit them and you disabled that for custom notifications. Calls are custom in that sense, too.