Запись звука непрерывно на C

Как продолжающийся небольшой мой проект, я работал над записью звука в . Вы можете прогрессировать код, просматривая предыдущие версии ( V1.0 , V2.0 ).

Я обнаружил, что мои прошлые версии были слишком ограниченными. Они требовали определенного времени, за которое они записывались, а затем останавливались. Это не очень практично для многих приложений реального мира, поэтому я переписал все, чтобы компьютер всегда слушал и записывал только необходимые данные.

Вот что я хотел бы рассмотреть:

  • Потребление памяти . Очевидно, что огромная потенциальная проблема - это много места, которое потребляется при записи и сохранении аудиофайла. Я вообще теряю пространство? Каждый маленький бит. Примечание: контейнер хранения файлов должен быть FLAC.

  • Скорость . Я должен обрабатывать данные в режиме реального времени . Я не могу ни на что винить, или я могу потерять драгоценные аудиоданные. Есть ли где-нибудь ускорить обработку?

  • Синтаксис /стиль . Как выглядит мой код? Что с этим может показаться лучше? Все, что я делаю неправильно, синтаксически?

Не стесняйтесь пересматривать и другие вещи, я бы хотел, чтобы обзоры были сосредоточены на вышеупомянутом. Имейте в виду, что я немного ушел от языка C, поэтому я, возможно, забыл некоторые из более простых вещей;)


main.c

#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#include <string.h>
#include <time.h>
#include <portaudio.h>
#include <sndfile.h>

#define FRAMES_PER_BUFFER 1024

typedef struct
{
    uint16_t formatType;
    uint8_t numberOfChannels;
    uint32_t sampleRate;
    size_t size;
    float *recordedSamples;
} AudioData;

typedef struct
{
    float *snippet;
    size_t size;
} AudioSnippet;

AudioData initAudioData(uint32_t sampleRate, uint16_t channels, int type)
{
    AudioData data;
    data.formatType = type;
    data.numberOfChannels = channels;
    data.sampleRate = sampleRate;
    data.size = 0;
    data.recordedSamples = NULL;
    return data;
}

float avg(float *data, size_t length)
{
    float sum = 0;
    for (size_t i = 0; i < length; i++)
    {
        sum += fabs(*(data + i));
    }
    return (float) sum / length;
}

long storeFLAC(AudioData *data, const char *fileName)
{

    uint8_t err = SF_ERR_NO_ERROR;
    SF_INFO sfinfo =
    {
        .channels = data->numberOfChannels,
        .samplerate = data->sampleRate,
        .format = SF_FORMAT_FLAC | SF_FORMAT_PCM_16
    };

    SNDFILE *outfile = sf_open(fileName, SFM_WRITE, &sfinfo);
    if (!outfile) return -1;

    // Write the entire buffer to the file
    long wr = sf_writef_float(outfile, data->recordedSamples, data->size / sizeof(float));
    err = data->size - wr;

    // Force write to disk and close file
    sf_write_sync(outfile);
    sf_close(outfile);
    puts("Wrote to file!!!!");
    return err;
}

