Skip to content

Commit

Permalink
Avoid some pointer arithmetic
Browse files Browse the repository at this point in the history
  • Loading branch information
kcat committed Mar 12, 2024
1 parent 0b8d7d7 commit 9018fe3
Show file tree
Hide file tree
Showing 27 changed files with 295 additions and 305 deletions.
71 changes: 40 additions & 31 deletions al/auxeffectslot.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
#include "core/fpu_ctrl.h"
#include "core/logging.h"
#include "direct_defs.h"
#include "effects/effects.h"
#include "effect.h"
#include "flexarray.h"
#include "opthelpers.h"
Expand Down Expand Up @@ -383,16 +384,16 @@ FORCE_ALIGN void AL_APIENTRY alDeleteAuxiliaryEffectSlotsDirect(ALCcontext *cont
std::lock_guard<std::mutex> slotlock{context->mEffectSlotLock};
if(n == 1)
{
ALeffectslot *slot{LookupEffectSlot(context, effectslots[0])};
ALeffectslot *slot{LookupEffectSlot(context, *effectslots)};
if(!slot) UNLIKELY
{
context->setError(AL_INVALID_NAME, "Invalid effect slot ID %u", effectslots[0]);
context->setError(AL_INVALID_NAME, "Invalid effect slot ID %u", *effectslots);
return;
}
if(slot->ref.load(std::memory_order_relaxed) != 0) UNLIKELY
{
context->setError(AL_INVALID_OPERATION, "Deleting in-use effect slot %u",
effectslots[0]);
*effectslots);
return;
}
RemoveActiveEffectSlots({&slot, 1u}, context);
Expand Down Expand Up @@ -471,24 +472,29 @@ AL_API void AL_APIENTRY alAuxiliaryEffectSlotPlayvSOFT(ALsizei n, const ALuint *
context->setError(AL_INVALID_VALUE, "Playing %d effect slots", n);
if(n <= 0) UNLIKELY return;

auto slots = std::vector<ALeffectslot*>(static_cast<ALuint>(n));
auto ids = al::span{slotids, static_cast<ALuint>(n)};
auto slots = std::vector<ALeffectslot*>(ids.size());
std::lock_guard<std::mutex> slotlock{context->mEffectSlotLock};
for(size_t i{0};i < slots.size();++i)
{
ALeffectslot *slot{LookupEffectSlot(context.get(), slotids[i])};
if(!slot) UNLIKELY
try {
auto lookupslot = [&context](const ALuint id) -> ALeffectslot*
{
context->setError(AL_INVALID_NAME, "Invalid effect slot ID %u", slotids[i]);
return;
}
ALeffectslot *slot{LookupEffectSlot(context.get(), id)};
if(!slot)
throw effect_exception{AL_INVALID_NAME, "Invalid effect slot ID %u", id};

if(slot->mState != SlotState::Playing)
{
slot->mPropsDirty = false;
slot->updateProps(context.get());
}
slots[i] = slot;
};
if(slot->mState != SlotState::Playing)
{
slot->mPropsDirty = false;
slot->updateProps(context.get());
}
return slot;
};
std::transform(ids.cbegin(), ids.cend(), slots.begin(), lookupslot);
}
catch(effect_exception& e) {
context->setError(e.errorCode(), "%s", e.what());
return;
}

AddActiveEffectSlots(slots, context.get());
for(auto slot : slots)
Expand Down Expand Up @@ -521,19 +527,22 @@ AL_API void AL_APIENTRY alAuxiliaryEffectSlotStopvSOFT(ALsizei n, const ALuint *
context->setError(AL_INVALID_VALUE, "Stopping %d effect slots", n);
if(n <= 0) UNLIKELY return;

auto slots = std::vector<ALeffectslot*>(static_cast<ALuint>(n));
auto ids = al::span{slotids, static_cast<ALuint>(n)};
auto slots = std::vector<ALeffectslot*>(ids.size());
std::lock_guard<std::mutex> slotlock{context->mEffectSlotLock};
for(size_t i{0};i < slots.size();++i)
{
ALeffectslot *slot{LookupEffectSlot(context.get(), slotids[i])};
if(!slot) UNLIKELY
try {
auto lookupslot = [&context](const ALuint id) -> ALeffectslot*
{
context->setError(AL_INVALID_NAME, "Invalid effect slot ID %u", slotids[i]);
return;
}

slots[i] = slot;
};
if(ALeffectslot *slot{LookupEffectSlot(context.get(), id)})
return slot;
throw effect_exception{AL_INVALID_NAME, "Invalid effect slot ID %u", id};
};
std::transform(ids.cbegin(), ids.cend(), slots.begin(), lookupslot);
}
catch(effect_exception& e) {
context->setError(e.errorCode(), "%s", e.what());
return;
}

RemoveActiveEffectSlots(slots, context.get());
for(auto slot : slots)
Expand Down Expand Up @@ -689,7 +698,7 @@ FORCE_ALIGN void AL_APIENTRY alAuxiliaryEffectSlotivDirect(ALCcontext *context,
case AL_EFFECTSLOT_TARGET_SOFT:
case AL_EFFECTSLOT_STATE_SOFT:
case AL_BUFFER:
alAuxiliaryEffectSlotiDirect(context, effectslot, param, values[0]);
alAuxiliaryEffectSlotiDirect(context, effectslot, param, *values);
return;
}

Expand Down Expand Up @@ -740,7 +749,7 @@ FORCE_ALIGN void AL_APIENTRY alAuxiliaryEffectSlotfvDirect(ALCcontext *context,
switch(param)
{
case AL_EFFECTSLOT_GAIN:
alAuxiliaryEffectSlotfDirect(context, effectslot, param, values[0]);
alAuxiliaryEffectSlotfDirect(context, effectslot, param, *values);
return;
}

Expand Down
28 changes: 16 additions & 12 deletions al/buffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1097,30 +1097,32 @@ FORCE_ALIGN void AL_APIENTRY alBufferivDirect(ALCcontext *context, ALuint buffer
case AL_AMBISONIC_LAYOUT_SOFT:
case AL_AMBISONIC_SCALING_SOFT:
case AL_UNPACK_AMBISONIC_ORDER_SOFT:
alBufferiDirect(context, buffer, param, values[0]);
alBufferiDirect(context, buffer, param, *values);
return;
}

ALCdevice *device{context->mALDevice.get()};
std::lock_guard<std::mutex> buflock{device->BufferLock};

ALbuffer *albuf = LookupBuffer(device, buffer);
al::span<const ALint> vals;
ALbuffer *albuf{LookupBuffer(device, buffer)};
if(!albuf) UNLIKELY
context->setError(AL_INVALID_NAME, "Invalid buffer ID %u", buffer);
else switch(param)
{
case AL_LOOP_POINTS_SOFT:
vals = {values, 2_uz};
if(albuf->ref.load(std::memory_order_relaxed) != 0) UNLIKELY
context->setError(AL_INVALID_OPERATION, "Modifying in-use buffer %u's loop points",
buffer);
else if(values[0] < 0 || values[0] >= values[1]
|| static_cast<ALuint>(values[1]) > albuf->mSampleLen) UNLIKELY
else if(vals[0] < 0 || vals[0] >= vals[1]
|| static_cast<ALuint>(vals[1]) > albuf->mSampleLen) UNLIKELY
context->setError(AL_INVALID_VALUE, "Invalid loop point range %d -> %d on buffer %u",
values[0], values[1], buffer);
vals[0], vals[1], buffer);
else
{
albuf->mLoopStart = static_cast<ALuint>(values[0]);
albuf->mLoopEnd = static_cast<ALuint>(values[1]);
albuf->mLoopStart = static_cast<ALuint>(vals[0]);
albuf->mLoopEnd = static_cast<ALuint>(vals[1]);
}
break;

Expand Down Expand Up @@ -1303,6 +1305,8 @@ FORCE_ALIGN void AL_APIENTRY alGetBufferivDirect(ALCcontext *context, ALuint buf

ALCdevice *device{context->mALDevice.get()};
std::lock_guard<std::mutex> buflock{device->BufferLock};

al::span<ALint> vals;
ALbuffer *albuf{LookupBuffer(device, buffer)};
if(!albuf) UNLIKELY
context->setError(AL_INVALID_NAME, "Invalid buffer ID %u", buffer);
Expand All @@ -1311,8 +1315,9 @@ FORCE_ALIGN void AL_APIENTRY alGetBufferivDirect(ALCcontext *context, ALuint buf
else switch(param)
{
case AL_LOOP_POINTS_SOFT:
values[0] = static_cast<ALint>(albuf->mLoopStart);
values[1] = static_cast<ALint>(albuf->mLoopEnd);
vals = {values, 2_uz};
vals[0] = static_cast<ALint>(albuf->mLoopStart);
vals[1] = static_cast<ALint>(albuf->mLoopEnd);
break;

default:
Expand Down Expand Up @@ -1524,7 +1529,7 @@ FORCE_ALIGN ALboolean AL_APIENTRY EAXSetBufferModeDirect(ALCcontext *context, AL
/* Special-case setting a single buffer, to avoid extraneous allocations. */
if(n == 1)
{
const auto bufid = buffers[0];
const auto bufid = *buffers;
if(bufid == AL_NONE)
return AL_TRUE;

Expand Down Expand Up @@ -1560,9 +1565,8 @@ FORCE_ALIGN ALboolean AL_APIENTRY EAXSetBufferModeDirect(ALCcontext *context, AL

/* Validate the buffers. */
std::unordered_set<ALbuffer*> buflist;
for(auto i = 0;i < n;++i)
for(const ALuint bufid : al::span{buffers, static_cast<ALuint>(n)})
{
const auto bufid = buffers[i];
if(bufid == AL_NONE)
continue;

Expand Down
58 changes: 42 additions & 16 deletions al/debug.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -456,30 +456,55 @@ FORCE_ALIGN ALuint AL_APIENTRY alGetDebugMessageLogDirectEXT(ALCcontext *context
context->setError(AL_INVALID_VALUE, "Negative debug log buffer size");
return 0;
}
auto sourcesOut = al::span{sources, sources ? count : 0u};
auto typesOut = al::span{types, types ? count : 0u};
auto idsOut = al::span{ids, ids ? count : 0u};
auto severitiesOut = al::span{severities, severities ? count : 0u};
auto lengthsOut = al::span{lengths, lengths ? count : 0u};
auto logOut = al::span{logBuf, logBuf ? static_cast<ALuint>(logBufSize) : 0u};

std::lock_guard<std::mutex> debuglock{context->mDebugCbLock};
ALsizei logBufWritten{0};
for(ALuint i{0};i < count;++i)
{
if(context->mDebugLog.empty())
return i;

auto &entry = context->mDebugLog.front();
const size_t tocopy{entry.mMessage.size() + 1};
if(logBuf)
if(logOut.data() != nullptr)
{
const size_t avail{static_cast<ALuint>(logBufSize - logBufWritten)};
if(avail < tocopy)
if(logOut.size() < tocopy)
return i;
std::copy_n(entry.mMessage.data(), tocopy, logBuf+logBufWritten);
logBufWritten += static_cast<ALsizei>(tocopy);
std::copy(entry.mMessage.cbegin(), entry.mMessage.cend(), logOut.begin());
logOut[entry.mMessage.size()] = '\0';
logOut = logOut.subspan(tocopy);
}

if(sources) sources[i] = GetDebugSourceEnum(entry.mSource);
if(types) types[i] = GetDebugTypeEnum(entry.mType);
if(ids) ids[i] = entry.mId;
if(severities) severities[i] = GetDebugSeverityEnum(entry.mSeverity);
if(lengths) lengths[i] = static_cast<ALsizei>(tocopy);
if(!sourcesOut.empty())
{
sourcesOut.front() = GetDebugSourceEnum(entry.mSource);
sourcesOut = sourcesOut.subspan<1>();
}
if(!typesOut.empty())
{
typesOut.front() = GetDebugTypeEnum(entry.mType);
typesOut = typesOut.subspan<1>();
}
if(!idsOut.empty())
{
idsOut.front() = entry.mId;
idsOut = idsOut.subspan<1>();
}
if(!severitiesOut.empty())
{
severitiesOut.front() = GetDebugSeverityEnum(entry.mSeverity);
severitiesOut = severitiesOut.subspan<1>();
}
if(!lengthsOut.empty())
{
lengthsOut.front() = static_cast<ALsizei>(tocopy);
lengthsOut = lengthsOut.subspan<1>();
}

context->mDebugLog.pop_front();
}
Expand Down Expand Up @@ -526,21 +551,22 @@ FORCE_ALIGN void AL_APIENTRY alGetObjectLabelDirectEXT(ALCcontext *context, ALen
if(label && bufSize == 0) UNLIKELY
return context->setError(AL_INVALID_VALUE, "Zero label bufSize");

auto copy_name = [name,bufSize,length,label](std::unordered_map<ALuint,std::string> &names)
const auto labelOut = al::span{label, label ? static_cast<ALuint>(bufSize) : 0u};
auto copy_name = [name,length,labelOut](std::unordered_map<ALuint,std::string> &names)
{
std::string_view objname;

auto iter = names.find(name);
if(iter != names.end())
objname = iter->second;

if(!label)
if(labelOut.empty())
*length = static_cast<ALsizei>(objname.length());
else
{
const size_t tocopy{std::min(objname.size(), static_cast<uint>(bufSize)-1_uz)};
std::memcpy(label, objname.data(), tocopy);
label[tocopy] = '\0';
const size_t tocopy{std::min(objname.size(), labelOut.size()-1_uz)};
std::copy_n(objname.cbegin(), tocopy, labelOut.begin());
labelOut[tocopy] = '\0';
if(length)
*length = static_cast<ALsizei>(tocopy);
}
Expand Down
2 changes: 1 addition & 1 deletion al/effect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ FORCE_ALIGN void AL_APIENTRY alEffectivDirect(ALCcontext *context, ALuint effect
switch(param)
{
case AL_EFFECT_TYPE:
alEffectiDirect(context, effect, param, values[0]);
alEffectiDirect(context, effect, param, *values);
return;
}

Expand Down
2 changes: 1 addition & 1 deletion al/effects/autowah.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ void EffectHandler::SetParamf(AutowahProps &props, ALenum param, float val)
}
}
void EffectHandler::SetParamfv(AutowahProps &props, ALenum param, const float *vals)
{ SetParamf(props, param, vals[0]); }
{ SetParamf(props, param, *vals); }

void EffectHandler::GetParami(const AutowahProps&, ALenum param, int*)
{ throw effect_exception{AL_INVALID_ENUM, "Invalid autowah integer property 0x%04x", param}; }
Expand Down
8 changes: 4 additions & 4 deletions al/effects/chorus.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ void EffectHandler::SetParami(ChorusProps &props, ALenum param, int val)
}
}
void EffectHandler::SetParamiv(ChorusProps &props, ALenum param, const int *vals)
{ SetParami(props, param, vals[0]); }
{ SetParami(props, param, *vals); }
void EffectHandler::SetParamf(ChorusProps &props, ALenum param, float val)
{
switch(param)
Expand Down Expand Up @@ -130,7 +130,7 @@ void EffectHandler::SetParamf(ChorusProps &props, ALenum param, float val)
}
}
void EffectHandler::SetParamfv(ChorusProps &props, ALenum param, const float *vals)
{ SetParamf(props, param, vals[0]); }
{ SetParamf(props, param, *vals); }

void EffectHandler::GetParami(const ChorusProps &props, ALenum param, int *val)
{
Expand Down Expand Up @@ -186,7 +186,7 @@ void EffectHandler::SetParami(FlangerProps &props, ALenum param, int val)
}
}
void EffectHandler::SetParamiv(FlangerProps &props, ALenum param, const int *vals)
{ SetParami(props, param, vals[0]); }
{ SetParami(props, param, *vals); }
void EffectHandler::SetParamf(FlangerProps &props, ALenum param, float val)
{
switch(param)
Expand Down Expand Up @@ -220,7 +220,7 @@ void EffectHandler::SetParamf(FlangerProps &props, ALenum param, float val)
}
}
void EffectHandler::SetParamfv(FlangerProps &props, ALenum param, const float *vals)
{ SetParamf(props, param, vals[0]); }
{ SetParamf(props, param, *vals); }

void EffectHandler::GetParami(const FlangerProps &props, ALenum param, int *val)
{
Expand Down
2 changes: 1 addition & 1 deletion al/effects/compressor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ void EffectHandler::SetParami(CompressorProps &props, ALenum param, int val)
}
}
void EffectHandler::SetParamiv(CompressorProps &props, ALenum param, const int *vals)
{ SetParami(props, param, vals[0]); }
{ SetParami(props, param, *vals); }
void EffectHandler::SetParamf(CompressorProps&, ALenum param, float)
{ throw effect_exception{AL_INVALID_ENUM, "Invalid compressor float property 0x%04x", param}; }
void EffectHandler::SetParamfv(CompressorProps&, ALenum param, const float*)
Expand Down
Loading

0 comments on commit 9018fe3

Please sign in to comment.