Moehammered / switch-remote-play

Let the switch remotely play PC games (similar to steam link or remote play)
GNU General Public License v3.0
301 stars 14 forks source link

[QUESTION] assert issue when compiling. #56

Open vamidi opened 2 years ago

vamidi commented 2 years ago

Hi there again :P,

I am trying to compile the project. I have included all the libraries, but when I compile it produces undefined behavior.

It says: ...\common\srp\network\networkdata.h(57,45): error GAB2E2F9B: size '18446744073709551606' of array 'padding' exceeds maximum object size '9223372036854775807'.

This is true because the following code does not produce 6 as a value, which would then fit into the int8_t padding[6].

auto constexpr PayloadPaddingSize = 264 - encoderConfigSize
    - commandCodeSize - controller::controllerConfigSize
    - mouse::mouseConfigSize - touch::touchConfigSize 
    - keyboard::keyboardConfigSize; // should be 6 but isn't

If I change int8_t padding[PayloadPaddingSize] to int8_t padding[6] it asserts because the object size becomes 288 instead of 264 which is 24 byes too much.

I think it has to do with the compiler I'm using. I was wondering if you could tell me how you compile the project.

If I look up the makefile then the compiler that is being used is aarch64-none-elf-gcc.

I am using a 64-bit machine running Windows 11 with the latest switch devkitpro (support to 14.0.0+)

vamidi commented 2 years ago

Also found that static_assert(mouse::mouseConfigSize == 32); is true, while the compiler complains that it should be 48..

Moehammered commented 2 years ago

Hey mate. Lucky I have the asserts there to catch when the output isn't matching as expected haha.

The networkdata header file is a shared header file and is used in the Windows and the Switch project.

For switch I'm using devkitpro (though I haven't updated in about 2 months so maybe something has changed?).

For the Windows project, I'm using the visual studio c++ compiler.

My dev environment is also 64 bit Windows. The payload sizes and such are const expressions so you should be able to hover over them in visual studio (or visual studio code) to see what their sizes are expected to evaluate to when compiled.

vamidi commented 2 years ago

Haha good job indeed on the asserts. Maybe if you update devkitpro you will get the error as well.

I am also going to check if I can compile on windows maybe that will give insight as well.

Thanks for the quick response

Moehammered commented 2 years ago

@vamidi I just did a quick test to compile with the latest libnx and switch tools libraries and I'm getting the same compile issues you mentioned above. I'll investigate what's going on, but my current assumption is that the size of HidNpadButton has changed or some other sdk data type. I'll push out a fix for it when I have time. 👍

Edit: Indeed the HidNpadButton SDK data type has been updated to be a long instead of an integer now. I will need to update all the payload data and configuration structures to work with the new sizes. I'll need to also edit some of the input code since now buttons can be 64bits in width rather than only 32bits.

I'll push out an update for this when I have time. I'll try and make some time this weekend if I can. Otherwise, please be patient. Alternatively, you can use an older version of the LibNX and Switch Tools libraries to compile the project if you're interested in playing around with the code.

vamidi commented 2 years ago

@Moehammered, Absolutely, take your time! I will see the changes whenever. Let me know if I can provide help of any sort.

vamidi commented 2 years ago

So I played around with the code (mostly reading and understanding and compiling) and debugging audio is really hard haha. I am curious where you think there is a race condition issue. It looks like you have separate threads for all of them.

Is it the fact that maybe you read the audiobuffer and that you aren't waiting until the buffer is filled?

EDIT: I see that you stop the process every thread? Maybe you should stop it when it is done sending the payload?

vamidi commented 2 years ago

I have here some output numbers, but I am not sure if they are helpful for you. gyazo

Moehammered commented 2 years ago

@vamidi my hunch about the race condition is less about the thread management and more to do with the audio buffer and its playback. The current audio code is a pure prototype implementation (that took me a couple weeks of head scratching) that I made based solely on the audio project contained in the switch-examples for devkitpro.

I have since read through TriPlayer to learn how audio is queued and played on the Switch. I noticed from looking through TriPlayer that the result of calling the play function returns a structure describing the state of the audio buffer, including whether it is still playing or finished.

There are more issues with my current implementation that make it very sensitive to timing issues that can arise from network conditions and the Switch's clockrate. There is also another issue with how I am handling sample rate and playback. Debugging audio is hard because even if your playback code is correct, if you are late by enough milliseconds to fetch data the audio sounds terrible.

I've made a separate project where I'm experimenting with audio via SDL and it is far cleaner. I plan to resume development within the next week and will be rewriting the audio implementation completely. It should be smaller and more manageable compared to now.

If you're curious and would like to learn, I'd recommend reading the SDL Audio Spec as that's what I've been using to implement the new audio prototype I'm making.

In summary: current audio implementation doesn't properly track if audio is still playing or not, and is sensitive to timing issues when it comes to fetching audio data in time to be played.

vamidi commented 2 years ago

Hi @Moehammered, thanks for the fast response!

Even though the current audio is a prototype, it works quite nicely for me in handheld mode, so you did well in my eyes. Now that I look at the switch-examples I saw the reference, so that is also quite nice for me to understand a little bit more about audio. Thanks for that.

I am looking forward to the SDL implementation. Hopefully, it will be easier to handle the network and clock rate conditions.

TriPlayer was also something I saw while looking through other posts. I did some digging as well and hopefully, I can be of service. I did some development also during my first year of school in SDL (quite a while back frankly haha), so I will take a look at your reference and see if I can come up with something. Maybe I will make some sort of queue buffer to take the network and the switch rate into count.

Lastly, I am wondering how are you with code changes, do you follow a coding standard or are there certain rules you want people to follow when they offer i.e. pull requests?

Anyway, Goodluck next week!

Moehammered commented 2 years ago

@vamidi I'm not too particular about code changes. Usually I do follow a standard but this project is a hobby project that had its scope grow well beyond what I originally planned for it. You can see my naming conventions are inconsistent but I do plan to go through and clean the code base up once a certain set of bugs have been fixed.

This is a pet project for me, so in general I work on it when I have time and to have fun. If you wanna make a pull request, just go for it and if there are any issues at the time I'll point them out.

Have fun! 😄

vamidi commented 2 years ago

Yes, it certainly has grown exceptionally 😄 !

I see that you have pushed the code to compile again with the latest devkitpro. Good job!

I will have a try at it and let you know if I have something to offer.

As for this ticket/question you can close it if you want.

Thanks again for all the info!

vamidi commented 2 years ago

So, I have the audio "working" with SDL, but unfortunately, the audio is too late with playing and a little bit distorted. I think it might have to do with the alignment of uint8_t which has 8 bits and the packetsPerFrame which is 32-bit. I might also have to make use of SDL_GetQueuedAudioSize which can keep track of where the audio currently is but did not have the time to look into that more this week, but I figured maybe you can use this as a start 😄.

// ... rest of the includes

#include <SDL2/SDL_mixer.h>
#include <SDL2/SDL_timer.h>

namespace
{
    AudioConfig constexpr defaultAudioSettings
    {
        .sampleRate {48000},
        .framerate {100},
        .channelCount {2},
        .bitrate {16}
    };

    // SDL settings --> NEW
    SDL_AudioDeviceID dev;
    SDL_AudioSpec wanted;
}

AudioPlayback::AudioPlayback()
    : settings{defaultAudioSettings}, 
      packetsPerFrame {settings.sampleRate/settings.framerate * settings.bitrate/8 * settings.channelCount},
      bufferCount {3}, bufferIndex {0},
      packetBuffers{}, switchAudioBuffers{},
      releaseBuffer {nullptr}
{
    // if the audio is not initialized return -1 when reading the packages.
    InitializeAudio();

    for(auto i = 0U; i < bufferCount; ++i)
    {
        AudioOutBuffer defaultBuffer
        {
            .next {nullptr}, .buffer {nullptr},
            .buffer_size {0}, .data_size {0}, .data_offset {44}
        };
        switchAudioBuffers.push_back(defaultBuffer);

       // CHANGED to uint8_t* instead of char* since SDL asks for it.
        auto buffer = (uint8_t*)memalign(audioBufferAlignment, packetsPerFrame);
        memset(buffer, 0, packetsPerFrame);
        packetBuffers.push_back(buffer);
    }

    std::cout << "AudioBufferAlignment: 0x" << std::setbase(16) << audioBufferAlignment << "\n";
    std::cout << std::setbase(10);
    std::cout << "Packets Per Frame: " << packetsPerFrame << "\n";
    std::cout << "Buffer Count: " << bufferCount << "\n";
}

AudioPlayback::AudioPlayback(AudioConfig config, uint32_t swapBuffers)
    : settings{config}, 
      packetsPerFrame {settings.sampleRate/settings.framerate * settings.bitrate/8 * settings.channelCount},
      bufferCount {swapBuffers}, bufferIndex {0},
      packetBuffers{}, switchAudioBuffers{},
      releaseBuffer {nullptr}
{
    // if the audio is not initialized return -1 when reading packets
    InitializeAudio();

    for(auto i = 0U; i < bufferCount; ++i)
    {
        AudioOutBuffer defaultBuffer
        {
            .next {nullptr}, .buffer {nullptr},
            .buffer_size {0}, .data_size {0}, .data_offset {44}
        };
        switchAudioBuffers.push_back(defaultBuffer);

        auto buffer = (uint8_t*)memalign(audioBufferAlignment, packetsPerFrame);
        packetBuffers.push_back(buffer);
    }
}

