Fixed Player::skip_one not updating player's length immediately.

[nieboczek]

Fixes #497.
Needed to add Done::should_decrement to achieve this fix.

Instead of updating sound_count in the periodic access source, I made it update directly in the skip_one function (clear affected too).
When controls.to_clear > 0 in periodic access, it will use Done::should_decrement to temporarily disable decrementing sound_count.

[nieboczek]

Something very bad seems to be happening with the Player, adding a simple assert_eq hangs the test on Player::clear:

        // ...
        assert_eq!(player.len(), 1);

        assert_eq!(source.next(), Some(1.0));

        player.clear();
        // ...

Could I get some help on this? Fixed.

[nieboczek]

I've hopefully fixed that. Also saw a queue issue where it wouldn't play the first audio appended in my own app with the version from the PR, but that is working correctly after my latest commit.

[yara-blue]

Hii,

Thanks for working on this! Sorry I could only get to this review now (was sick for a bit but all is well now).

I'm a little hesitant, I'm not sure should_decrement is useful outside fixing the Player and once we add a new API it's hard to remove it. Maybe we should rethink Done and see if we can make it more useful in general. That might allow a cleaner fix, even though it would involve a breaking change. I think a breaking change is fine if it enables new use cases.

An idea:
We could make Done::new take a closure as argument and run that when the source finishes. Something like:

pub struct Done<I, F: FnMut()> {
    input: I,
    callback: F,
    signal_sent: bool,
}

    pub fn new(input: I, callback: F) -> Done<I, F> {
        Done {
            input,
            callback,
            signal_sent: false,
        }
    }

Then users could get the current behavior by calling:

let mut signal_sent = false;
Done::new(source, || 
        if !signal_sent {
            signal.fetch_sub(1, Ordering::Relaxed);
            signal_sent = true;
        }
)

To stress: this is just an idea I'm not sure if this is a better solution but I'd love to have your thoughts.

So what do you think:

• Would this allow a cleaner solution for this issue?
• Is such a Done source more useful then the one we have here?

[nieboczek]

I removed should_decrement as I also think it wouldn't be very useful.
Done now accepts a callback instead of an Arc<AtomicUsize> (breaking).
Also added Skippable::skipped that returns whether the skip has happened to achieve this fix easier.
skipped seems more useful than should_decrement though :)

[yara-blue]

skipped seems pretty useful yeah! :)

[yara-blue]

thanks for the great PR (great to have an upgrade guide entry too!)