int main(void)
{
    PaError err = paNoError;
    if((err = Pa_Initialize())) goto done;
    const PaDeviceInfo *info = Pa_GetDeviceInfo(Pa_GetDefaultInputDevice());
    AudioData data = initAudioData(44100, info->maxInputChannels, paFloat32);
    AudioSnippet sampleBlock =
    {
        .snippet = NULL,
        .size = FRAMES_PER_BUFFER * sizeof(float) * data.numberOfChannels
    };
    PaStream *stream = NULL;
    sampleBlock.snippet = malloc(sampleBlock.size);
    time_t talking = 0;
    time_t silence = 0;
    PaStreamParameters inputParameters =
    {
        .device = Pa_GetDefaultInputDevice(),
        .channelCount = data.numberOfChannels,
        .sampleFormat = data.formatType,
        .suggestedLatency = info->defaultHighInputLatency,
        .hostApiSpecificStreamInfo = NULL
    };

    if((err = Pa_OpenStream(&stream, &inputParameters, NULL, data.sampleRate, FRAMES_PER_BUFFER, paClipOff, NULL, NULL))) goto done;
    if((err = Pa_StartStream(stream))) goto done;
    for(int i = 0;;)
    {
        err = Pa_ReadStream(stream, sampleBlock.snippet, FRAMES_PER_BUFFER);
        if (err) goto done;
        else if(avg(sampleBlock.snippet, FRAMES_PER_BUFFER) > 0.000550) // talking
        {
            printf("You're talking! %d\n", i);
            i++;
            time(&talking);
            data.recordedSamples = realloc(data.recordedSamples, sampleBlock.size * i);
            data.size = sampleBlock.size * i;
            if (data.recordedSamples) memcpy((char*)data.recordedSamples + ((i - 1) * sampleBlock.size), sampleBlock.snippet, sampleBlock.size);
            else
            {
                free(data.recordedSamples);
                data.recordedSamples = NULL;
                data.size = 0;
            }
        }
        else //silence
        {
            double test = difftime(time(&silence), talking);
            if (test >= 1.5 && test <= 10)
            {
                char buffer[100];
                snprintf(buffer, 100, "file:%d.flac", i);
                storeFLAC(&data, buffer);
                talking = 0;
                free(data.recordedSamples);
                data.recordedSamples = NULL;
                data.size = 0;
            }
        }
    }

done:
    free(sampleBlock.snippet);
    Pa_Terminate();
    return err;
}

Скомпилируйте с помощью: gcc main.c -lsndfile -lportaudio (если у вас установлены эти библиотеки)

35 голосов | спросил syb0rg 20 MaramFri, 20 Mar 2015 03:05:41 +03002015-03-20T03:05:41+03:0003 2015, 03:05:41

5 ответов


28

Действительно, вы, должно быть, некоторое время работали над этим проектом, так как я некоторое время отвечал на вопрос о SO, связанный с одной из ваших старых версий. По крайней мере, некоторые из отзывов, которые у меня были, по-прежнему актуальны и сейчас.

