Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FLAC decoder falls into an infinite read loop if ogg stream serialno changes #568

Open
alxvasilev opened this issue Mar 15, 2023 · 7 comments · May be fixed by #735
Open

FLAC decoder falls into an infinite read loop if ogg stream serialno changes #568

alxvasilev opened this issue Mar 15, 2023 · 7 comments · May be fixed by #735

Comments

@alxvasilev
Copy link

alxvasilev commented Mar 15, 2023

With some ogg/FLAC online radios, the stream serialno changes with each song. This causes the decoder, at the start of the next song, to enter an infinite loop of calling the read callback and discarding the data, not outputting any decoded data and not returning from FLAC__stream_decoder_process_single() until the read callback returns an ABORT. Even if the callback returns FLAC__STREAM_DECODER_READ_STATUS_END_OF_STREAM, the infinite loop is not aborted.
Commenting out the line in framing.c in libogg:
if(serialno!=os->serialno)return(-1);
solves the issue for my use case.

@ktmf01
Copy link
Collaborator

ktmf01 commented Mar 20, 2023

Is it possible for you to supply such a file, or point to radio station that streams such files?

@alxvasilev
Copy link
Author

This is one such radio:
http://198.204.228.202:8157/flac
I can provide a sample as well, but FLAC files are quite large, so only if needed.

@ktmf01
Copy link
Collaborator

ktmf01 commented Mar 21, 2023

Thanks for providing the stream. This seems related to #254

Can you provide some more information on how you are using libFLAC? It seems you're not using the flac command line tool here, so I need some more details.

@alxvasilev
Copy link
Author

alxvasilev commented Mar 21, 2023

Yes, this seems exactly the #254 issue.
I am using the libFLAC library in an application, so the commandline tool I haven't even compiled. The actual usage is in an embedded application for the ESP32 microcontroller, but I built a linux test app with similar usage of libFLAC just to confirm the issue is not related to the environment. The way I'm using it is:
create decoder: FLAC__stream_decoder_new()
init decoder for ogg stream and attach callbacks: FLAC__stream_decoder_init_ogg_stream()
repeatedly call FLAC__stream_decoder_process_single()
I can attach the source code of the linux test program if needed.

@alxvasilev
Copy link
Author

alxvasilev commented Mar 21, 2023

Here is the code of the test app. Note that it's suited for the radio link I posted - the pulseaudio output is configured for 24-bit 96 kHz. You can also disable audio output by commenting-out ENABLE_PA, which will still illustrate the issue

#include <stdio.h>
#include <FLAC/stream_decoder.h>
#include <errno.h>
#include <string.h>
#include <memory>
#include <alloca.h>
#define ENABLE_PA 1
// g++ -o player ./flac-test.cpp -lpulse -lpulse-simple -lFLAC -lm -g
#if ENABLE_PA
#include <pulse/simple.h>
#include <pulse/error.h>

struct paFreeDeleter {
    void operator()(pa_simple* device) const { if (device) pa_simple_free(device); }
};
typedef std::unique_ptr<pa_simple, paFreeDeleter> paDeviceUptr;
paDeviceUptr paDevice;
#endif

FLAC__StreamDecoder* decoder = nullptr;
FILE* inFile = nullptr;
int inOffs = 0;
long millis()
{
    struct timespec tv;
    clock_gettime(CLOCK_REALTIME, &tv);
    return tv.tv_sec * 1000 + tv.tv_nsec / 1e6;
}
int nReads = 0;
FLAC__StreamDecoderReadStatus readCb(const FLAC__StreamDecoder *decoder, FLAC__byte buffer[], size_t *bytes, void* userp)
{
    printf("readCb\n");
    if (++nReads > 20) {
        printf("Too many reads without producing output\n");
    }
    int nread = fread(buffer, 1, *bytes, inFile);
    printf("[%ld] readCb(%zu) -> 0x%x + %d\n", millis(), *bytes, inOffs, nread);

    if (nread > 0) {
        *bytes = nread;
        inOffs += nread;
        return FLAC__STREAM_DECODER_READ_STATUS_CONTINUE;
    }
    *bytes = 0;
    return (nread == 0) ? FLAC__STREAM_DECODER_READ_STATUS_ABORT : FLAC__STREAM_DECODER_READ_STATUS_ABORT;
}
void errorCb(const FLAC__StreamDecoder *decoder, FLAC__StreamDecoderErrorStatus status, void *client_data)
{
    printf("FLAC decode error: %s(%d)", FLAC__StreamDecoderErrorStatusString[status], status);
}
void metadataCb(const FLAC__StreamDecoder *decoder, const FLAC__StreamMetadata *metadata, void *client_data)
{
    printf("FLAC metadata\n");
}

FLAC__StreamDecoderWriteStatus writeCb(const FLAC__StreamDecoder *decoder,
    const FLAC__Frame* frame, const FLAC__int32* const buffer[], void *userp)
{
    int nSamples = frame->header.blocksize;
    size_t audioLen = nSamples * 8;
    int32_t* audioBuf = (int32_t*)alloca(audioLen);
    auto wptr = audioBuf;
    for (int i = 0; i< nSamples; i++) {
        *(wptr++) = buffer[0][i] << 8;
        *(wptr++) = buffer[1][i] << 8;
    }
#if ENABLE_PA
    int error;
    if (pa_simple_write(paDevice.get(), audioBuf, audioLen, &error) < 0) {
        fprintf(stderr, "pa_simple_write() failed: %s\n", pa_strerror(error));
        return FLAC__STREAM_DECODER_WRITE_STATUS_ABORT;
    }
#endif
    return FLAC__STREAM_DECODER_WRITE_STATUS_CONTINUE;
}

int main(int argc, char* argv[])
{
    if (argc < 2) {
        printf("Usage: %s <file>\n", argv[0]);
        return 1;
    }
    inFile = fopen(argv[1], "r");
    if (!inFile) {
        printf("Error opening input file: %s\n", strerror(errno));
        return 2;
    }
#if ENABLE_PA
    int error;
    static const pa_sample_spec ss = { .format = PA_SAMPLE_S32LE, .rate = 96000, .channels = 2 };
    paDevice.reset(pa_simple_new(NULL, "FLAC player", PA_STREAM_PLAYBACK, NULL, "playback", &ss, NULL, NULL, &error));
    if (!paDevice) {
        printf("pa_simple_new() failed\n");
        return 255;
    }
#endif
    decoder = FLAC__stream_decoder_new();
    auto ret = FLAC__stream_decoder_init_ogg_stream(decoder, readCb, nullptr, nullptr, nullptr, nullptr, writeCb, metadataCb, errorCb, nullptr);
    if (ret != FLAC__STREAM_DECODER_INIT_STATUS_OK) {
        printf("decoder init failed: %d\n", ret);
        return 1;
    }

    for (;;) {
        nReads = 0;
        auto ok = FLAC__stream_decoder_process_single(decoder);
        printf("decoder returned %d\n", ok);
        if (!ok) {
            auto err = FLAC__stream_decoder_get_state(decoder);
            const char* errStr = (err >= 0) ? FLAC__StreamDecoderStateString[err] : "(invalid code)";
            printf("Decoder returned error %s(%d)\n", errStr, err);
            return 3;
        }
    }
}

@ktmf01
Copy link
Collaborator

ktmf01 commented Mar 21, 2023

Getting this fixed properly will take a lot of work, which has been on my TODO list for more than a year now. There is a PR (#261) implementing basis chaining support, but that doesn't involve seeking, metadata handling etc. It will also require quite a few changes to the API.

So, the hack to libogg you mention will have to do for quite some time for your own use case I'm afraid.

@alxvasilev
Copy link
Author

No problem, thanks a lot for the quick response. I hope people who implement FLAC radio streaming will find this thread and not waste too much time solving the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants