Trying to solve issue #203

[Foaly]

I started to work on an issue that has been bugging me for quiet some time now: issue #203. The issue is, that if a music is paused or stopped and you call setPlayingOffset() on it the music gets restarted.
The problem is not as simple as it might first look. My solution to the problem was to simply get the status of the music at the beginning of the function and restore it at the end. I tryed this:

void SoundStream::setPlayingOffset(Time timeOffset)
{
    // Get old playing status
    Status oldStatus = getStatus();

    // Stop the stream
    stop();

    // Let the derived class update the current position
    onSeek(timeOffset);

    // Calculate new position
    m_samplesProcessed = static_cast<Uint64>(timeOffset.asSeconds() * m_sampleRate * m_channelCount);

    // Play
    m_isStreaming = true;
    m_thread.launch();


    // Restore to old status
    if(oldStatus == Stopped)
        stop();   // stop
    else if(oldStatus == Paused)
        pause();   // paused
}

The problem with this code is that it's correct, but it doesn't work as expected, because if the music is paused it still restarts. After some investigation i found out, this is because the pause command is executed faster than the play command in the threaded function. So the music is actually paused, but played right afterwards. If I insert a sleep(milliseconds(20)); before restoring the old state (which is obviously not an option) everything works as expected.
I also tried this:

    // Calculate new position
    m_samplesProcessed = static_cast<Uint64>(timeOffset.asSeconds() * m_sampleRate * m_channelCount);

    // Restore to old status
    if(oldStatus != Stopped)
    {
        m_isStreaming = true;   // paused
        if(oldStatus == Playing)
            m_thread.launch();   // played
    }

At first it seemed to work fine, but when the music was in a paused state it is not possible to resume it afterwards, because the thread is never actually launched...

So I am a little stuck now and not really sure how to continue from here. So I thought I share what I found so far and ask if somebody has another idea. Because this seems like an annoying bug, that should be fixed
Anyways I have a debugging environment set up and am happy to help.
Foaly

[Laurent]

Thanks for trying to fix this issue and sharing your work so far

But this function is really inefficient, it must be rewritten, not fixed.

So you should keep your workaround for now, and if you're lucky maybe I'll work on it for SFML 2.1

[eXpl0it3r]

I haven't looked into the code, but from what you've described, it sounds like a race condition.
Race conditions can have different approaches in solving them. Often the easiest way is to use a mutex and lock things down, but that is often not wanted due to performance etc. The best way would be to change the logic in a way that one can get around without locks, but this can involve a complete rewrite of the surrounding functions. How good is your knowledge in parallel programming?
Then again maybe I just jumped to a false assumption...

Btw. the link to the play call is broken.

[Foaly]

this function [...] must be rewritten, not fixed.

I was thinking that, when I was working on it It's not possible to solve this problem with the current setup of methods.

My knowledge in parallel programming is alright, not super good, but maybe if I find some time, I can try and see if I find a non-blocking solution to this problem, that Laurent can use as a base, for his rewrite.

Just out of curiosity: What in this class would need a rewrite Laurent?

For now I will just link this thread to the issue, so the information doesn't get lost.

[Laurent]

Just out of curiosity: What in this class would need a rewrite Laurent?

Only this function, I'd say.

[Foaly]

Ah ok I see. I'm just not quiet sure if it's possible with the way class is working right now.

Oh and I forget to include the minimal code i was testing with:

#include <SFML/Audio.hpp>
#include <SFML/Window.hpp>
#include <iostream>

int main()
{
    bool bRunning = true;

    sf::Music music;
    if(!music.openFromFile("music.wav"))
        return -1;
    music.play();
    sf::sleep(sf::seconds(3));
    std::cout << music.getPlayingOffset().asMilliseconds() << std::endl;
    music.pause(); // music.stop();
    music.setPlayingOffset(sf::milliseconds(500));
    std::cout << music.getPlayingOffset().asMilliseconds() << std::endl;
//    music.play();
    std::cout << music.getPlayingOffset().asMilliseconds() << std::endl;

    while(bRunning)
    {

        if (sf::Keyboard::isKeyPressed(sf::Keyboard::Escape))
        {        
            std::cout << music.getPlayingOffset().asMilliseconds() << std::endl;

            bRunning = false;
        }
    }
}

[Laurent]

I'm just not quiet sure if it's possible with the way class is working right now.

I don't think it will require other major modifications. Perhaps a few minor additions in the stream() function.