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

Wrong number of frames in pipe-write mode #77

Closed
mgeier opened this issue Sep 18, 2014 · 9 comments
Closed

Wrong number of frames in pipe-write mode #77

mgeier opened this issue Sep 18, 2014 · 9 comments
Assignees

Comments

@mgeier
Copy link

mgeier commented Sep 18, 2014

When opening a named FIFO for writing, I'd expect the number of frames to be reported as 0 (like when opening a normal file in SFM_WRITE mode).

When I tried it, however, frames was -13.

This is some code to reproduce it:

#include <sndfile.h>
int main() {
  SNDFILE* sf;
  SF_INFO sfinfo;
  sfinfo.samplerate = 44100;
  sfinfo.channels = 1;
  sfinfo.format = SF_FORMAT_AU | SF_FORMAT_PCM_16;
  sf = sf_open("myfifo", SFM_WRITE, &sfinfo);
  printf("%d\n", sfinfo.frames);
  sf_close(sf);
}

Before starting this, a named FIFO has to be created and something has to read from it, e.g. like this:

mkfifo myfifo
cat myfifo

Is the negative value intentional?
Does it mean something?

@erikd
Copy link
Member

erikd commented Sep 18, 2014

This is not a libsndfile problem, this is a C problem.

If you ran your program under Valgrind (which I highly recommand) then Valgrind would complain that sfinfo struct has un-initialized values. To understand what that is, you should google "C automatic variable un-initialized".

The fix is to add:

memset (&sfinfo, 0, sizoef (sfinfo));

before the line

sfinfo.samplerate = 44100;

Does that make sense?

@mgeier
Copy link
Author

mgeier commented Sep 19, 2014

Thanks for the quick reply, but I'm sorry to say that it doesn't really make sense to me.

I'd expect libsndfile to set sfinfo.frames always (and never read from it before that), so it shouldn't matter if it's initialized or not.

Anyway, I had tried already to set sfinfo.frames = 0; before opening, this didn't change anything.
I just tried your suggestion with memset(), which - as expected - didn't change anything either.
The return value of -13 is not the result of some uninitialized memory in the SF_INFO struct, it seems to be actively set by libsndfile.

I had a quick look in the source:

In src/common.c:1383 in psf_decode_frame_count() you return SF_COUNT_MAX.
Similarly, in src/sndfile.c:2721 there is psf->filelength = SF_COUNT_MAX.
In src/file_io.c there is a pipeoffset mentioned a few time, maybe that has something to do with it?

So what's the intended behavior?

Should frames be 0 in the case of my example?
Or -1? ... or SF_COUNT_MAX ...?

@erikd
Copy link
Member

erikd commented Sep 19, 2014

I'd expect libsndfile to set sfinfo.frames always

What should libsndfile set it to?

The intended behaviour is that it works correctly. You have a value that you think in anomolous. I still don't see any big here.

If you want me to look at this further I need a repeatable test case that displays a problem. Feel free to hack the file pipe_test.c in the tests/ directory of the source tree.

@mgeier
Copy link
Author

mgeier commented Oct 16, 2014

What should libsndfile set [sfinfo.frames] to?

To the number of available frames, I guess?

In case of a freshly opened file in SFM_WRITE mode this would mean 0.
Zero.
Like libsndfile does for normal (= non-pipe) files, as expected.

I still don't see any big here.

It's nothing big.
It's actually very small and unimportant.
It's still strange, that's why I asked.
And I think it's a bug, even if it's a tiny one.

Feel free to hack the file pipe_test.c in the tests/ directory of the source tree.

I played around a bit but I don't know how to check if some variable has a value of 0 and generate a test failure if not.

But what I did is add a line in tests/stdout_test.c right after sf_open():

fprintf(stderr, "frames: %d not ", sfinfo.frames);

This prints some interesting numbers:

format frames expected
raw -1 0
au -13 0
paf -1025 0
ircam -513 0
pvf -9 0

So I guess frames is set to SF_COUNT_MAX for some reason and then the header size or something is subtracted for some reason.

Do you really not find that strange?

@erikd
Copy link
Member

erikd commented Oct 17, 2014

What should libsndfile set [sfinfo.frames] to?

To the number of available frames, I guess?

How is it supposed to figure that out? While writing a file, the number of frames written cannot be determined until the file is closed, regardless of whether you are writing to a file or a pipe.

But what I did is add a line in tests/stdout_test.c right after sf_open():

 fprintf(stderr, "frames: %d not ", sfinfo.frames);

At this point, the value of sfinfo.frames is irrelevant and un-important. Consider the value at that point to be the result of "un-defined behaviour".

Do you really not find that strange?

For undefined behaviour, not even nasal daemons (http://www.catb.org/jargon/html/N/nasal-demons.html) would be considered strange.

@mgeier
Copy link
Author

mgeier commented Oct 17, 2014

What should libsndfile set [sfinfo.frames] to?

To the number of available frames, I guess?

How is it supposed to figure that out?

That's easy. It's always zero.

As I understand it, sfinfo.frames is supposed to tell me how many frames are available right after opening the file. It is set in sf_open() after all and I'm not expecting it to look into the future.
In case of SFM_WRITE it is unambiguously and without any doubt clear that the number of available frames is zero right after opening. That's by definition. Because SFM_WRITE is supposed to truncate the file. So there will always be zero frames.

While writing a file, the number of frames written cannot be determined until the file is closed,

Sure. No problem there. The number of frames when closing the file can only be determined when closing the file.
But, as written above, the number of frames right after opening is still zero.
The value sfinfo.frames is not claiming to update itself whenever a file is written to.

Following your argument, the same problem would occur in SFM_RDWR mode.
But there everything works just as expected: right after opening, the value is meaningful, after writing to the file it may not be meaningful anymore. So what? Nobody expects that.

regardless of whether you are writing to a file or a pipe.

But why does it then work as expected with "normal" files and does yield a strange value only with pipes?

Consider the value at that point to be the result of "un-defined behaviour".

OK, if you want to make libsndfile's interface more complicated than necessary, I can live with that (Thanks for the nasal daemons link, BTW). But then you should mention it in the documentation.

Anyway, thanks very much for your responses.
I will just overwrite sfinfo.frames with 0 (of course only for SFM_WRITE) in the Python wrapper I'm working on, then it will yield the expected value with any current and future version of libsndfile.

@erikd
Copy link
Member

erikd commented Oct 17, 2014

As I understand it, sfinfo.frames is supposed to tell me how many frames are available
right after opening the file.

Thats an incorrect assumption. That field is undefined when opening a file in SFM_WRITE mode.

Following your argument, the same problem would occur in SFM_RDWR mode.

No it wouldn't. In SFM_RDWR mode there are samples there so it gives a valid value.

But why does it then work as expected with "normal" files and does yield a strange value only with pipes?

The code paths are different for "normal" files.

Anyway, I've messed with the tests a little and found that a trival rearrangement of the code in the library can result in sfinfo.frames being zero when opening a file in SFM_WRITE as you expect.

@erikd erikd self-assigned this Oct 17, 2014
erikd added a commit that referenced this issue Oct 17, 2014
When opening a file in SFM_WRITE mode where the output is a pipe,
the sfinfo.frames value was coming back with strange values. The
fix is simply to make sure it get set to zero.

Closes: #77
@erikd
Copy link
Member

erikd commented Oct 17, 2014

Fixed in commit 19e12f5.

@erikd erikd closed this as completed Oct 17, 2014
@mgeier
Copy link
Author

mgeier commented Oct 17, 2014

Great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants