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

🚫🎵 No Sound from Pings🎵🚫 #1228

Closed
Felanbird opened this issue Aug 22, 2019 · 11 comments · Fixed by #4334
Closed

🚫🎵 No Sound from Pings🎵🚫 #1228

Felanbird opened this issue Aug 22, 2019 · 11 comments · Fixed by #4334
Labels
bug Something isn't working as intended, or works in a confusing/unintuitive way for the user Feature: Highlights Issues related to highlights and ping sounds

Comments

@Felanbird
Copy link
Collaborator

Describe the bug

Ping sounds don't generate a noise

To Reproduce

This issue was referenced to me by another person so I recreated it by doing this:

I downloaded the newest nightly and added a portable folder (as to not add any settings), added my account, added (and subsequently disabled) an extra highlight of my username for testing, and pinged my username via another chatterino instance + account, and no sound was generated.

Expected behavior

Ping sound to make a noise.

Screenshots

https://streamable.com/ex5qg
Commit on 2nd instance covered by box is cbd93f9

Chatterino version

Chatterino Nightly 2.1.0 (21.08.2019 3079765)

Additional context

I use multiple instances of chatterino normally, a forked version of feef6c6 & cbd93f9 both of these instances have no ping problems at all.

I also tested on 381aaaa and I thought it generated the same issue as 3079765 but after closing OBS I was unable to recreate the issue with this instance, even with reopening OBS and recording with it.

I think this issue involves the fact that 3079765 doesn't show up on volume mixer , as the person who referenced this issue to me mentioned the same problem.
I do personally use VoiceMeeter Banana to keep discord out of OBS but the person who referenced me this does not use any sound alteration programs (that he knows of).

Myself and the person who referenced this are both on Windows Version 1903 (OS Build 18362.295) , and I was forcibly updated to this version a few days ago actually. 😤

I closed the normal gaming issue suspects, Battle.net/Steam/Discord , included a PC restart.
I think this is probably a windows side issue but I tried my best. /shrug

@Felanbird Felanbird added the bug Something isn't working as intended, or works in a confusing/unintuitive way for the user label Aug 22, 2019
@gempir
Copy link
Contributor

gempir commented Jul 3, 2020

Happend on stable 2.1.7 recently sound worked for a few mins and after some setting changes suddendly sound was not working despite the Setting being on

Maybe a combination of adding new highlights and enable sounds on them, disabling on other ones?

@Someonexddd
Copy link

Still experiencing this on 2.2.2

@XanthosD
Copy link

Anyone still having this issue? I just started experiencing it.

@Someonexddd
Copy link

Anyone still having this issue? I just started experiencing it.

Yes still experiencing it

@Felanbird
Copy link
Collaborator Author

While I don't want to randomly bump this ancient issue, I want to make a note of it.

This issue seems to always been specific to Chatterino not appearing on the Volume Mixer (and the technical aspects of this fact), as most people who have the issue are able to use this visual example while restarting to "fix" the issue.

Not sure if there's anyway we can re-force Chatterino in this manor, but I just wanted to note my findings over the past year.

@Felanbird Felanbird mentioned this issue May 1, 2022
4 tasks
@Mm2PL Mm2PL added the Feature: Highlights Issues related to highlights and ping sounds label Aug 12, 2022
@Greenlandicsmiley
Copy link
Contributor

I may have found the issue.

// update the media player url if necessary
if (currentPlayerUrl != this->highlightSoundUrl_)
{
player->setMedia(this->highlightSoundUrl_);
currentPlayerUrl = this->highlightSoundUrl_;
}

By commenting out the if statement and leaving player->setMedia and currentPlayerUrl variables uncommented in the above code snippet, I've found that the ping sounds work perfectly as they should.

I've only tested with my own system so far, so if someone could test this or look into a better solution then that'd be perfect.

Specs:
Chatterino 7.4.0 commit 79c64e9
QT 5.15.7
Arch Linux kernel 6.0.10-zen2-1-zen
Pipewire 1:0.3.61-1

@Greenlandicsmiley
Copy link
Contributor

I realized that there are only two functions with the variable currentPlayerUrl

static QUrl currentPlayerUrl;

// update the media player url if necessary
if (currentPlayerUrl != this->highlightSoundUrl_)
{
player->setMedia(this->highlightSoundUrl_);
currentPlayerUrl = this->highlightSoundUrl_;
}

if (currentPlayerUrl != highlightSoundUrl)
{
player->setMedia(highlightSoundUrl);
currentPlayerUrl = highlightSoundUrl;
}

The currentPlayerUrl variable is never used anywhere else to my knowledge, so is there any reason to keep them?

If there is no reason, then maybe the following code snippet can be turned from this

if (currentPlayerUrl != this->highlightSoundUrl_)
    {
        player->setMedia(this->highlightSoundUrl_);

        currentPlayerUrl = this->highlightSoundUrl_;
    }
player->play();

into this?

player->setMedia(this->highlightSoundUrl_);

player->play();

As well as removing static QUrl currentPlayerUrl;

I'm new to this, so I'm not sure if I'm doing this correctly :)

@Nerixyz
Copy link
Contributor

Nerixyz commented Dec 3, 2022

The currentPlayerUrl variable is never used anywhere else to my knowledge, so is there any reason to keep them?

One reason might be, that setMedia doesn't have this check internally. Thus, on some platforms, the player will reopen the file, even though it was already loaded (e.g. on Windows). Not sure if this is a blocker.

Assuming this bug is because media doesn't get loaded, it might be possible to listen to the signals of the player - specifically error and mediaChanged and update/reset the currentPlayerUrl.

@Greenlandicsmiley
Copy link
Contributor

@Nerixyz
Copy link
Contributor

Nerixyz commented Dec 4, 2022

What does everyone think of the following code snippets?

Looks like the correct approach, but your comments don't match your implementation. Your implementation should probably be:

if (!(player->mediaStatus() == QMediaPlayer::LoadedMedia ||
      player->mediaStatus() == QMediaPlayer::BufferedMedia)) {}
// or the equivalent status != loaded && status != buffered

I think you could include the LoadingMedia, BufferingMedia, EndOfMedia statuses, and potentially StalledMedia.

@Greenlandicsmiley
Copy link
Contributor

Thank you for guiding me, the lines have been updated

https://github.com/Greenlandicsmiley/chatterino2/blob/1f7d231328a89f5bd89cc12323c27ef81fa8d5e1/src/controllers/notifications/NotificationController.cpp#L106-L114

https://github.com/Greenlandicsmiley/chatterino2/blob/26223ccf6746ef64b5dca9efb6ee7e9de1d27e07/src/messages/SharedMessageBuilder.cpp#L232-L240

I tried including the EndOfMedia status and found that the ping sound goes away after a few pings, so that cannot be included. However the other statuses you suggested are fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as intended, or works in a confusing/unintuitive way for the user Feature: Highlights Issues related to highlights and ping sounds
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants