ralph-irving / squeezelite

Lightweight headless squeezebox player for Lyrion Media Server
https://sourceforge.net/projects/lmsclients/files/squeezelite/
Other
391 stars 98 forks source link

codec min space in OggFlac can cause dead lock #202

Closed philippe44 closed 10 months ago

philippe44 commented 10 months ago

When decoding flac, the min size is 16384. When such level is reached in streambuf, a decode for a single audio frame is made. That triggers the flac callback mechanism to call the following code to get data

static FLAC__StreamDecoderReadStatus read_cb(const FLAC__StreamDecoder *decoder, FLAC__byte buffer[], size_t *want, void *client_data) {
    size_t bytes;
    bool end;

    LOCK_S;
    bytes = min(_buf_used(streambuf), _buf_cont_read(streambuf));
    bytes = min(bytes, *want);
    end = (stream.state <= DISCONNECT && bytes == 0);

    memcpy(buffer, streambuf->readp, bytes);
    _buf_inc_readp(streambuf, bytes);
    UNLOCK_S;

    *want = bytes;

    return end ? FLAC__STREAM_DECODER_READ_STATUS_END_OF_STREAM : FLAC__STREAM_DECODER_READ_STATUS_CONTINUE;
}

This is fine as long as that callback returns enough data for flac to decode. It's unlikely to have frames of more than 16k. Now, should that happen, the callback is going to hammer us. It's fine on high CPU systems as the stream thread will find time to acquire more bytes and ultimately fill the streambuf, but that's not great. For lower CPU, that can be a deadlock

Now, I've observed that all the time for OggFlac and I'm pretty sure it's because the flac decoder tries to acquire a full Ogg page which easily can be larger than 16kB.

I've solved the issue by simply giving back CPU to the system in that callback

    if (!end && !bytes) usleep(100 * 1000);

I don't know if you want a PR as this is less likely to happen on typical squeezelite targets.

philippe44 commented 10 months ago

oops ... I did not see it was already there... my embedded forks are too old 😄