Комментарии к коду и стилю

  1. #include <stdio.h>
    #include <stdlib.h>
    #include <math.h>
    #include <string.h>
    #include <time.h>
    #include <portaudio.h>
    #include <sndfile.h>
    

    Проблема начинается уже здесь. Вы сильно используете uint32_t и другие подобные типы, без #include <stdint.h>. Я предполагаю, что это работает, потому что вы, вероятно, все еще находитесь в Mac OS X, и это также объясняет, почему ваш GCC не жалуется на начальные объявления цикла C99 без -std=gnu99.

  2. Как упоминалось ранее, вы делаете ненужное использование арифметики указателя, что делает код трудным для чтения. Я определенно предпочитаю версию вашего среднего @ nhgrif ; Я нахожу sum += fabs(data[i]); значительно проще на глазу.

  3. Функция avg() имеет как минимум несколько других проблем. Во-первых, если вы хотите вычислить, когда кто-то говорит, я бы не делал прямого среднего, даже с трюком вроде fabs(). Вместо этого я бы использовал RMS (Root-Mean-Square), а затем вычитал из RMS среднее значение ( без fabs()) стоимость; это вычисляет аудио " power " в буфере, игнорируя любое смещение постоянного тока (отличное от нуля), которое не поддерживает ваш код. Это напрямую связано с количеством децибел в вашем образце по тому же закону мощности, который используется для « compand » ввода микрофона (это может быть А-закон или U-закон, или, может быть, что-то еще ).

    Во-вторых, и связанные; При 1024 кадрах и частоте дискретизации 44100 Гц ваши буферы охватывают период всего 23 миллисекунды; Другими словами, только один полный цикл звука с частотой 43 Гц. Я не знаю, как идет низкий голос @ rob0t, но вы можете увеличить эту сумму, если будете иметь дело с низкочастотным контентом. При вычислении RMS и DC (в среднем) вы можете получить только точный результат, если вы точно фиксируете целое число циклов для интересующих вас частот; Но если ваш буфер охватывает достаточно циклов всех интересующих вас частот, любая неточность, введенная частичными циклами, будет заглушаться полными в среднем.

    В-третьих, ваш буфер имеет 1024 кадра, а аккумулятор - float. Я подозреваю, что здесь вас не волнует точность, но вам может быть интересно узнать, что, поскольку арифметика с плавающей запятой IEEE не совсем похожа на реальную математику, порядок, в котором добавляются значения с плавающей запятой, может иметь небольшую разницу. В частности, если вы должны добавить 1024 одинаковых значения, вы потеряете около 10 из 24 бит точности при ваших последних скоплениях. Это связано с тем, что накопитель увеличится до 2 ^ 10-кратного размера накопителей, и добавление в аккумулятор будет использовать только 14 MSB при округлении 10 LSB. Я бы использовал double здесь, если бы мне не было доказано, что avg() является узким местом производительности и что векторизация уже сделала все возможное.

    В-четвертых, наконец,

      

    return (float) sum / length;

    полностью избыточен: sum уже является float, и вы снова добавляете его в float. Если вы хотите выполнить это разделение с использованием математики с плавающей запятой, это уже гарантировано; В C значения с плавающей запятой превосходят любое целое число в неявных преобразованиях. Помните, что броски не применяются к операциям (/), а к их операнду (здесь sum).

  4. storeFLAC() не регистрируется в случае ошибок, поэтому у меня нет немедленных объяснений относительно того, почему ваш код, построенный на Linux, преуспевает только при создании файлов с нулевой длиной. Код ошибки, который он возвращает, полностью игнорируется.

    SNDFILE *outfile = sf_open(fileName, SFM_WRITE, &sfinfo);
    if (!outfile) return -1;
    
    // Write the entire buffer to the file
    

    И из-за отсутствия брекетов вокруг ранних return операторов, рекрутерам сложнее добавить свой собственный код отладки, потому что это гораздо больше хлопот, чтобы добавить фигурные скобки поверх отладки printf(). Приятно быть внимательным к вашим рецензентам, включая ваше будущее.

  5. main() страдает необходимость рефакторинга, в частности, извлечение нескольких функций для каждого основного шага. Код, который вы написали, не «говорит» мне; Я должен приложить некоторые усилия, чтобы понять, что он делает. По крайней мере, этопримерно экранный, который, на мой взгляд, хороший размер для функции.

  6. Это грубое злоупотребление goto:

    int main(void)
    {
      PaError err = paNoError;
      if((err = Pa_Initialize())) goto done;
      const PaDeviceInfo *info = Pa_GetDeviceInfo(Pa_GetDefaultInputDevice());
      AudioData data = initAudioData(44100, info->maxInputChannels, paFloat32);
      AudioSnippet sampleBlock =
      {
          .snippet = NULL,
          .size = FRAMES_PER_BUFFER * sizeof(float) * data.numberOfChannels
      };
      ...
    
      done:
      free(sampleBlock.snippet);
      Pa_Terminate();
      return err;
    }
    

    В то время как я одобряю goto для обработки ошибок, вы здесь совершили грех , перепрыгивая через инициализацию локальных переменных , а затем потрудившись использовать их. Ничего не гарантирует, что sampleBlock.snippet инициализируется при попытке выполнить free() его: вы даже не выполнили инициализацию sampleBlock, если PortAudio не удалось инициализировать!

    Это ошибка, которую вы обнаружили бы, если бы вы использовали флаги GCC и Clang -Wall -Wextra, чтобы задать много полезной диагностики. Об этой ошибке сообщалось как

    portaudio.c: In function ‘main’:
    portaudio.c:135:6: warning: ‘sampleBlock.snippet’ may be used uninitialized in this function [-Wmaybe-uninitialized]
      free(sampleBlock.snippet);
          ^
    

    . Обратите внимание, что это предупреждение появляется только при включенной оптимизации ; Это связано с тем, что анализ проходит GCC требует, чтобы обнаружить, что такие неинициализированные использования выполняются только в -O1 и выше. Таким образом, может быть полезно, чтобы вы регулярно собирались на высоких уровнях оптимизации, где GCC прилагает больше усилий для анализа и может, поскольку побочный эффект обнаруживает потенциальные ошибки.

  7. Эта строка:

    if((err = Pa_OpenStream(&stream, &inputParameters, NULL, data.sampleRate, FRAMES_PER_BUFFER, paClipOff, NULL, NULL))) goto done;
    

    Совершенно очевидно, что если вам нужно прокрутить это право, вы значительно превысили мой 80-ти столбцовый предел. Вы также не поместили фигурные скобки вокруг своего утверждения, и здесь был бы полезен хороший отладочный printf. Я, конечно, хочу знать, не открылся ли мой поток.

    Для функций с большим количеством аргументов с длинными именами я склоняю их по одному на строку. Не бойтесь тратить на них несколько строк.

  8. Поздравляем вас с использованием difftime() для переносимости и безопасного определения временных разниц. Я кое-что узнал. Однако time_t обычно определяется как целое число секунд с момента Epoch. Поскольку вы будете звонить time() около 43 раз в секунду, многие последовательные вызовы будут давать одно и то же время и, следовательно, разницу в 0 s, тогда как некоторые вызовы будут давать разница 1 s, несмотря на то, что она действительно разделена только на 23 ms.

    Лучше здесь было бы использовать таймер настенных часов с более высоким разрешением. Для этого типа времени я советую gettimeofday(); Для экстремальных разрешений я рекомендую clock_gettime() в Linux или гораздо более простой в использовании и более низкий служебный mach_absolute_time() в Mac OS X.

  9. Это безумие:

    data.recordedSamples = realloc(data.recordedSamples, sampleBlock.size * i);
    data.size = sampleBlock.size * i;
    if (data.recordedSamples) memcpy((char*)data.recordedSamples + ((i - 1) * sampleBlock.size), sampleBlock.snippet, sampleBlock.size);
    else
    {
      free(data.recordedSamples);
      data.recordedSamples = NULL;
      data.size = 0;
    }
    

    Во-первых, одна из ваших строк здесь слишком длинная. Вы использовали испорченный x = realloc(x, ...) idiom, что ужасно, так как если realloc() не работает, вы будете утечка памяти.

    Кроме того, вы являетесь memcpy(), добавляя новый блок данных в свой вектор изменения размера homebrew, поверх любого memcpy() внутри realloc(). Оставляя в стороне тот факт, что каждые 23 мс речи вы потенциально realloc() и скопируете весь массив данных, собранных до сих пор , я серьезно спрашиваю о необходимости даже перемещения любые данные вообще. Возможно, вам удастся создать себе двойной буфер или, в более общем плане, круговой буфер и вообще не копировать.

    И затем, вдобавок ко всему, снова посмотрите на свое условное утверждение. Если data.recordedSamples действительно NULL (и, таким образом, вы уже пропустили память, потеряв последнюю ссылку на этот блок памяти), почему вы free() 'указатель NULL, а затем установить указатель NULL на NULL снова ? Это совершенно неэффективно.

