Skip to content

Commit

Permalink
Fix audio distortion after reactivating the device.
Browse files Browse the repository at this point in the history 
After deactivating and reactivating the device, the audio from the
device's input stream was sometimes distorted for a short time.

The problem was that the host doesn't always call our StopIO function
for every client when the device is deactivated. That stopped BGM_Device
from resetting the timeline of the timestamps it returns from
BGM_Driver::GetZeroTimeStamp.

The timestamps would have host times far in the past and that made the
host try to catch up by skipping over some samples.

The fix is to reset the client data when the device is activated, which
ensures that it will reset the timestamps (i.e. the loopback clock).

Also, change the driver's UUID in Info.plist so it's not the same as the
one Background Music uses. As far as I know, that just stops coreaudiod
from logging a warning about it.
  • Loading branch information
@kyleneideck @scottjg
kyleneideck authored and scottjg committed Mar 2, 2022
1 parent 7041bc9 commit 533b66d
Show file tree
Hide file tree
Showing 9 changed files with 209 additions and 49 deletions.
2 changes: 1 addition & 1 deletion BGMDriver/BGMDriver.xcodeproj/xcshareddata/xcschemes/Rewatch Loopback Device.xcscheme
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
</Testables>
</TestAction>
<LaunchAction
buildConfiguration = "Debug"
buildConfiguration = "Release"
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
debugAsWhichUser = "root"
Expand Down
140 changes: 104 additions & 36 deletions BGMDriver/BGMDriver/BGM_Device.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
// Copyright © 2017 Andrew Tonner
// Copyright © 2019 Gordon Childs
// Copyright (C) 2013 Apple Inc. All Rights Reserved.
// Copyright © 2020 MakeTheWeb
// Copyright © 2020, 2021 MakeTheWeb
//
// Based largely on SA_Device.cpp from Apple's SimpleAudioDriver Plug-In sample code. Also uses a few sections from Apple's
// NullAudio.c sample code (found in the same sample project).
Expand Down Expand Up @@ -161,6 +161,12 @@ void BGM_Device::Activate()
// Open the connection to the driver and initialize things.
//_HW_Open();

// Reset the clients. This does nothing if this is the first time the device has been
// activated. If it's not, we might still have clients registered from the previous
// activation. The host doesn't always call our StopIO and RemoveClient for every client
// when we deactivate the device, so we need to remove the old clients here.
mClients.RemoveAll();

mInputStream.Activate();
mOutputStream.Activate();

Expand All @@ -179,20 +185,25 @@ void BGM_Device::Activate()

SendDeviceIsAlivePropertyNotifications();
}
else
{
LogWarning("BGM_Device::Activate: NOT activating BGM%s device. Already active.",
GetObjectID() == kObjectID_Device_UI_Sounds ? " UI sounds" : "");
}
}

void BGM_Device::Deactivate()
{
DebugMsg("BGM_Device::Deactivate: Deactivating BGM%s device",
GetObjectID() == kObjectID_Device_UI_Sounds ? " UI sounds" : "");

// When this method is called, the object is basically dead, but we still need to be thread
// safe. In this case, we also need to be safe vs. any IO threads, so we need to take both
// locks.
CAMutex::Locker theStateLocker(mStateMutex);

if(IsActive())
{
DebugMsg("BGM_Device::Deactivate: Deactivating BGM%s device",
GetObjectID() == kObjectID_Device_UI_Sounds ? " UI sounds" : "");

CAMutex::Locker theIOLocker(mIOMutex);

// Mark the device's sub-objects inactive.
Expand All @@ -209,6 +220,11 @@ void BGM_Device::Deactivate()

SendDeviceIsAlivePropertyNotifications();
}
else
{
LogWarning("BGM_Device::Deactivate: NOT deactivating BGM%s device. Already inactive.",
GetObjectID() == kObjectID_Device_UI_Sounds ? " UI sounds" : "");
}
}

