pschatzmann / arduino-liblame

A simple mp3 encoder (not only) for Arduino using LAME
20 stars 3 forks source link

Memory is fragmented and leads to a crash, here is a fix: #2

Closed zackees closed 1 year ago

zackees commented 1 year ago

The USE_STACK_HACK #define allocates and free's huge blocks of data (always the same size) which the allocator seems to have problems with over time. If the microcontroller is using the encoder for just a single thread (this is the common use case) then the primebuff0 and primebuff1 buffers can just be recycled.

This is the fix I am using on my own code.

In config.h

// If you know the encoder will be used in a single threaded environment, you can use this hack to just
// recycle the memory. This will prevent memory fragmentation. Only use this if you are sure that the
// encoder will be called from a single thread.
#ifndef USE_STACK_HACK_RECYCLE_ALLOCATION_SINGLE_THREADED
#define USE_STACK_HACK_RECYCLE_ALLOCATION_SINGLE_THREADED 0
#endif

Then in encoder.c


static void
lame_encode_frame_init(lame_internal_flags *gfc, const sample_t *const inbuf[2])
{
    DEBUGF(gfc, __FUNCTION__);
    SessionConfig_t const *const cfg = &gfc->cfg;

    int ch, gr;

    if (gfc->lame_encode_frame_init == 0)
    {
#if USE_STACK_HACK
#if USE_STACK_HACK_RECYCLE_ALLOCATION_SINGLE_THREADED
        if (primebuff0 == NULL)
            primebuff0 = lame_calloc(sample_t, 286 + 1152 + 576);
        if (primebuff1 == NULL)
            primebuff1 = lame_calloc(sample_t, 286 + 1152 + 576);
#else
        primebuff0 = lame_calloc(sample_t, 286 + 1152 + 576);
        primebuff1 = lame_calloc(sample_t, 286 + 1152 + 576);
#endif
#else
        sample_t primebuff0[286 + 1152 + 576];
        sample_t primebuff1[286 + 1152 + 576];
        memset(primebuff0, 0, sizeof(primebuff0));
        memset(primebuff1, 0, sizeof(primebuff1));
#endif
        int const framesize = 576 * cfg->mode_gr;
        /* prime the MDCT/polyphase filterbank with a short block */
        int i, j;
        gfc->lame_encode_frame_init = 1;
        for (i = 0, j = 0; i < 286 + 576 * (1 + cfg->mode_gr); ++i)
        {
            if (i < framesize)
            {
                primebuff0[i] = 0;
                if (cfg->channels_out == 2)
                    primebuff1[i] = 0;
            }
            else
            {
                primebuff0[i] = inbuf[0][j];
                if (cfg->channels_out == 2)
                    primebuff1[i] = inbuf[1][j];
                ++j;
            }
        }
        /* polyphase filtering / mdct */
        for (gr = 0; gr < cfg->mode_gr; gr++)
        {
            for (ch = 0; ch < cfg->channels_out; ch++)
            {
                gfc->l3_side.tt[gr][ch].block_type = SHORT_TYPE;
            }
        }
        mdct_sub48(gfc, primebuff0, primebuff1);

        /* check FFT will not use a negative starting offset */
#if 576 < FFTOFFSET
#error FFTOFFSET greater than 576: FFT uses a negative offset
#endif

#if USE_STACK_HACK
#if USE_STACK_HACK_RECYCLE_ALLOCATION_SINGLE_THREADED
        memset(primebuff0, 0, sizeof(sample_t) * (286 + 1152 + 576));
        memset(primebuff1, 0, sizeof(sample_t) * (286 + 1152 + 576));
#else
        lame_free(primebuff0);
        lame_free(primebuff1);
        primebuff1 = NULL;
        primebuff0 = NULL;
#endif

#endif

        /* check if we have enough data for FFT */
        assert(gfc->sv_enc.mf_size >= (BLKSIZE + framesize - FFTOFFSET));
        /* check if we have enough data for polyphase filterbank */
        assert(gfc->sv_enc.mf_size >= (512 + framesize - 32));
    }
}
pschatzmann commented 1 year ago

Makes sense, can you submit this with a pull request ?

zackees commented 1 year ago

Yes, I can work on it later today.

zackees commented 1 year ago

It's now fixed with the recent pull request.