Add wave bars customization options

[YannickGagnon]

Addressing issue 111

• Add bar width (pixels)
• Add bar gap (pixels)
• Add bar style rounded (true/false)

FYI: I did not implement waveform-style, it's simply determined by the bar width option

./audiowaveform -i ./path/to/audio.mp3 -o ./path/to/audio.png -w 896 -h 184 --no-axis-labels --background-color "00000000" --waveform-color EF5350FF --bar-width 8 --bar-gap 4 --bar-style-rounded true --pixels-per-second 5

Output

[chrisn]

Thanks! I'll try this out

[chrisn]

I needed to fix a couple of issues to get this to compile, and couldn't push to your branch, so here's my branch: https://github.com/bbc/audiowaveform/tree/bar-customisation-for-png

Here's an example, using these options:

audiowaveform -i test/data/test_file_stereo.mp3 --bar-width 8 --bar-gap 4 -o test.png

I'm wondering if the bar heights should be such that the image is symmetrical?

[YannickGagnon]

I needed to fix a couple of issues to get this to compile, and couldn't push to your branch, so here's my branch: https://github.com/bbc/audiowaveform/tree/bar-customisation-for-png

Here's an example, using these options:

audiowaveform -i test/data/test_file_stereo.mp3 --bar-width 8 --bar-gap 4 -o test.png

I'm wondering if the bar heights should be such that the image is symmetrical?

I didn't even see it at first but now I do as well. I'll fix it so that it is symmetrical.

Regarding the fact that it wasn't compiling, strangely enough it was for me on my Mac using the Xcode compiler. I'll merge your changes.

[chrisn]

In addition to PNGs, audiowaveform also generates .json or .dat files containing the raw waveform data. So I'm thinking whether it should be possible to save the raw data so that you can render the image using some other software.

This would mean moving the calculation of the data points outside GdImageRenderer. We already have the code to do this in WaveformRescaler - i.e., downsample the audio to a very high number of samples per pixel, then have two separate rendering methods in GdImageRenderer, one for waveform style and one for bars style.

[YannickGagnon]

In addition to PNGs, audiowaveform also generates .json or .dat files containing the raw waveform data. So I'm thinking whether it should be possible to save the raw data so that you can render the image using some other software.

This would mean moving the calculation of the data points outside GdImageRenderer. We already have the code to do this in WaveformRescaler - i.e., downsample the audio to a very high number of samples per pixel, then have two separate rendering methods in GdImageRenderer, one for waveform style and one for bars style.

In all honesty, I'm no serious c++ developer, I've done some like 25 years ago. Nevertheless I'm still keen to help out with this feature.

As to the current state of the PR, I've simply copied the previous bar PR request and fiddled my way to make some tweaks and managed to monkey my way around adding the CLI params (options).

What you're suggesting here seems like a great idea, but I would need help or serious guidance 😅

Also, after looking at the actual GdImageRenderer, it seems that the unsymmetrical issue isn't a new thing, it just had been amplified by the addition of the bar-gaps.

Let me know what you think, meanwhile, I can fix the bar on the horizontal axis.

[chrisn]

Having thought a bit more, the approach in this PR seems good. I pushed couple more changes to my branch.

One thing I noticed is that the bar width is actually one pixel too big, so if you specify --bar-width 8, you actually get 9 pixel bars.

[chrisn]

Also, if you compare these images, the waveform should look the same, just scrolled to the left by 0.5 seconds, but they look very different.

With --start 0 (default)

With --start 0.5

[chrisn]

Also, after looking at the actual GdImageRenderer, it seems that the unsymmetrical issue isn't a new thing, it just had been amplified by the addition of the bar-gaps.

This is true, and I don't want to change that, as it gives a more accurate representation of the waveform. See this example.

[chrisn]

I'm working on a solution...

[YannickGagnon]

Also, after looking at the actual GdImageRenderer, it seems that the unsymmetrical issue isn't a new thing, it just had been amplified by the addition of the bar-gaps.

This is true, and I don't want to change that, as it gives a more accurate representation of the waveform. See this example.

I guess we could update the logic to be different here, when using bar-style we draw them to be symmetrical, what do you think?

[YannickGagnon]

@chrisn I pushed some changes which fix the bar width as well has the radius issues

Only the symmetrical issue remains if that is something we wish to address of course

[YannickGagnon]

Also, if you compare these images, the waveform should look the same, just scrolled to the left by 0.5 seconds, but they look very different.

With --start 0 (default)

With --start 0.5

Honestly, I have no idea what causes this, surely it's a bug related to the sampling of the data prior to rendering.
Any ideas?

[chrisn]

Could you enable the "Allow edits from maintainers" option on your pull request please? I just fixed the test cases to get the code to compile so would like to update this PR with that change.

I also notice lots of compiler warnings about float to int conversion, e.g:

audiowaveform/src/GdImageRenderer.cpp:313:43: warning: conversion from ‘double’ to ‘int’ may change value
  [-Wfloat-conversion]
  313 |         gdImageFilledRectangle(image_, x1 + rad, y1, x2 - rad, y1 + rad, waveform_color_);
      |                                        ~~~^~~~~

[chrisn]

Honestly, I have no idea what causes this, surely it's a bug related to the sampling of the data prior to rendering.
Any ideas?

It's because the code assumes a bar should start at the first pixel in the image, which may not be the case if the start time is not zero.

[YannickGagnon]

