Replacing float2mozzy.py with new audio2mozzi.py #295
Conversation
leobel96
changed the title
|
As mentioned in the commit message: |
|
Haven't try but looks good to me at first glance! One thing that might be worth adding as an option would be to allow to make the table "symmetrical" (see #280 ), which, I think Audacity is not doing per default but which can be made easily with your script? |
Uhm, I guess that you are talking about Oscil wavetables here, not Sample ones, right? |
|
Principally yes, but can also be relevant for samples? (This allows to negate them). |
|
And with "symmetrical" you don't actually mean that the first half of the table has to be identical to the specular of the second part, right? You only mean that the last value has to be identical to the first one to not have a gap and avoid weird artifacts. So you just want me to append a tail of cells to the table that gradually connects the last cell to the first one. Did I understand everything correctly? |
|
Ah, sorry for the confusion, I think symmetrical I maybe not the right word. By that I mean more a symmetrical range, so that the samples go from -127 to 127 and not to 128. That does not the sound but that kind simplify things sometimes... That's maybe nitpicking... For the start and end point, I think it makes sense that they start at 0 to avoid a click, but I guess that's more the job of the one cutting the samples than your script.... |
Uhm, how can they go to 128 if they are |
|
Yes, sorry for the typo and the confusion. |
|
So is the whole thing just a matter of replacing all the -128 with -127? That's it? 😂 |
|
Well, that's one way to do it ;). But a cleaner way would be to renormalize the whole signal in that range (to avoid saturation/hard clip) around -127. But I actually think that it is probably nearly equivalent when starting with 8bits… (For instance, you can check how I did it for some of the existing tables here (second code snip). |
|
Ah! This makes sense, your snippet makes everything more clear. I can add this bit in the next few days and amend the commit above |
|
Sometimes a bit of code is better than a full speech ;). Sorry for the confusion |
|
Hey! I finally found some time to add that symmetrical part. Do you mind testing the script and, if it works as expected, merge it? @tomcombriat |
|
Nice! I'll have a look at it very soon! Thanks for this |
| cnt_33 = 0 | ||
| else: | ||
| cnt_33 = 0 | ||
| if args.make_symmetrical: |
There was a problem hiding this comment.
I think this block should read:
if args.make_symmetrical:
value_to_remove = -max_output_value
if value_to_remove in out_values:
in_min = value_to_remove
in_max = max_output_value
out_max = max_output_value
out_min = -max_output_value+1
out_values = (int((value - in_min) * (out_max - out_min) / (in_max - in_min) + out_min) for value in out_values)in the current version: value_to_remove = -max_output_value - 1 is -128-1=-129 for 8bits, hence the test is never passed. out_min should be set to -127 in that case to return the correct rounded value. Also note the conversion back to int :)
There was a problem hiding this comment.
Everything should be nicer now 🙂
extras/python/audio2mozzi.py
Outdated
| out_values.append( | ||
| math.trunc((value * max_output_value) + 0.5) | ||
| if args.input_encoding == "float" | ||
| else value |
There was a problem hiding this comment.
Shouldn't some kind of normalization happen here in case of 16bits as input? From my tests, inputting a 16bits and expecting an 8bits output produces some way out of range values.
There was a problem hiding this comment.
Yeah, you are right, I didn't test it properly. Using some help from Numpy 😄
| parser.add_argument( | ||
| "-e", | ||
| "--input-encoding", | ||
| choices=("float", "int"), |
There was a problem hiding this comment.
In float mode, with default number of output bits, some values reach 128 (which do not fit). Maybe you should discard my comment on the make symmetric but subtract -1 to the max output value?
There was a problem hiding this comment.
uhm, I didn't really understand this. Do you mean that the output of the script reaches 128? That's not expected
extras/python/audio2mozzi.py
Outdated
| @@ -0,0 +1,167 @@ | |||
| #!/usr/bin/env python3 | |||
| """ | |||
| For converting raw audio files to Mozzi table | |||
There was a problem hiding this comment.
Maybe add some copyright info (you or Mozzi team and you depending on how much you drew from the original script) and a license (LGPL and GPL are already in Mozzi)?
extras/python/audio2mozzi.py
Outdated
| for value in in_values: | ||
| cnt_33 = 0 | ||
| out_values.append( | ||
| math.trunc((value * max_output_value) + 0.5) |
There was a problem hiding this comment.
In conjunction with my other comments on the output range, might be easier to use round instead of trunc?
There was a problem hiding this comment.
(Note that I did not look at the original script, where some of the rough edges I found might already be ;).) The script is very easy to use (and made me discovered ArgumentParser… I was still doing that manually…)
There was a problem hiding this comment.
Yeah, this trunc thing was already present in the original script and confused me too. I wanted to keep this script as close as possible to the original. I think that round would work better too. Fixed in the new version.
extras/python/audio2mozzi.py
Outdated
| fout.write(table) | ||
| fout.write("\n\n") | ||
| fout.write(f"#endif /* {tablename}_H_ */\n") | ||
| print(f"wrote {table} to {output_path}") |
There was a problem hiding this comment.
I would personally remove the print statements that, IMO, clutters the terminal.
There was a problem hiding this comment.
(or maybe as a --verbose argument?
There was a problem hiding this comment.
Okay, added a --verbose argument for this
The script allows to create mozzi tables from raw files and it's supposed to replace all the "*2mozzy.py" scripts thanks to its high configurability (it also supports int16/int32 tables in case they'll be supported in the future)
|
Thanks! Give me a few days to test it :) |
There was a problem hiding this comment.
Sorry for the delay!
Works like a charm when converting to the same number of bits, but I think still fails a bit (pun unintended) when having different number of bits between input and output (that should work right?):
- when scaling down (from 16 to 8 say) the output is very weird, looks like an overflow from the l.120.
- when scaling up (from 8 to 16 say), the values are not changed (they stay in the 8 bits range)
- when using --make-symmetrical, the values in the output files are floating points (even when saying 8 bits as an output)..
Here are the files I use for testing (simple sine wave, generated in Audacity, 1s at 8192sampling rate):
raws.zip
| else: | ||
| raise Exception("Unrecognised --input-bits value") | ||
|
|
||
| output_array = input_array.astype(output_dtype) |
There was a problem hiding this comment.
I think this conversion should happen after. If you start with a 16 bits raw file, and want to convert it to 8bits, this line provokes an overflow, hence the values in output_array are not valid anymore…
I think, if I were to do it, I would probably make everything float, scale to the final output range, knowing the input range (or finding it actually, as long as you know how they are encoded) (your map_value seems to be made for that), and convert everything back in the output type at the very end (and think which "downscaler" to use… round? floor?). This might benefit from the higher resolution of the scaling done in float.
But maybe I understood you script wrong?
There was a problem hiding this comment.
Adding on this, I see now that your mapping is not used when make_symmetrical is false. astype does not change the values, but just reinterpret them to the new type (100 as int8 will stay 100 as int16).
| elif args.output_bits == 32: | ||
| output_dtype = np.int32 | ||
| else: | ||
| raise Exception("Unrecognised --input-bits value") |
There was a problem hiding this comment.
Nitpick: I think this should read "--output-bits"
| max_value_to_normalize = np.iinfo(output_dtype).max | ||
| min_final_value = np.iinfo(output_dtype).min + 1 | ||
| max_final_value = max_value_to_normalize | ||
| if min_value_to_normalize in output_array: |
There was a problem hiding this comment.
I do not understand this line actually… When changing types (or working with non-normalized samples) this if is not necessary true, even though the values still need to be rescaled no?
No description provided.