Arkrissym / Discord.CPP

Discord.CPP is a C++ library for interaction with the Discord API.
MIT License
58 stars 8 forks source link

Voice player out of sync makes music "crash" #5

Closed MysticCoss closed 3 years ago

MysticCoss commented 3 years ago

I have tried the way you send the udp packets here:

auto timer = pplx::create_task([run_timer, this, buffer] {
    waitFor(chrono::milliseconds(5 * FRAME_MILLIS)).wait();
    while (run_timer->get_is_set() || buffer->size() > 0) {
        waitFor(chrono::milliseconds(FRAME_MILLIS)).then([this, buffer] {
            if (buffer->size() > 0) {
                _udp->send(buffer->front());
                buffer->pop();
            }
        }).wait();
    }
});

the music is so "broken" My guess the cause: packets are sent slower than they should because sleep_for is not accurate. I have tried to modify above code:

auto timer = concurrency::create_task([run_timer, this, buffer, FRAME_MILLIS, interval, shared_token] {
while (run_timer->get_is_set() || buffer->size() > 0) {
      utils::sleep(5 * FRAME_MILLIS);
      int loop = 0;
      auto start = std::chrono::system_clock::now();
      while (buffer->size() > 0) {
          loop++;
          int next_time = (FRAME_MILLIS)*loop;
          auto now = std::chrono::system_clock::now();
          if (udpclient.send(buffer->front()) != 0) {
              std::cout << "Stop sendding voice data due to udp error";
              return;
          }
          buffer->pop();

          int ms_elapsed = (std::chrono::duration_cast<std::chrono::milliseconds>(now - start)).count(); // elapsed time from start loop
          int delay = std::max(0, FRAME_MILLIS+ (next_time - ms_elapsed));
          if ((*shared_token)->is_canceled()) {
              std::cout << "Stop sending voice data due to cancel\n";
              concurrency::cancel_current_task();
          }
          utils::sleepex(delay); //sleep_for from std::chrono
    }     
}
});

The result is better but it's not as smooth as i expect, the music broken sometimes, not all the time. There are 2 possible causes:

  1. Bad network : I don't have access to any good network, so after all i haven't seen it work perfectly
  2. Bad code: Because i have limited skill so i currently can't see any problem, so far I hope the bot can run as smooth as public bot Thank you! Edit: I've downloaded an open source bot, it works perfectly, so the problem is mostly code
Arkrissym commented 3 years ago

You guessed right, the packets are indeed sent slower than they should, mainly because the code didn't care how long it actually takes to send a packet and therefore created an additional delay. I did some modifications on the audio sending code based on your suggestion and pushed it to the development branch. Feel free to check it out. Please let me know if that fixes the issue for you.

MysticCoss commented 3 years ago

Thank you for that useful code. That is a step further to fix this issue. It took me half a month testing and trying various methods to achieve the best audio quality. My guess is right, our target is the sleep function, i was surprise that whatever library it comes from(boost::chrono, boost::asio::steady_timer, std etc...), it has the same core function provided by the OS(mine is window, i have no experience with Linux yet). I met this piece of code on the internet but i lost its link as credit. A bit explain about this piece of code:

Arkrissym commented 3 years ago

Hello, i had a look at opus encoder options recently. Setting the expected packet loss rate as you suggested brought some improvements. Additionally enabling the inband error correction finally did the trick. The voice client is now working as expected on both my windows and my linux machines, so i'm closing this issue now. Again thanks for report and your suggestions.