void BGM_Device::SendDeviceIsAlivePropertyNotifications()
Expand Down Expand Up @@ -1218,11 +1234,21 @@ void BGM_Device::StartIO(UInt32 inClientID)
// We only tell the hardware to start if this is the first time IO has been started.
if(didStartIO)
{
DebugMsg("BGM_Device::StartIO: Client %u started IO. IO was previously stopped for all "
"clients.",
inClientID);

kern_return_t theError = _HW_StartIO();
ThrowIfKernelError(theError,
CAException(theError),
"BGM_Device::StartIO: Failed to start because of an error calling down to the driver.");
}
else
{
DebugMsg("BGM_Device::StartIO: Client %u started IO. IO was NOT previously stopped for "
"all clients.",
inClientID);
}

clientIsBGMApp = mClients.IsBGMApp(inClientID);
bgmAppHasClientRegistered = mClients.BGMAppHasClientRegistered();
Expand Down Expand Up @@ -1273,46 +1299,57 @@ void BGM_Device::StopIO(UInt32 inClientID)
// we tell the hardware to stop if this is the last stop call
if(didStopIO)
{
DebugMsg("BGM_Device::StopIO: Client %u stopped IO. All clients have stopped IO.",
inClientID);
_HW_StopIO();
}
else
{
DebugMsg("BGM_Device::StopIO: Client %u stopped IO. NOT all clients have stopped IO.",
inClientID);
}
}

void BGM_Device::GetZeroTimeStamp(Float64& outSampleTime, UInt64& outHostTime, UInt64& outSeed)
{
// accessing the buffers requires holding the IO mutex
CAMutex::Locker theIOLocker(mIOMutex);

if(mWrappedAudioEngine != NULL)
{
}
else
// Accessing mLoopbackTime requires holding the IO mutex.
CAMutex::Locker theIOLocker(mIOMutex);

// We base our timing on the host. This is mostly from Apple's NullAudio.c sample code.

// Get the current host time.
UInt64 theCurrentHostTime = CAHostTimeBase::GetTheCurrentTime();

// Calculate the next host time.
Float64 theHostTicksPerRingBuffer =
mLoopbackTime.hostTicksPerFrame * kLoopbackRingBufferFrameSize;
Float64 theHostTickOffset =
static_cast<Float64>(mLoopbackTime.numberTimeStamps + 1) * theHostTicksPerRingBuffer;
UInt64 theNextHostTime = mLoopbackTime.anchorHostTime + static_cast<UInt64>(theHostTickOffset);

// Go to the next time if the next host time is less than the current time.
if(theNextHostTime <= theCurrentHostTime)
{
// Without a wrapped device, we base our timing on the host. This is mostly from Apple's NullAudio.c sample code
UInt64 theCurrentHostTime;
Float64 theHostTicksPerRingBuffer;
Float64 theHostTickOffset;
UInt64 theNextHostTime;

// get the current host time
theCurrentHostTime = CAHostTimeBase::GetTheCurrentTime();

// calculate the next host time
theHostTicksPerRingBuffer = mLoopbackTime.hostTicksPerFrame * kLoopbackRingBufferFrameSize;
theHostTickOffset = static_cast<Float64>(mLoopbackTime.numberTimeStamps + 1) * theHostTicksPerRingBuffer;
theNextHostTime = mLoopbackTime.anchorHostTime + static_cast<UInt64>(theHostTickOffset);

// go to the next time if the next host time is less than the current time
if(theNextHostTime <= theCurrentHostTime)
{
mLoopbackTime.numberTimeStamps++;
}

// set the return values
outSampleTime = mLoopbackTime.numberTimeStamps * kLoopbackRingBufferFrameSize;
outHostTime = static_cast<UInt64>(mLoopbackTime.anchorHostTime + (static_cast<Float64>(mLoopbackTime.numberTimeStamps) * theHostTicksPerRingBuffer));
// TODO: I think we should increment outSeed whenever this device switches to/from having a wrapped engine
outSeed = 1;
mLoopbackTime.numberTimeStamps++;
}

// Set the return values.
outSampleTime = mLoopbackTime.numberTimeStamps * kLoopbackRingBufferFrameSize;
outHostTime = static_cast<UInt64>(
mLoopbackTime.anchorHostTime +
(static_cast<Float64>(mLoopbackTime.numberTimeStamps) * theHostTicksPerRingBuffer));
outSeed = mLoopbackTime.seed;

#if DetailedIOLogging
DebugMsg("GZTS theCurrentHostTime %llu mLoopbackTime.hostTicksPerFrame %f "
"theHostTicksPerRingBuffer %f mLoopbackTime.numberTimeStamps %llu "
"theHostTickOffset %f mLoopbackTime.anchorHostTime %llu theNextHostTime %llu",
theCurrentHostTime, mLoopbackTime.hostTicksPerFrame, theHostTicksPerRingBuffer,
mLoopbackTime.numberTimeStamps, theHostTickOffset, mLoopbackTime.anchorHostTime,
theNextHostTime);
DebugMsg("GZTS outSampleTime %f outHostTime %llu outSeed %llu",
outSampleTime, outHostTime, outSeed);
#endif
}

void BGM_Device::WillDoIOOperation(UInt32 inOperationID, bool& outWillDo, bool& outWillDoInPlace) const
Expand Down Expand Up @@ -1493,6 +1530,8 @@ void BGM_Device::ReadInputData(UInt32 inIOBufferFrameSize, Float64 inSampleTime,
case kCARingBufferError_CPUOverload:
// Write silence to the buffer.
memset(outBuffer, 0, abl.mBuffers[0].mDataByteSize);
// Not a warning because we don't ever want to log on an IO thread in release builds.
DebugMsg("BGM_Device::ReadInputData: CPU overload");
break;
case kCARingBufferError_TooMuch:
// Should be impossible, but handle it just in case. Write silence to the buffer and
Expand All @@ -1504,6 +1543,18 @@ void BGM_Device::ReadInputData(UInt32 inIOBufferFrameSize, Float64 inSampleTime,
default:
throw CAException(kAudioHardwareUnspecifiedError);
}

#if DetailedIOLogging
DebugMsg("Dist from prev input: %f (buf size %u, prev %llu)",
(inSampleTime - mLastInputSampleTime),
inIOBufferFrameSize,
mLastInputBufferSize);

mLastInputSampleTime = inSampleTime;
mLastInputBufferSize = inIOBufferFrameSize;

DebugMsg("Read head to write head dist: %f", (mLastOutputSampleTime - inSampleTime));
#endif
}

void BGM_Device::WriteOutputData(UInt32 inIOBufferFrameSize, Float64 inSampleTime, const void* inBuffer)
Expand Down Expand Up @@ -1532,6 +1583,22 @@ void BGM_Device::WriteOutputData(UInt32 inIOBufferFrameSize, Float64 inSampleTim
{
Throw(CAException(err));
}

if (err == kCARingBufferError_CPUOverload)
{
// Not a warning because we don't ever want to log on an IO thread in release builds.
DebugMsg("BGM_Device::WriteOutputData: CPU overload");
}

#if DetailedIOLogging
DebugMsg("Dist from prev input: %f (buf size %u, prev %llu)",
(inSampleTime - mLastOutputSampleTime),
inIOBufferFrameSize,
mLastOutputBufferSize);

mLastOutputSampleTime = inSampleTime;
mLastOutputBufferSize = inIOBufferFrameSize;
#endif
}

void BGM_Device::ApplyClientRelativeVolume(UInt32 inClientID, UInt32 inIOBufferFrameSize, void* ioBuffer) const
Expand Down Expand Up @@ -1832,6 +1899,7 @@ kern_return_t BGM_Device::_HW_StartIO()
// Reset the loopback timing values
mLoopbackTime.numberTimeStamps = 0;
mLoopbackTime.anchorHostTime = CAHostTimeBase::GetTheCurrentTime();
mLoopbackTime.seed++;
// ...and the most-recent audible/silent sample times. mAudibleState is usually guarded by the
// IO mutex, but we haven't started IO yet (and this function can only be called by one thread
// at a time).
Expand Down
20 changes: 18 additions & 2 deletions BGMDriver/BGMDriver/BGM_Device.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// Copyright © 2016, 2017, 2019 Kyle Neideck
// Copyright © 2019 Gordon Childs
// Copyright (C) 2013 Apple Inc. All Rights Reserved.
// Copyright © 2020 MakeTheWeb
// Copyright © 2020, 2021 MakeTheWeb
//
// Based largely on SA_Device.h from Apple's SimpleAudioDriver Plug-In sample code.
// https://developer.apple.com/library/mac/samplecode/AudioDriverExamples
Expand Down Expand Up @@ -52,6 +52,12 @@
#include <pthread.h>


// If this is enabled, the IO functions will log debug info. They'll log several messages every IO
// cycle, which means something like every 9 ms. It will likely cause performance issues and
// glitches, since logging is slow and not realtime safe, so it's usually best to leave it disabled.
#define DetailedIOLogging 0


class BGM_Device
:
public BGM_AbstractDevice
Expand Down Expand Up @@ -248,7 +254,10 @@ class BGM_Device
struct {
Float64 hostTicksPerFrame = 0.0;
UInt64 numberTimeStamps = 0;
UInt64 anchorHostTime = 0;
UInt64 anchorHostTime = 0;
// Used to tell the host when we've reset the clock. Not sure it's actually doing anything
// currently.
UInt64 seed = 1;
} mLoopbackTime;

BGM_Stream mInputStream;
Expand All @@ -267,6 +276,13 @@ class BGM_Device
bool mPendingOutputVolumeControlEnabled = true;
bool mPendingOutputMuteControlEnabled = true;

#if DetailedIOLogging
Float64 mLastInputSampleTime = 0;
UInt64 mLastInputBufferSize = 0;
Float64 mLastOutputSampleTime = 0;
UInt64 mLastOutputBufferSize = 0;
#endif

};

#endif /* BGMDriver__BGM_Device */
Expand Down
1 change: 1 addition & 0 deletions BGMDriver/BGMDriver/BGM_PlugInInterface.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
//
// Copyright © 2016, 2017 Kyle Neideck
// Copyright (C) 2013 Apple Inc. All Rights Reserved.
// Copyright © 2021 MakeTheWeb
//
// Based largely on SA_PlugIn.cpp from Apple's SimpleAudioDriver Plug-In sample code.
// https://developer.apple.com/library/mac/samplecode/AudioDriverExamples
Expand Down
19 changes: 19 additions & 0 deletions BGMDriver/BGMDriver/DeviceClients/BGM_ClientMap.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
//
// Copyright © 2016, 2017, 2019 Kyle Neideck
// Copyright © 2017 Andrew Tonner
// Copyright © 2021 MakeTheWeb
//

// Self Include
Expand Down Expand Up @@ -126,6 +127,24 @@ BGM_Client BGM_ClientMap::RemoveClient(UInt32 inClientID)
return theClient;
}

void BGM_ClientMap::RemoveAll()
{
CAMutex::Locker theShadowMapsLocker(mShadowMapsMutex);

// Remove all clients from the shadow maps.
mClientMapShadow.clear();
mClientMapByPIDShadow.clear();
mClientMapByBundleID.clear();

// Swap the maps with their shadow maps.
SwapInShadowMaps();

// Remove the clients again so the maps and their shadow maps are kept identical.
mClientMapShadow.clear();
mClientMapByPIDShadow.clear();
mClientMapByBundleID.clear();
}

bool BGM_ClientMap::GetClientRT(UInt32 inClientID, BGM_Client* outClient) const
{
CAMutex::Locker theMapsLocker(mMapsMutex);
Expand Down
3 changes: 3 additions & 0 deletions BGMDriver/BGMDriver/DeviceClients/BGM_ClientMap.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
// BGMDriver
//
// Copyright © 2016 Kyle Neideck
// Copyright © 2021 MakeTheWeb
//

#ifndef __BGMDriver__BGM_ClientMap__
Expand Down Expand Up @@ -87,6 +88,8 @@ class BGM_ClientMap
public:
// Returns the removed client
BGM_Client RemoveClient(UInt32 inClientID);
/*! Remove all clients. */
void RemoveAll();

// These methods are functionally identical except that GetClientRT must only be called from real-time threads and GetClientNonRT
// must only be called from non-real-time threads. Both return true if a client was found.
Expand Down
Loading

0 comments on commit 533b66d

Please sign in to comment.