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

Missing points at the last buffer #55

Closed
bjkwon opened this issue Mar 20, 2019 · 5 comments
Closed

Missing points at the last buffer #55

bjkwon opened this issue Mar 20, 2019 · 5 comments

Comments

@bjkwon
Copy link

bjkwon commented Mar 20, 2019

To start off, thank you so much for the great tool, erikd!

But I have found an issue and, after some trials-and-errors, came up with a simple solution. I would like to share---

When src_process is called multiple times with a connected set of buffers, the last half_filter_chan_len points from the input buffer are not processed. In other words, you will lose these data points (which was 47 samples when I tried with SRC_SINC_MEDIUM_QUALITY, see below). This is because the while loop on line 375 in src_sinc.c breaks when filter->out_gen equals filter->out_count. Logically it makes sense, except that it should further generate output (half_filter_chan_len points more) for the very last buffer, because the processing of the very first buffer was done for filter->out_count MINUS half_filter_chan_len.

To resolve this issue, I am simply changing line 375 to while (1) and let the separate termination check do the work (lines 390 and 393). And I am making sure, in my application, to allocate the data_out buffer big enough to absorb this in addition to the expected output buffer size.

The same logic should apply to other functions--sinc_stereo_vari_process, sinc_quad_vari_process, sinc_hex_vari_process, and sinc_multichan_vari_process, but I'm not using them and I can't check this issue in these functions. I would let others do it. But this change works well for my application, auxlab, so I'm using my own hacked version of libsamplerate for auxlab for now (as of 3/20/2019).

The easiest way to replicate the issue is the following:

float * buffer_in = (float*)calloc(sizeof(float), 10000); // input buffer size is 10000 points
float * buffer_out = (float*)calloc(sizeof(float), 10000); // output is the same size; testing src_ratio = 1
// prepare the buffer
int errcode;
SRC_STATE* handle = src_new(SRC_SINC_MEDIUM_QUALITY, 1, &errcode);
SRC_DATA data;
data.src_ratio = 1;
data.data_in = buffer_in;
data.data_out = buffer_out;
data.input_frames = 5000; // half the buffer
data.output_frames = 5000; 
data.end_of_input = 0; 
errcode = src_process(handle, &data); // processing the first half
//after the call, 
//data.input_frames_used was 5000 but
//data.output_frames_gen was 4953, which is 5000 minus half_filter_chan_len in src_sinc.c
data.data_in = buffer_in + data.input_frames_used ;
data.data_out = buffer_out + data.output_frames_gen;
data.end_of_input = 1; 
errcode = src_process(handle, &data); // processing the second half
//Now you get 
//data.input_frames_used would be still 5000 (i.e., all input buffer has been read)
//data.output_frames_gen would be 5000. And, this is not right. We should have gotten 5047 points.

This hack will get you all the output data, without missing the last 47 points.
Hope this helps.

@erikd
Copy link
Member

erikd commented Mar 20, 2019

I assume this is the change you are talking about:

diff --git a/src/src_sinc.c b/src/src_sinc.c
index 6b465a7..6379ea0 100644
--- a/src/src_sinc.c
+++ b/src/src_sinc.c
@@ -372,7 +372,7 @@ sinc_mono_vari_process (SRC_PRIVATE *psrc, SRC_DATA *data)
        terminate = 1.0 / src_ratio + 1e-20 ;
 
        /* Main processing loop. */
-       while (filter->out_gen < filter->out_count)
+       while (1)
        {
                /* Need to reload buffer? */
                samples_in_hand = (filter->b_end - filter->b_current + filter->b_len) % filter->b_len ;

With that change, the test suite fails with a segfault.

@bjkwon
Copy link
Author

bjkwon commented Mar 20, 2019

Hmm... A quick fix is not a good fix, I guess.
Then, I would appreciate a proper way to address this issue.

Or, was there anything I missed? Perhaps was I calling src_process a wrong way? That is possible because I had to do a lot of guesswork while using it.

@erikd
Copy link
Member

erikd commented Mar 20, 2019

I am still unconvinced that there even is an issue. This is a widely used library, that has changed little in a decade and it has a comprehensive test suite.

The tests and the example code in this repo show how to use src_process.

@Ashymad
Copy link
Contributor

Ashymad commented Apr 3, 2019

@bjkwon I was having the same problem and it seems that you are supposed to call src_process one more time, after sending the last block of input data, with input_frames = 0. This returns the last few missing samples.

@erikd
Copy link
Member

erikd commented Apr 3, 2019

Yes, as @Ashymad says, you are supposed to keep calling src_proccess with zero input samples and the end_of_input field set to 1 until you get not more output samples.

The examples do this, for example examples/timewarp-file.c.

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

No branches or pull requests

3 participants