Запись музыки на компьютер в WAV-файл в C

Смотрите следующий итерация .

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

#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>

/******************************
*  Magic file format strings. *
******************************/
const char fChunkID[]     = {'R', 'I', 'F', 'F'};
const char fFormat[]      = {'W', 'A', 'V', 'E'};
const char fSubchunk1ID[] = {'f', 'm', 't', ' '};
const char fSubchunk2ID[] = {'d', 'a', 't', 'a'};

/********************************
* WriteWavePCM() configuration: *
* - 2 channels,                 *
* - frequency 44100 Hz.         *
********************************/
const unsigned short N_CHANNELS = 2;
const unsigned int SAMPLE_RATE = 44100;
const unsigned short BITS_PER_BYTE = 8;

bool WriteWavePCM(short* sound, size_t pairAmount, char* fileName){
    const static unsigned int fSubchunk1Size = 16;
    const static unsigned short fAudioFormat = 1;
    const static unsigned short fBitsPerSample = 16;

    unsigned int fByteRate = SAMPLE_RATE * N_CHANNELS *
                             fBitsPerSample / BITS_PER_BYTE;

    unsigned short fBlockAlign = N_CHANNELS * fBitsPerSample / BITS_PER_BYTE;
    unsigned int fSubchunk2Size;
    unsigned int fChunkSize;

    FILE* fout;
    size_t ws;

    if (!sound || !fileName || !(fout = fopen( fileName, "w" ))) return false;

    fSubchunk2Size = pairAmount * N_CHANNELS * fBitsPerSample / BITS_PER_BYTE;
    fChunkSize = 36 + fSubchunk2Size;

    // Writing the RIFF header:
    fwrite(&fChunkID, 1, sizeof(fChunkID),      fout);
    fwrite(&fChunkSize,  sizeof(fChunkSize), 1, fout);
    fwrite(&fFormat, 1,  sizeof(fFormat),       fout);

    // "fmt" chunk:
    fwrite(&fSubchunk1ID, 1, sizeof(fSubchunk1ID),      fout);
    fwrite(&fSubchunk1Size,  sizeof(fSubchunk1Size), 1, fout);
    fwrite(&fAudioFormat,    sizeof(fAudioFormat),   1, fout);
    fwrite(&N_CHANNELS,      sizeof(N_CHANNELS),     1, fout);
    fwrite(&SAMPLE_RATE,     sizeof(SAMPLE_RATE),    1, fout);
    fwrite(&fByteRate,       sizeof(fByteRate),      1, fout);
    fwrite(&fBlockAlign,     sizeof(fBlockAlign),    1, fout);
    fwrite(&fBitsPerSample,  sizeof(fBitsPerSample), 1, fout);

    /* "data" chunk: */
    fwrite(&fSubchunk2ID, 1, sizeof(fSubchunk2ID),      fout);
    fwrite(&fSubchunk2Size,  sizeof(fSubchunk2Size), 1, fout);

    /* sound data: */
    ws = fwrite(sound, sizeof(short), pairAmount * N_CHANNELS, fout);
    fclose(fout);
    return true;
}

/************************************
* Around 23 seconds of pure techno! *
************************************/
const unsigned int N_SAMPLE_PAIRS = 1048576;

int main(int argc, char* argv[]){
    short* sound;
    int i;
    int j;
    bool status;
    char* file_name;

    sound = (short*) malloc( sizeof(short) * N_SAMPLE_PAIRS * N_CHANNELS );

    if (!sound)
    {
        puts("Could not allocate space for the sound data.");
        return (EXIT_FAILURE);
    }

    for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += 2, j++ )
    {
        short datum1 = 450 * ((j >> 9 | j >> 7 | j >> 2) % 128);
        short datum2 = 450 * ((j >> 11 | j >> 8 | j >> 3) % 128);

        sound[i]     = datum1; // One channel.
        sound[i + 1] = datum2; // Another channel.
    }

    file_name = argc > 1 ? argv[1] : "Default.wav";
    status = WriteWavePCM(sound, N_SAMPLE_PAIRS, file_name);
    free(sound);

    if (status)
    {
        printf("Discotheque is ready in \"%s\"\n", file_name);
    }
    else
    {
        puts( "Something seems to have gone wrong." );
        return (EXIT_FAILURE);
    }

    return 0;
}

(Что происходит с воспроизведением сгенерированного файла, VLC и QuickTime Player способны его воспроизвести, не уверены в других медиаплеерах.)

40 голосов | спросил coderodde 21 stEurope/Moscowp30Europe/Moscow09bEurope/MoscowMon, 21 Sep 2015 18:20:28 +0300 2015, 18:20:28

5 ответов


16

Порядок байтов

Ваша программа работает только на маленьких конечных машинах. Если вы хотите, чтобы это работало на любой машине, вам нужно было бы исправить все вызовы fwrite() для вывода специально в маленьком концевом формате.

Использовать struct для заголовка

Я бы предложил создать структуру для заголовка файла. Таким образом, вы можете заполнить все поля (в правильном порядке), а затем сделать один единственный fwrite(). Из кода видно, что вы можете объединить все поля в один заголовок, но если имеет смысл иметь несколько заголовков (заголовок RIFF, заголовок форматирования, заголовок данных и т. Д.).

