Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: Ability to have much less samples #104

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Patrykot
Copy link

@Patrykot Patrykot commented Nov 16, 2019

At this moment I'm not shure how to write tests for it.
Feature will give you the the possibility to set maximum number of pixels on the output files. In order to achive it we need to take number of all frames (frame is block of samples, one for each channel) and devide it by desired number of pixels. We will receive then samples per pixel. But since samples per pixel is integer, higher values of desired number of pixels can get the SAME number of samples per pixel. Consider this example, based on test_file_mono.wav:
It has 113519 of frames. Let's say i want to 376 samples.

samples_per_pixel1 = static_cast<int>(frames_count_ / pixels_count_ +
            ((frames_count_ % pixels_count_) > 0 ? 1 : 0)); // =302 double is 301.9122

Now, let's say i want to 377 samples.

samples_per_pixel2 = static_cast<int>(frames_count_ / pixels_count_ +
            ((frames_count_ % pixels_count_) > 0 ? 1 : 0)); // ALSO =302 !! double is 301.1114

Thus we are able to set precisly pixels count till the diferrence between these to samples_per_pixel1 - samples_per_pixel2 is greater than 1. The higher the pixels-count the more often you will encounter lower values than given the maximum.

This is kind of a remedy for issue: #41

@chrisn
Copy link
Member

chrisn commented Nov 17, 2019

Thanks! I'll take a look.

static std::unique_ptr<ScaleFactor> createScaleFactor(const Options& options)
// Returns the frame count of a given audio. Frame is block of samples, one for each channel.

static std::pair<bool, long> getFrameCount(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have a getDuration function that does the same job, only returning duration in seconds, rather than frames. I'd prefer not to duplicate code where possible.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what you suggest?

  1. Remove getFrameCount
  2. Replace usage of "FrameCount" with duration*sample_rate?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking to modify getDuration to return frames instead of seconds, then divide by sample rate where we need seconds

@chrisn
Copy link
Member

chrisn commented Nov 20, 2019

But since samples per pixel is integer, higher values of desired number of pixels can get the SAME number of samples per pixel.

This is true, and means that it's not currently possible to achieve the exact length of waveform you specify, due to the rounding error. This issue affects the PNG output, where the image size is as specified on the command line, the actual waveform width may not exactly fill the image.

@chrisn chrisn mentioned this pull request Feb 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants