View Issue Details

IDProjectCategoryView StatusLast Update
0000613Gameplay + OpenGL[All Projects] Bugpublic2017-04-23 02:07
ReporterMajor Cooke 
Assigned To 
PrioritynormalSeverityminorReproducibilityhave not tried
Status resolvedResolutionfixed 
Summary0000613: OpenAL console errors
Descriptionhttps://puu.sh/vrt0H/cc3f5ca7be.jpg

Upon starting a new game, using D4D.

The good news, LOOP_ tags are at least working.
TagsNo tags attached.

Relationships

Activities

Chris

Chris

2017-04-21 16:06

developer   ~0001507

Looks like it's trying to specify a loop_end that's beyond the size of the buffer. The end point can't be greater than the number of samples in the buffer, so it needs to be clamped. Something like this:

diff --git a/src/sound/oalsound.cpp b/src/sound/oalsound.cpp
index b697282bb..c14855ea1 100644
--- a/src/sound/oalsound.cpp
+++ b/src/sound/oalsound.cpp
@@ -649,6 +649,16 @@ static size_t GetChannelCount(ChannelConfig chans)
        return 0;
 }
 
+static size_t GetSampleSize(SampleType type)
+{
+       switch(type)
+       {
+               case SampleType_UInt8: return 1;
+               case SampleType_Int16: return 2;
+       }
+       return 0;
+}
+
 static float GetRolloff(const FRolloffInfo *rolloff, float distance)
 {
        if(distance <= rolloff->MinDistance)
@@ -1241,7 +1251,7 @@ std::pair<SoundHandle,bool> OpenALSoundRenderer::LoadSound(uint8_t *sfxdata, 
int
        {
                size_t chancount = GetChannelCount(chans);
                size_t frames = data.Size() / chancount /
-                                               (type == SampleType_Int16 ? 2 : 1);
+                               GetSampleSize(type);
                if(type == SampleType_Int16)
                {
                        short *sfxdata = (short*)&data[0];
@@ -1283,6 +1293,8 @@ std::pair<SoundHandle,bool> OpenALSoundRenderer::LoadSound(uint8_t *sfxdata, 
int
        if (!endass) loop_end = Scale(loop_end, srate, 1000);
        if (loop_start < 0)     loop_start = 0;
 
+       loop_end = MIN<uint32_t>(loop_end, data.Size() / GetChannelCount(chans) /
+                                          GetSampleSize(type));
        if ((loop_start > 0 || loop_end > 0) && loop_end > loop_start && AL.SOFT_loop_points)
        
{
                ALint loops[2] = { static_cast<ALint>(loop_start), static_cast<ALint>(loop_end) 
};

_mental_

_mental_

2017-04-22 03:23

developer   ~0001514

Seems like some more fixes are needed.
Like Graf asked in 0000409 could someone craft a small example with bunch of cases covered? Like start tag only, end tag only, both tags, sample and time formats, invalid values, etc.
I'm trying to do it myself but it's easy to miss something.
Graf Zahl

Graf Zahl

2017-04-22 04:03

administrator   ~0001518

To be honest, I think the only thing needed here is to remove the error check or to change it to be more expressive. What do I care that there was an error in line 1291 of the source file, I want to know what's wrong with the loop tags!
_mental_

_mental_

2017-04-22 06:30

developer   ~0001519

There are several issues with the current implementation: start and end points are not clamped (i.e. out of range case), start beyond end isn't handled too, start at zero is a valid loop point for FMOD, probably something else I didn't discovered yet.
Graf Zahl

Graf Zahl

2017-04-22 08:12

administrator   ~0001521

In other words: It doesn't handle cases well, where the data is not correct. What should be done there? Normally I'd just say, let OpenAL decide and just don't print an error message if it finds the data is not correct. The fallback in all cases should be to ignore the loop because the conditions for looping are never met.

And start = 0 has to be a valid case, if that is enough to make OpenAL puke, it's an error in OpenAL.
_mental_

_mental_

2017-04-22 08:25

developer   ~0001522

Last edited: 2017-04-22 08:52

View 3 revisions

The last is not an error in OpenAL: LOOP_START=0 (with no end point) is indistinguishable from 'no loop' case but with FMOD it was looping whole sound.

Actually it doesn't matter as specifying CHAN_LOOP will loop whole sound anyway.

However the case with non-zero start point and no end point is not covered yet, whole sound will loop in this case.

Chris

Chris

2017-04-22 16:41

developer   ~0001530

Last edited: 2017-04-22 17:36

View 2 revisions

Now that loop_end is clamped to the buffer's sample count, reverting this commit should work. The default loop_end would be ~0u (UINT_MAX) again, which if no proper LOOP_END tag is specified, gets clamped to the end of the buffer. A non-zero loop start would then cause it to loop between the specified start point and the end, as desired.

EDIT: Made PR 302 to fix that.

Issue History

Date Modified Username Field Change
2017-04-21 08:06 Major Cooke New Issue
2017-04-21 09:31 Graf Zahl Status new => resolved
2017-04-21 09:31 Graf Zahl Resolution open => fixed
2017-04-21 16:06 Chris Note Added: 0001507
2017-04-22 00:52 _mental_ Status resolved => needs review
2017-04-22 03:23 _mental_ Status needs review => feedback
2017-04-22 03:23 _mental_ Note Added: 0001514
2017-04-22 04:03 Graf Zahl Note Added: 0001518
2017-04-22 06:30 _mental_ Note Added: 0001519
2017-04-22 08:12 Graf Zahl Note Added: 0001521
2017-04-22 08:25 _mental_ Note Added: 0001522
2017-04-22 08:41 _mental_ Note Edited: 0001522 View Revisions
2017-04-22 08:52 _mental_ Note Edited: 0001522 View Revisions
2017-04-22 11:14 _mental_ Status feedback => resolved
2017-04-22 16:41 Chris Note Added: 0001530
2017-04-22 17:36 Chris Note Edited: 0001530 View Revisions
2017-04-23 00:33 _mental_ Status resolved => needs review
2017-04-23 00:33 _mental_ Resolution fixed => reopened
2017-04-23 02:07 Graf Zahl Status needs review => resolved
2017-04-23 02:07 Graf Zahl Resolution reopened => fixed