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

Already on GitHub? Sign in to your account

Cracks with ogg files. #271

Closed
Marisa-Chan opened this Issue Aug 13, 2012 · 22 comments

Comments

Projects
None yet
2 participants

In current RC 2.0 version bug with ogg files - cracks while playback and no sense how music is loaded sf::Music or sf::Sound.
In 1.6 all works good.
If I try play "wav" file created by oggdec - all OK.

Original file: http://rghost.ru/39759201
How it's playing: http://rghost.ru/39759228

I'm running: ArchLinux 32Bit.

This is bug of libsndfile when you are reading ogg with sf_read_short, but with sf_read_float all goes OK.
With sf_read_float format of openal buffer mustbe AL_FORMAT_STEREO_FLOAT32 (in alext.h), and in this file i found AL_FORMAT_VORBIS_EXT, may be openal can play vorbis directly?

Owner

LaurentGomila commented Aug 14, 2012

Thanks a lot for investigating. Unfortunately, SFML uses short for audio samples, not float. I'll contact the author to see if this can be fixed in libsndfile.

in this file i found AL_FORMAT_VORBIS_EXT, may be openal can play vorbis directly?

"OpenAL Soft does not support the Vorbis and MP3 extensions, however these are considered deprecated"

Owner

LaurentGomila commented Aug 14, 2012

By the way, which version of libsndfile do you have?

1.0.25, and i already tried to replace it in sfml build but it was useless

Problem is in conversion from float to short and int in libsndfile. Libsndfile's code don't care about normalizing floats when converts them to integer and we get samples "short = float * 32767" when float above 1.0 or below -1.0.

Looking into code of libsndfile i can say it's not easy to correct this by design(or easy correction, but worst quality, because some samples will be overdrived). Easy way is if vorbis detected - get maximum signal level by "sf_command (sndfile, SFC_CALC_SIGNAL_MAX, ..." and read samples with scaling and right conversion to short, but it's workaround, and it's must be maked by libsndfile.

Hmm, i found SFC_SET_SCALE_FLOAT_INT_READ designed for such cases, but it not works for ogg vorbis, it's libsndfile bug. I write issue to libsnd github, but no reaction.

Owner

LaurentGomila commented Aug 15, 2012

Thanks for investigating, and for posting an issue on libsndfile tracker. Let me know if you get some news.

Erik answered me:
"Sorry, really busy this week. Its unlikely I'll have a chance to look at this before the weekend."
direct link for issue: erikd/libsndfile#16

Bug fixed, now with new libsndfile after sf_open if format is vorbis we need to use SFC_SET_SCALE_FLOAT_INT_READ, like this:

m_file = sf_open(filename.c_str(), SFM_READ, &fileInfos);
if (!m_file)
{
    err() << "Failed to open sound file \"" << filename << "\" (" << sf_strerror(m_file) << ")" << std::endl;
    return false;
}

if (fileInfos.format & SF_FORMAT_VORBIS)
sf_command (m_file, SFC_SET_SCALE_FLOAT_INT_READ, (void *)1, sizeof (char)) ;
Owner

LaurentGomila commented Sep 25, 2012

now with new libsndfile after sf_open if format is vorbis we need to use SFC_SET_SCALE_FLOAT_INT_READ

Why? Won't 1 produce the same result as if it wasn't set? Isn't it useful only if we know the specific maximum value of the file? And in this case, how are we supposed to know it?

And what about SFC_SET_SCALE_INT_FLOAT_WRITE? Doesn't it need to be fixed the same way?

SFC_SET_SCALE_FLOAT_INT_READ - it's simple command, it automatic seek file
and find maximum value.
:
case SFC_SET_SCALE_FLOAT_INT_READ :
old_value = psf->float_int_mult ;

psf->float_int_mult = (datasize != 0) ? SF_TRUE : SF_FALSE ;
if (psf->float_int_mult && psf->float_max < 0.0)
psf->float_max = psf_calc_signal_max (psf, SF_FALSE) ;
return old_value ;

for SFC_SET_SCALE_INT_FLOAT_WRITE i will check code.

2012/9/25 Laurent Gomila notifications@github.com

now with new libsndfile after sf_open if format is vorbis we need to use
SFC_SET_SCALE_FLOAT_INT_READ

Why? Won't 1 produce the same result as if it wasn't set? Isn't it useful
only if we know the specific maximum value of the file? And in this case,
how are we supposed to know it?

