0
\$\begingroup\$

This question is a continuation of this.

Here, I modified the original code with the following changes.
1. Make a class Recorder() to store the information of audio data.

class Recorder {
    public:
        Recorder(uint32_t sample_rate, uint32_t rec_time);
        ~Recorder();
        uint32_t frame_step;
        uint32_t sample_rate;
        uint32_t step_buf_size;
        uint32_t num_sample;
        uint32_t sample_counter;
        uint32_t record_time;
        int16_t *total_buf;
        void append(int16_t *step_buf, uint32_t n);
    private:
};

Recorder::Recorder(uint32_t rate, uint32_t rec_time)
{
    sample_rate = rate;
    frame_step = sample_rate / 100;
    step_buf_size = sizeof(int16_t) * frame_step;
    num_sample = rec_time * sample_rate;
    record_time = rec_time;
    sample_counter = 0;
    total_buf = new int16_t[num_sample];
}

Recorder::~Recorder()
{
    delete[] total_buf;
}

void Recorder::append(int16_t *step_buf, uint32_t n)
{
    memcpy(&total_buf[sample_counter], step_buf, n);
    sample_counter += n;
}

2. Use the following code to save a *.wav file.

static int32_t DumpWav(char fname[], int16_t lpsSample[], uint32_t nNumSample, uint32_t nSampleRate)
{   // arrays passed to functions are converted to pointers.
    class WavFile wf;
    wf.Create(fname, 1, 16, nSampleRate);
    if (lpsSample != NULL && nNumSample > 0) {
        wf.Write((BYTE *)lpsSample, nNumSample * 2);
        wf.Flush();
    }
    return nNumSample;
}
void main(int argc, char *argv[])
{
    Recorder rec(44100, 10);
    class CWaveRecord oRecorder;

    // Here I have to use new, because It's not allow to use a variable to init array.
    // int16_t tmp_buf[sizeof(uint16_t) * rec.frame_step]; <-- this does not compile.
    int16_t *step_buf = new int16_t[sizeof(uint16_t) * rec.frame_step];

    // Do something to record audio
    oRecorder.Initialize(1, 16, rec.sample_rate, rec.step_buf_size, 300, NULL, 0); // 0.01 * 300 = 3s circular buffer
    char ch;
    while (1) {
        printf("Press any key to record , and press again to stop, 'q' to exit ...\nMaximum recording time is %d seconds.\n", rec.record_time);
        ch = _getch();
        if (ch == 'q' || ch == 'Q' || ch == 27)
            break;

        printf("Start Record ...\n");
        rec.sample_counter = 0;

        oRecorder.StartRecord();
        while (oRecorder.GetWave((unsigned char *)step_buf) == WR_ERROR_SUCCESS) {
            uint32_t n;
            n = oRecorder.GetSamplePerBuffer();
            if (rec.sample_counter < rec.num_sample) {
                //rec.append(step_buf, sizeof(uint16_t) * n); <-- This line doesn't work. Why?
                memcpy(&rec.total_buf[rec.sample_counter], step_buf, sizeof(uint16_t) * n);
                rec.sample_counter += n;
            } else {
                oRecorder.StopRecord();
                printf("Buffer is empty. Done\n");
                break;
            }

        }

        char lpszFileName[256];
        SYSTEMTIME time;
        GetLocalTime(&time);
        sprintf_s(lpszFileName, "cmd_%02d_%02d_%02d_%02d_%02d_%02d.wav", time.wMonth, time.wDay, time.wHour, time.wMinute, time.wSecond, 1);
        DumpWav(lpszFileName, rec.total_buf, rec.sample_counter, rec.sample_rate);
    }

    delete[] step_buf;
}

This block of code works on my PC, but I still have few questions.
1. Why is my Recorder::append not working?
2. I didn't use vector() to store my audio data. The reason is that I have to pass address of total_buf to memcpy() and DumpWav(). Is using vector possible here to replace array?
3. Do you see any other improvements of my code?
Thank you.

\$\endgroup\$
  • 2
    \$\begingroup\$ You might want to pick up a good, or at least reasonably decent C++ book. StackOverflow has a big list. I say this because you write void main, which is a clear sign that you have a poor book (it's int main() - different return type, and you're not using arc/argv) \$\endgroup\$ – MSalters Oct 20 '17 at 13:09
3
\$\begingroup\$

There are a lot of things here you should definitely improve on:

  1. Where are your includes? Your first file is missing the includes for cstdint and cstring, your second file for cstdint, cstdio, whatever header WavFile belongs to, the header for CWaveRecorder, for SYSTEMTIME and so on and so forth. It's a miracle that your code even compiles successfully.

  2. Everything from standard headers must be prefixed with std::. This applies to all the [u]int**_t types as well as to printf, memcpy, etc.

  3. class WavFile wf; is superfluous; Wavfile wf; will do fine. This is not C.

  4. Why do you bother with printf, _getch and the like? This is C++, and it has a standard library that offers a great range of very handy tools for such things, such as std::cout and std::cin. Don't reinvent the wheel. Again, this is not C.

  5. Don't use NULL, use nullptr. This is not C.

  6. Why does Recorder have a private: followed by nothing? Remove that.

  7. Why are all of Recorder's member variables public? This violates data encapsulation. Instead, provide getters and setters for data that should be accessible to the public.

  8. To answer the second question of yours: Yes, it is perfectly possible here to use std::vector and you should definitely opt to do so. First of all, std::vector provides a method named data() which allows access to the underlying array. Secondly, there is also std::copy, which you should prefer in almost all cases.


This answer is by far not comprehensive, but I suggest you go and rework your code, then come back for another review.

I am not going to sugarcoat it for you: Your code is horrible by C++ standards. You freely mix C legacy functions with code bearing only the slightest resemblance with what is called modern C++.

There is a middle ground between C and C++ which is called "C with classes" by most people. If you want to write code that's closer to C, you could consider sticking with that. Otherwise, I strongly recommend you to pick up a good book and learn how to write reasonably good C++ first.

\$\endgroup\$
0
\$\begingroup\$

std::vector<T> adds a lot of functionality to arrays, but at the core it's still a T[] array. In particular, &vec[n] == &vec[0] + n just like arrays. memcpy will therefore work. Having said that, C++ uses std::copy.

Other things: WavFile::Create is an oddity. In C++, objects are created by the constructor.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.