// Initializing SDL audio
void AudioPlayback::InitializeAudio()
{
    /* Set the audio format */
    wanted.freq = static_cast<int>(settings.sampleRate);
    wanted.format = AUDIO_S16;
    wanted.channels = static_cast<Uint8>(settings.channelCount);    /* 1 = mono, 2 = stereo */
    wanted.samples = 1024;  /* Good low-latency value for callback */
    wanted.callback = nullptr;
    wanted.userdata = nullptr;

    /* Open the audio device, forcing the desired format */
    dev = SDL_OpenAudioDevice(nullptr, 0, &wanted, nullptr, 0);
    if (dev == 0) {
        fprintf(stderr, "Couldn't open audio: %s\n", SDL_GetError());
        return;
    }

    /* Start playing, "un pause"  --> playing silence*/
    SDL_PauseAudioDevice(dev, 0);

    m_bAudioInitialized = true;
}

int32_t AudioPlayback::ReadPackets(int32_t const & udpSocket)
{
    auto totalRead = 0U;
    socklen_t slen = sizeof(sockaddr_in);
    do
    {
        sockaddr_in si_other;
        uint8_t* dst = packetBuffers[bufferIndex] + totalRead;
        auto lastRead = recvfrom(udpSocket, dst, packetsPerFrame-totalRead, 0, (sockaddr*)&si_other, &slen);
        if(lastRead > 0)
            totalRead += lastRead;
        else if(lastRead <= 0)
        {
            std::cout << "Audio Stream - Received Error Res: " << lastRead << "\n";
            return -1;
        }
    } while (totalRead < packetsPerFrame && udpSocket > 0);

    return totalRead;
}

void AudioPlayback::Play()
{
    // TODO make custom struct that works for SDL instead of the audout struct.
    switchAudioBuffers[bufferIndex].buffer = packetBuffers[bufferIndex];
    switchAudioBuffers[bufferIndex].buffer_size = packetsPerFrame;
    switchAudioBuffers[bufferIndex].data_size = packetsPerFrame;
    // audoutPlayBuffer(&switchAudioBuffers[bufferIndex], &releaseBuffer);

    // Instead of audout playing --> play SDL.
    SDL_QueueAudio(dev, packetBuffers[bufferIndex], packetsPerFrame);

    // std::cout << "ds = " << releaseBuffer->data_size << "\n";
    // std::cout << "bs = " << releaseBuffer->buffer_size << "\n";
    // std::cout << "next = " << releaseBuffer->next << "\n\n\n";

    if(++bufferIndex >= bufferCount)
        bufferIndex = 0;

}

void AudioPlayback::Cleanup()
{
// Close SDL audio
    SDL_PauseAudioDevice(dev, 1);
    SDL_ClearQueuedAudio(dev);
    SDL_CloseAudioDevice(dev);
    for(auto b : packetBuffers)
        free(b);

    packetBuffers.clear();
    switchAudioBuffers.clear();
}

Also for some reason in the screen renderer class if you init the audio and the video at the same time it crashes, so I made a new function that initializes the subsystems after the video init.

bool ScreenRenderer::InitializeSubsystems()
{

    if (SDL_InitSubSystem(SDL_INIT_AUDIO | SDL_INIT_TIMER) < 0)
    {
        std::cout << "Failed to initialise SDL. SDL ERROR: " << SDL_GetError() << std::endl;
        return false;
    }

    return true;
}

Lastly what I wanted to ask is how you debug crashes. I tried to use aarch64-none-elf-addr2line.exe, but I can't get it to work, also if I use Netloader to send the .nro I can't seem to make a connection with the host.

Hopefully next week I hope that I get it to work. 🤞🏽

Moehammered commented 2 years ago

@vamidi thanks for the code snippet. I tested it and it has similar issues to what I'm facing with using the audio callback method. In my test project, there were no audio issues. However, I tested it within Linux and not Windows. sdl2-audio-stream.zip

You can experiment with the zipped project, but you seem to have the general gist. In my current implementation of audio within Switch-Remote-Play I don't receive any crashes initialising video and audio at the same time. It could be because you're also initialising the timer? Dunno.

As for debugging crashes, for a different project (nx-puni) I was able to use addr2line successfully but I've forgotten how. There might be notes about it in that repo on how to use it.

If you make any progress, let me know. :)

vamidi commented 2 years ago

Thanks @Moehammered! This piece you send me will really help me with debbugging the audio, also great mention on the nx-puni repo I will take a look at it in the future.

Thank you for the info, I will keep you posted on the progress.