And what about SFC_SET_SCALE_INT_FLOAT_WRITE? Doesn't it need to be fixed
the same way?


Reply to this email directly or view it on GitHubhttps://github.com/LaurentGomila/SFML/issues/271#issuecomment-8855785.

Owner

LaurentGomila commented Sep 25, 2012

Oh I see, so "1" means "enable it". But then it seems strange to make this feature optional.

Anyway, I'll apply the fix and wait until there's a new release of libsndfile. Thanks a lot for your help!

Owner

LaurentGomila commented Sep 25, 2012

By the way, why is your code different from the example in the documentation?

sf_command (sndfile, SFC_SET_SCALE_FLOAT_INT_READ, NULL, SF_TRUE) ;

What's the correct way of calling it?

@LaurentGomila LaurentGomila reopened this Sep 25, 2012

from documentation, because i don't look into doc while write this code.

2012/9/25 Laurent Gomila notifications@github.com

By the way, why is your code different from the example in the
documentation?

sf_command (sndfile, SFC_SET_SCALE_FLOAT_INT_READ, NULL, SF_TRUE) ;

What's the correct way of calling it?


Reply to this email directly or view it on GitHubhttps://github.com/LaurentGomila/SFML/issues/271#issuecomment-8859075.

SFC_SET_SCALE_INT_FLOAT_WRITE not needs to set, because SFML use "short"
samples and value of short not above 32767, it's only input issue, because
some audio formats with float samples may be not normalized to 1.0(MAX) and
while converting to short or int it may cause overflow. When your try
writing data to libsnd by "short" it's already
normalized. SFC_SET_SCALE_INT_FLOAT_WRITE is needed if you want to make new
audio maximum not at 1.0.

2012/9/26 Lancelot Lynx llancelot7@gmail.com

from documentation, because i don't look into doc while write this code.

2012/9/25 Laurent Gomila notifications@github.com

By the way, why is your code different from the example in the
documentation?

sf_command (sndfile, SFC_SET_SCALE_FLOAT_INT_READ, NULL, SF_TRUE) ;

What's the correct way of calling it?


Reply to this email directly or view it on GitHubhttps://github.com/LaurentGomila/SFML/issues/271#issuecomment-8859075.

Owner

LaurentGomila commented Sep 26, 2012

from documentation, because i don't look into doc

Hmm... You got it from the doc because you didn't look at the doc? :D

SFC_SET_SCALE_INT_FLOAT_WRITE not needs to set, because SFML use "short"
samples

Yeah, this was more a general comment, without SFML in mind. Just the other side of your patch.

no-no-no, correct calling writed in a doc )

2012/9/26 Laurent Gomila notifications@github.com

from documentation, because i don't look into doc

Hmm... You got it from the doc because you didn't look at the doc? :D

SFC_SET_SCALE_INT_FLOAT_WRITE not needs to set, because SFML use "short"
samples

Yeah, this was more a general comment, without SFML in mind. Just the
other side of your patch.


Reply to this email directly or view it on GitHubhttps://github.com/LaurentGomila/SFML/issues/271#issuecomment-8879975.

Owner

LaurentGomila commented Sep 26, 2012

Ok, sorry for misunderstanding your previous answer.

But does it mean that you didn't test your patched libsndfile + modified SFML, to make sure that it solved the original problem?

tested, and it works OK. My version will works too, but it not correct by
logic and design way, but works :) libsndfile checks only size of
additional data and if it not null, when scaling will be switched on. If
last argument is 0 - when scaling switching off.

2012/9/26 Laurent Gomila notifications@github.com

Ok, sorry for misunderstanding your previous answer.

But does it mean that you didn't test your patched libsndfile + modified
SFML, to make sure that it solved the original problem?


Reply to this email directly or view it on GitHubhttps://github.com/LaurentGomila/SFML/issues/271#issuecomment-8888046.

Owner

LaurentGomila commented Sep 26, 2012

Ok, thanks.

How about read samples as float and if regular sample > 1.0 then it's will
be new high level. It's not accurate with sound volume level in music file,
but it will solve slow determination of maximal sound level.

2012/9/26 Laurent Gomila notifications@github.com

Ok, thanks.


Reply to this email directly or view it on GitHubhttps://github.com/LaurentGomila/SFML/issues/271#issuecomment-8888782.

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