Дизайнкомментарии

  1. В вашем коде отсутствует общая документация. Было бы полезно узнать тонкости определенных вещей. Например, я могу быстро считать, что вы записываете только формат float, так как ваши недокументированные структуры используют только float*; Но что такое образец, фрагмент или кадр, согласно вам или PortAudio? Какова схема чередования каналов, если таковая имеется? В чем принципиальное отличие между AudioSnippet и AudioData, который заставил вас создать для них другую структуру?

  2. Учитывая, что вы делаете акцент на том, чтобы вообще не отбрасывать аудиоданные, я советую вам сделать это приложение заправленным. Один поток будет вести чтение данных в круговой буфер и будет выполнять задачи с незначительной вычислительной сложностью (предположительно, RMS займет достаточно мало времени, чтобы никогда не быть узким местом). Если этот поток обнаруживает речь, он может сообщить рабочий поток, чтобы начать чтение этого буфера с заданным смещением и закодировать его в файл FLAC. Конечно, если рабочий поток слишком медленный, в конечном итоге поток чтения будет исчерпывать циклический буфер и ему придется блокировать, возможно, отбрасывать кадры.

ответил Iwillnotexist Idonotexist 21 MaramSat, 21 Mar 2015 07:29:03 +03002015-03-21T07:29:03+03:0007 2015, 07:29:03
15

Если он идет как массив и говорит как массив, используйте его как массив.

float avg(float *data, size_t length)
{
    float sum = 0;
    for (size_t i = 0; i < length; i++)
    {
        sum += fabs(*(data + i));
    }
    return (float) sum / length;
}

Итак, data - это массив, не так ли? Почему мы не рассматриваем его как один и не получаем его с помощью индексов?