ответил JS1 22 ndEurope/Moscowp30Europe/Moscow09bEurope/MoscowTue, 22 Sep 2015 01:33:43 +0300 2015, 01:33:43
15

fopen должен иметь b во втором параметре. Это прежде всего предотвращает замену окон на любой \n байт с помощью \r\n, который повредит файл.

fopen( fileName, "wb" )

Сделайте порядок параметров fwrite согласованным сначала sizeof, а затем количество элементов:

// Writing the RIFF header:
fwrite(&fChunkID,    sizeof(fChunkID),   1, fout);
fwrite(&fChunkSize,  sizeof(fChunkSize), 1, fout);
fwrite(&fFormat,     sizeof(fFormat),    1, fout);

Говоря о размерах, размер стандартных примитивов не гарантируется на всех платформах одинаково (long). Вместо этого используйте целые типы из stdint.h.

ответил ratchet freak 21 stEurope/Moscowp30Europe/Moscow09bEurope/MoscowMon, 21 Sep 2015 18:51:06 +0300 2015, 18:51:06
11

На самом деле это выглядит не так уж плохо. Некоторые незначительные nitpicks

  1. Вы не должны указывать возвращаемое значение malloc .

  2. fByteRate и fBlockAlign может быть const, поскольку они вычисляются только из значений const .

  3. Я не совсем уверен, почему почти все переменные в WriteWavePCM имеют префикс f. Я предполагаю, что это означает, что они связаны с данными, записываемыми в файл. Поскольку большинство кода в этом методе связано с написанием вещей в файл, это кажется излишним для меня и немного отвлекающим, но YMMV.

  4. Здесь используется магическая константа fChunkSize = 36 + fSubchunk2Size; - заменить именованную константу (рассчитывается или определяется - зависит от того, откуда приходит 36).

  5. fBitsPerSample определяется как 16, который предполагает, что short равен 16 бит (поскольку он связан с записью данных short* sound в файл). Это может быть не так на всех платформах. Таким образом, fBitsPerSample должен быть либо sizeof(*sound), либо передан.

  6. Вы используете непоследовательный стиль привязки:

    int main(int argc, char* argv[]){
    

    против

    if (!sound)
    {
    
  7. Вы определили N_CHANNELS, но основной цикл жестко закодирован для увеличения на 2, поэтому этот

    for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += 2, j++ )
    

    должен быть

    for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += N_CHANNELS, j++)
    
ответил ChrisWue 21 stEurope/Moscowp30Europe/Moscow09bEurope/MoscowMon, 21 Sep 2015 23:13:27 +0300 2015, 23:13:27
9

В дополнение к превосходным советам, приведенным в других ответах, я бы рекомендовал полностью пересмотреть этот цикл:

for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += 2, j++ )
{
    short datum1 = 450 * ((j >> 9 | j >> 7 | j >> 2) % 128);
    short datum2 = 450 * ((j >> 11 | j >> 8 | j >> 3) % 128);

    sound[i]     = datum1; // One channel.
    sound[i + 1] = datum2; // Another channel.
}

У вас есть 9 магических констант. Они должны иметь разумные имена, чтобы описать, что они делают.

Всякий раз, когда вы видите это:

    sound[i]     = datum1; // One channel.
    sound[i + 1] = datum2; // Another channel.

это указание, что вам действительно нужен struct. Вы должны определить структуру, которая содержит 2 части данных вместо массива, где вы должны знать, что каждая другая часть данных является одним каналом. Что-то вроде:

struct AudioSample {
    int16_t leftChannel;
    int16_t rightChannel;
};

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

ответил user1118321 22 ndEurope/Moscowp30Europe/Moscow09bEurope/MoscowTue, 22 Sep 2015 06:55:19 +0300 2015, 06:55:19
8

Нет причин собирать много условий в одном и том же if:

if (!sound || !fileName || !(fout = fopen( fileName, "w" ))) 

Вполне может быть:

if (sound == NULL || fileName == NULL) {
    return false;
} 

FILE * fout = fopen(fileName, "wb");
if (fout == NULL) {
    return false
} 

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

Вы также выполняете эти проверки, которые могут прекратить выполнение функции после нескольких присваиваний переменных, которые теряются, если параметры являются нулевыми или файл не может быть открыт. Неважно, но, тем не менее, субоптимально. C99 позволяет объявлять переменные в любом месте, поэтому их можно переместить в начало функции.


Здесь вы назначаете ws:

ws = fwrite(sound, sizeof(short), pairAmount * N_CHANNELS, fout);

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


Мне нравится держать глобальную область как можно более ясной, поэтому я бы переместил все константы, которые используются только внутри функции wav записи в области функций. Вы можете сделать их static константами, чтобы гарантировать, что они не перераспределяются в стеке каждый раз.

ответил glampert 21 stEurope/Moscowp30Europe/Moscow09bEurope/MoscowMon, 21 Sep 2015 22:42:09 +0300 2015, 22:42:09

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

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

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