Could you enable the "Allow edits from maintainers" option on your pull request please? I just fixed the test cases to get the code to compile so would like to update this PR with that change.

I also notice lots of compiler warnings about float to int conversion, e.g:

audiowaveform/src/GdImageRenderer.cpp:313:43: warning: conversion from ‘double’ to ‘int’ may change value
  [-Wfloat-conversion]
  313 |         gdImageFilledRectangle(image_, x1 + rad, y1, x2 - rad, y1 + rad, waveform_color_);
      |                                        ~~~^~~~~

I found this documentation from GitHub but couldn't find the proper interface. I have no idea if it's related to where I saved my fork (my business GitHub company)

You have any idea?

True, I'll create variables to prevent the conversion warnings

[chrisn]

From this discussion, it seems it's not possible from an organization account. I didn't realise.

[chrisn]

It's because the code assumes a bar should start at the first pixel in the image, which may not be the case if the start time is not zero.

I'm not sure what the right thing to do here is. If you're using the --start option, maybe you expect the first bar to start on the first pixel? But then, if you generate a sequence of images each with different --start options you may expect the waveform to look the same, just shifted left or right, even if that means the first bar isn't fully visible or there's a gap before the first bar.

I'm happy write the code to try out both alternatives to see how well each works.

[YannickGagnon]

From this discussion, it seems it's not possible from an organization account. I didn't realise.

If that's fine with you, you can still push to your branch and I'll rebase from your branch and you do the same.

[YannickGagnon]

It's because the code assumes a bar should start at the first pixel in the image, which may not be the case if the start time is not zero.

I'm not sure what the right thing to do here is. If you're using the --start option, maybe you expect the first bar to start on the first pixel? But then, if you generate a sequence of images each with different --start options you may expect the waveform to look the same, just shifted left or right, even if that means the first bar isn't fully visible or there's a gap before the first bar.

I'm happy write the code to try out both alternatives to see how well each works.

Looking forward to it

[YannickGagnon]

@chrisn I just pushed the 2 things we discussed namely fix the implicit conversion warnings and center the bars for waveform bar style

Two things that could be upgraded:

1. Option --bar-style-rounded (bool) could be upgraded to a String value to support more than two styles
2. Option --waveform-style-bars (bool) could also be upgraded to a String to support more than two styles

[chrisn]

Thanks! I agree with your suggestions, using string values is more flexible.

[YannickGagnon]

Thanks! I agree with your suggestions, using string values is more flexible.

I couldn't figure it out quickly so I went with the path of least resistance hahaha

Is this something you could work out? Or at least tell me how to implement string params the most effective way?

[chrisn]

The --colors option might be a good example to copy, see the code here.

[chrisn]

@YannickGagnon I have added this feature to the master branch. Please try it out and let me know what you think.

[YannickGagnon]

Thanks @chrisn for moving forward with this PR, I've been busy with something else meanwhile.
You've made some nice improvements to the code 💪🏻

There's a small typo in the help, it states that --bar-style is either square or rounded, but implementation is square or round

Also, I'm seeing some differences in the output though and I don't know if it's expected or not 🤔

Here's are the different outputs from my PR and then from Master using the same input file and arguments

[chrisn]

Thanks. I'll fix the typo. I also need to add some test cases, and maybe update how the parameter validation is done.

I'll check your branch again, but here's a test that compares the current 1.7.1 release with the latest code on master, with the --waveform-style bars option:

audiowaveform -i test_file_stereo.mp3 -o waveform-z64.png -z 64

audiowaveform -i test_file_stereo.mp3 -o bars-z64.png -z 64 --waveform-style bars

[YannickGagnon]

Thanks. I'll fix the typo. I also need to add some test cases, and maybe update how the parameter validation is done.

I'll check your branch again, but here's a test that compares the current 1.7.1 release with the latest code on master, with the --waveform-style bars option:

audiowaveform -i test_file_stereo.mp3 -o waveform-z64.png -z 64

audiowaveform -i test_file_stereo.mp3 -o bars-z64.png -z 64 --waveform-style bars

The problem seems to appear when you set the width and height parameter, can you confirm?

[chrisn]

I think the --width and --height options are OK. Here's another example, using the -z auto option:

audiowaveform -i test_file_stereo.mp3 -o waveform-zauto.png -z auto --width 600 --height 100

audiowaveform -i test_file_stereo.mp3 -o bars-zauto.png -z auto --waveform-style bars --width 600 --height 100

[YannickGagnon]

I think the --width and --height options are OK. Here's another example, using the -z auto option:

audiowaveform -i test_file_stereo.mp3 -o waveform-zauto.png -z auto --width 600 --height 100

audiowaveform -i test_file_stereo.mp3 -o bars-zauto.png -z auto --waveform-style bars --width 600 --height 100

You're right, if I set -z auto, it works thanks

[YannickGagnon]

@chrisn Any idea when this gets released? Assuming it will be available in upcoming version 1.7.2?

[chrisn]

Yes, it'll be a new version, probably 1.8.0. The only thing remaining is to add some more test cases. I can't promise exactly when it'll be ready to release (this is all in my personal time) ... but shouldn't be long.

[chrisn]

I have now released 1.8.0, see https://github.com/bbc/audiowaveform/releases/tag/1.8.0. Many thanks for your help! I suggest that we close this PR and please raise issues if anything needs improvement or fixing.

[YannickGagnon]

Thanks to you, wonderful work you've been doing there ✨