Skip to main content
make quoted code quoted
Source Link
rolfl
  • 98.1k
  • 17
  • 220
  • 419

In addition to the excellent advice given in the other answers, I'd recommend completely overhauling this loop:

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.
}
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.
}

You have 9 magical constants in there. They should have sensible names to describe what they do.

Whenever you see this:

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

it's an indication that you really wanted a struct. You should define a structure that contains 2 pieces of data instead of an array where you have to know that every other piece of data is one channel. Something like:

struct AudioSample {
    int16_t leftChannel;
    int16_t rightChannel;
};

Or whatever's appropriate for your use case. That would also allow you to write the loop with only a single control variable, since i and j would now be equal.

In addition to the excellent advice given in the other answers, I'd recommend completely overhauling this loop:

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.
}

You have 9 magical constants in there. They should have sensible names to describe what they do.

Whenever you see this:

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

it's an indication that you really wanted a struct. You should define a structure that contains 2 pieces of data instead of an array where you have to know that every other piece of data is one channel. Something like:

struct AudioSample {
    int16_t leftChannel;
    int16_t rightChannel;
};

Or whatever's appropriate for your use case. That would also allow you to write the loop with only a single control variable, since i and j would now be equal.

In addition to the excellent advice given in the other answers, I'd recommend completely overhauling this loop:

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.
}

You have 9 magical constants in there. They should have sensible names to describe what they do.

Whenever you see this:

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

it's an indication that you really wanted a struct. You should define a structure that contains 2 pieces of data instead of an array where you have to know that every other piece of data is one channel. Something like:

struct AudioSample {
    int16_t leftChannel;
    int16_t rightChannel;
};

Or whatever's appropriate for your use case. That would also allow you to write the loop with only a single control variable, since i and j would now be equal.

Source Link
user1118321
  • 12k
  • 1
  • 20
  • 46

In addition to the excellent advice given in the other answers, I'd recommend completely overhauling this loop:

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.
}

You have 9 magical constants in there. They should have sensible names to describe what they do.

Whenever you see this:

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

it's an indication that you really wanted a struct. You should define a structure that contains 2 pieces of data instead of an array where you have to know that every other piece of data is one channel. Something like:

struct AudioSample {
    int16_t leftChannel;
    int16_t rightChannel;
};

Or whatever's appropriate for your use case. That would also allow you to write the loop with only a single control variable, since i and j would now be equal.