2
\$\begingroup\$

This code gets a list of signals with predefined frequency from struc.num

I need to encode it to wave format, so i use this code. This works, but i dont sure it is a correct decision to encode like this in python. Is there any ways to do it better?

def compiler():
    import out
    print("Loaded out.py")
    print("Compiling...")
    lst = list(my_code(out.code))
    print("Compiled!")
    print("Linking...")

    print("Bytes len: ", len(lst), " = ", blen * len(lst))

    music = wave.open('out.wav', 'w')
    music.setparams((2, 1, freq, 0, 'NONE', 'not compressed'))

    for i in lst:
        if (i == "0"):
            packed_value = wave.struct.pack('h', 0)
            for _ in range(100):
                music.writeframes(packed_value)
            continue

        key = i[0:i.find("(")]
        frame = Syntax.struc.num[int(key)]

        duration = 0.05
        samplerate = freq  # Hz
        samples = duration * samplerate
        frequency = frame #Hz
        period = samplerate / float(frequency)  # in sample points
        omega = N.pi * 2 / period

        xaxis = N.arange(int(period), dtype=N.float) * omega
        ydata = 16384 * N.sin(xaxis)

        signal = N.resize(ydata, 400) # 2-й параметр - скорость

        ssignal = b''
        for i in range(len(signal)):
            ssignal += wave.struct.pack('h', int(signal[i]))  # transform to binary

        music.writeframes(signal)

        #packed_value = wave.struct.pack('h', frame)
        #for _ in range(blen):
        #    music.writeframes(packed_value)
    music.close()

print("Linked to out.wav!")
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

It is hard to tell without knowing how lst is structured, but here is what I can say from what you have shown:

  1. It is safer and cleaner to open files with the with syntax, since it will safely close the file no matter what happens.
  2. It is better to do imports at the top of a module, rather than in a function.
  3. It is a convention to use ALL CAPS for module-level constants, which you appear to have several of here. This makes it easier to keep track of what is module-level and what isn't.
  4. You re-define certain constants every time through the loop. It is clearer and faster to define them once outside the loop.
  5. It is a convention to import numpy as np, rather than N.
  6. This would be much easier using scipy.io.wavfile if you can. I will assume for now that you can't, though.
  7. Although wave has a link to the struct module, it is, as far as I can tell, undocumented. Even if it was documented, it would be clearer to use the struct module directly.
  8. You potentially re-use the same struct value many times. It would probably be easier and quicker to define it once and re-use it.
  9. Along similar lines, you write the same value multiple times. Since it is just a bytes object, it would probably be easier and cleaner to just duplicate the value. They are small enough that this shouldn't be a problem.
  10. You use i for two different things in the same loop. Don't do that. You should use a more informative name for the first i.
  11. You only use a small bit of the i variable. You should probably strip that off at the beginning.
  12. Rather than finding a particular value in i and then slicing to that, you can just use i.partition('(') to split it at the first '(', then take the first value returned using i.partition('(')[0].
  13. You don't even want the key, you want the frame, or rather the frequency.
  14. You create variables only to immediately re-name them, then never use the original name. This doesn't have much effect on performance, but it is confusing.
  15. I don't know what my_code produces, but I will assume for my improvement it is an iterable. If it isn't, then just put the list() wrapper you have now back in.
  16. Is Syntax.struc.num a numpy array? If it is, then change freqs = np.array([Syntax.struc.num[key] if key else 0 for key in keys]) to freqs = Syntax.struc.num[keys].
  17. If Syntax.struc.num is already numeric, then you can drop the freqs = freqs.astype(np.float).
  18. If you define your frequencies in a numpy array at the beginning, you can also pre-calculate period and omega much faster in a single step rather than at every point in the loop.
  19. Try to avoid "magic numbers", which are seemingly-arbitrary numbers in your code. They should be named variables for clarity.
  20. You never use duration or samples.
  21. Numpy has a tobytes method that converts an array directly into binary.
  22. You create ssignal, but then write signal instead. If you can just write numbers, you don't need ssignal at all, right?
  23. I think it would be easier to read in the future if you use the music.set* functions rather than setting everything in one big function.
  24. Are you sure you want the max value to be 16384? 16 bit wave files can handle 32760.

So this is how I would structure your code, assuming you are not able or not willing to use scipy.

import struct
import wave

import numpy as np

import out


def compiler():
    print("Loaded out.py")
    print("Compiling...")
    lst = my_code(out.code)
    keys = (int(ilst.partition('(')[0]) for ilst in lst)
    freqs = np.array([Syntax.struc.num[key] if key else -1 for key in keys])
    freqs = freqs.astype(np.float)
    periods = FREQ/freqs
    omegas = np.pi * 2 / period
    periods = np.floor(periods)
    print("Compiled!")
    print("Linking...")

    print("Bytes len: {0} = {0} ".format(len(periods), BLEN*len(periods)))

    packed_value = np.zeros(100, dtype=np.int16).tobytes()

    maxamp = 2**14
    maxlen = 400
    nchan = 2
    sampwidth = 1
    nframes = 0

    with wave.open('out.wav', 'w') as music:
        music.setnchannels(nchan)
        music.setsampwidth(sampwidth)
        music.setframerate(FREQ)

        for period, omega in zip(periods, omegas):
            if period < 0:
                music.writeframes(packed_value)
                continue

            signal = maxamp * np.sin(np.arange(period)*omega).
            signal = np.resize(signal, maxlen)  # duplicate up to the correct length
            signal = signal.astype(np.int16).tobytes()  # transform to binary

            music.writeframes(signal)

print("Linked to out.wav!")
\$\endgroup\$
4
  • \$\begingroup\$ According to PEP 8, you should use all caps for constants; there is no special naming convention for global variables. \$\endgroup\$ Jul 1, 2016 at 21:27
  • \$\begingroup\$ Sorry, you are right. I meant module-level constants. Fixed. \$\endgroup\$ Jul 1, 2016 at 21:29
  • \$\begingroup\$ Thanks. I did not know that scipy has a wave module \$\endgroup\$ Jul 1, 2016 at 21:39
  • \$\begingroup\$ I made some minor fixups to the code. Please see the edit history. \$\endgroup\$ Jul 2, 2016 at 5:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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