float avg(float *data, size_t length)
{
    float sum = 0;
    for (size_t i = 0; i < length; ++i)
    {
        sum += fabs(data[i]);
    }
    return (float) sum / length;
}

И я не знаю C достаточно хорошо, чтобы сказать наверняка, но это отличное выражение в строке return кажется ненужным (sum уже является float


Не обманывайте свои бессердечные 1-строчные if операторы.

if (data.recordedSamples) memcpy((char*)data.recordedSamples + ((i - 1) * sampleBlock.size), sampleBlock.snippet, sampleBlock.size);

В то время как я в порядке, с очень простыми однострочными операторами, находящимися на одной и той же линии и бессвязными, это довольно смешно. Это одна строка, потому что вы обманули и набросали слишком много материала.

if (data.recordedSamples)
{
    size_t skipIndex = (i - 1) * sampleBlock.size;
    char *destination = (char*)data.recordedSamples + skipIndex;
    memcpy(destination, sampleBlock.snippet, sampleBlock.size);
}

Это намного лучше (хотя, по общему признанию, имена переменных, которые я выбрал, могут не иметь смысла).


Он называется main(), а не everything()

У нас есть надстройка main(). И это, в конечном счете, привело нас к редкому сценарию, когда goto на самом деле является довольно простым способом организации кода внутри функции.

Я еще раз скажу: это на самом деле относительно чистый способ использования инструкции goto. Немедленная альтернатива - это глубоко вложенные выражения if, которые действительно не чисты вообще.

Но есть лучший подход, который делает код более чистым и обладает бонусом побочных эффектов для удаления goto все вместе: не так сильно вписывается в main()

Сделайте еще несколько функций. Вызовите свои функции. И пусть у ваших функций есть ранний return s.

Функции помогают читателю сравнивать куски вашего кода. Вы отлично справились с функцией avg. «AVG» является общей аббревиатурой для среднего. Я могу догадаться только именем функции, что она делает, и мне не нужно читать через 6 простых строк логики, чтобы понять, что она делает. Все, что мне нужно сделать, это просто увидеть, что вы вызываете avg(), и я знаю, что происходит.

Итак, найдите еще несколько способов разбить и разделить ваш код на более усвояемые фрагменты, чтобы прочитанные могли просто увидеть имя функции и получить суть происходящего здесь, без необходимости видеть каждую строку исполняемого кода.

ответил nhgrif 20 MaramFri, 20 Mar 2015 04:02:41 +03002015-03-20T04:02:41+03:0004 2015, 04:02:41
11

Обзор дизайна

Непрерывное перераспределение data.recordedSamples звучит не так. В конечном итоге это может закончиться неудачей, потому что вам требуется не просто много памяти, но много непрерывной памяти. Подумайте хотя бы о повторном использовании data.recordedSamples вместо его освобождения. Для этого потребуется всего один счетчик (сколько блоков он может в настоящее время размещать) и значительно сократить количество перераспределений.

Также кажется, что вы хотите сохранить записанные сэмплы в непрерывном пространстве, чтобы иметь возможность fwrite их в одном вызове. Я не думаю, что сокращение количества вызовов записи покупает любую производительность. Большая часть времени тратится на передачу данных и не зависит от количества вызовов. Фактически, большие записи делают профиль выполнения менее плавным: вместо постоянно тратить мало время, ваша программа иногда тратит много em> время. Прекрасная возможность пропустить важное звуковое событие.

Стандартным подходом к таким проблемам является двойная буферизация (я полагаю, что запись буфера происходит быстрее, чем запись) по строкам:

    while (keep_going) {
        read_buffer(buf1);
        swap(buf1, buf2);
        write_nonblock(buf2);
    }

Обзор кода

  • 0.000550 - это странно выглядящее магическое число. const float talking_threshold возможно? В любом случае значение требует некоторого обоснования.

  • Вы записываете некоторое молчание. Я также рекомендую записать некоторые результаты. В противном случае вы можете пропустить несколько «говорящих» образцов в самом начале события.

  • avg - отличный кандидат на ускорение. Рассмотрим аппаратное ускорение.

  • goto абсолютно необоснован.

ответил vnp 20 MaramFri, 20 Mar 2015 04:17:04 +03002015-03-20T04:17:04+03:0004 2015, 04:17:04
11

При условии, что условия часто замалчиваются как плохая практика ... и по какой-то причине. Почти во всех ситуациях, когда используется goto, это происходит потому, что вы слишком много делаете в методе. Рассмотрим ваш основной метод:

int main(void)
{
    PaError err = paNoError;
    if((err = Pa_Initialize())) goto done;

    .....

    AudioData data = initAudioData(44100, info->maxInputChannels, paFloat32);
    AudioSnippet sampleBlock =
    {
        .snippet = NULL,
        .size = FRAMES_PER_BUFFER * sizeof(float) * data.numberOfChannels
    };
    PaStream *stream = NULL;
    sampleBlock.snippet = malloc(sampleBlock.size);


    .....

    if((err = Pa_OpenStream(&stream, &inputParameters, NULL, data.sampleRate, FRAMES_PER_BUFFER, paClipOff, NULL, NULL))) goto done;
    if((err = Pa_StartStream(stream))) goto done;
    for(int i = 0;;)
    {
        err = Pa_ReadStream(stream, sampleBlock.snippet, FRAMES_PER_BUFFER);
        if (err) goto done;

        ........

    }

done:
    free(sampleBlock.snippet);
    Pa_Terminate();
    return err;
}

Все это можно сделать просто:

int main(void)
{
    time_t talking = 0;
    time_t silence = 0;

    PaStream *stream = NULL;
    PaError err = setup_stream(stream);
    while (!err) {
        err = process_stream(stream);
    }
    free_stream(stream);
    return err;
}

Затем ваш различный метод может просто return err; в них, если есть проблема:

int setup_stream(PaStream stream) {
    PaError err = paNoError;
    err = Pa_Initialize();
    if (err) {
        return err;
    }
    const PaDeviceInfo *info = Pa_GetDeviceInfo(Pa_GetDefaultInputDevice());
    AudioData data = initAudioData(44100, info->maxInputChannels, paFloat32);
    AudioSnippet sampleBlock =
    {
        .snippet = NULL,
        .size = FRAMES_PER_BUFFER * sizeof(float) * data.numberOfChannels
    };

    sampleBlock.snippet = malloc(sampleBlock.size);
    PaStreamParameters inputParameters =
    {
        .device = Pa_GetDefaultInputDevice(),
        .channelCount = data.numberOfChannels,
        .sampleFormat = data.formatType,
        .suggestedLatency = info->defaultHighInputLatency,
        .hostApiSpecificStreamInfo = NULL
    };

    err = Pa_OpenStream(&stream, &inputParameters, NULL, data.sampleRate, FRAMES_PER_BUFFER, paClipOff, NULL, NULL);
    if (err) {
        return err;
    }
    return Pa_StartStream(stream);

}

Даже этот метод кажется слишком длинным. Процесс создания одних и тех же данных проблематичен.

Кроме того, я, вероятно, испортил входной stream параметр ... он должен быть типом указателя, который указывает на вновь созданный поток при успешном завершении ...

ответил rolfl 20 MaramFri, 20 Mar 2015 05:29:05 +03002015-03-20T05:29:05+03:0005 2015, 05:29:05
10

Во-первых, вы не используете фигурные скобки вокруг однострочных if операторов:

if (!outfile) return -1;

Неспособность использовать фигурные скобки была причиной ошибки SSL от Apple, как описано здесь .


Зачем ты это делаешь?

#define FRAMES_PER_BUFFER 1024

Не должно быть static const int FRAMES_PER_BUFFER = 1024;?

Этот вопрос SO обсуждает это.


Как упоминалось в комментарии @RubberDuck, у вас очень много логики в main(). Вероятно, это должно быть делегировано отдельному методу или нескольким методам. main() не должен содержать кучу логики, но должен использоваться больше как точка входа в программу.


Помимо этих пунктов, это один из самых чистых, опрятных кодов, которые я видел. Вы использовали пробелы вокруг своих операторов, и ваш код обычно выглядит аккуратно.

ответил Hosch250 20 MaramFri, 20 Mar 2015 03:43:26 +03002015-03-20T03:43:26+03:0003 2015, 03:43:26

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132