ReplayGain support

[jlama]

Hi,
I've been playing with SFBAudioEngine and it's great.

Any tips/ideas where to implement ReplayGain support in the audio stack?
I've tried inserting an AVAudioUnitEQ between the player node and the main mixer but sometimes it crashes when the player switches decoder (due to unsupported format error -10868 and I'm unsure how to solve it).
The crash happens in -[SFBAudioPlayer configureEngineForGaplessPlaybackOfFormat:]

I guess I should remove the EQ node before SFBAudioPlayer attempts to switch decoder, but this does not feel right. Maybe there is a better/cleaner way to implement this?

Thanks.

[sbooth]

What I would recommend is in the player's rendering will start delegate method, schedule a volume parameter change on the main mixer node using the desired replay gain value. You can schedule the parameter to change at the time the track will start if you want to be extra precise. There shouldn't be any need to add additional Audio Units.

[jlama]

Thanks for your help.

If I understand well you can only apply attenuation with the main mixer volume. What if you also want some positive preamp gain?

I realize now my question is more general since I may also need to do more advanced processing (like n-band EQ): how do you properly insert Audio Units in the audio graph and handle configuration (format) changes?

[sbooth]

I added https://github.com/sbooth/SFBAudioEngine/wiki/Adding-an-AVAudioUnit-to-AudioPlayer which I hope explains the process in a bit more detail.

[jlama]

Thank you very much, that's helpful. Sadly it did not work: audioPlayerAVAudioEngineConfigurationChange: is not called when the player node is updated.

However I think I finally understand why I get the kAudioUnitErr_FormatNotSupported error.
When [SFBAudioPlayer configureEngineForGaplessPlaybackOfFormat:] is called with a new format, it breaks the connection point between the old player node and its output (the EQ node), and then reconnects the new player node to its previous output with the new format. Yet the output format of the EQ node is not updated (and all other in/outs of subsequent custom nodes in the chain), causing the error.

So maybe a hook is needed in configureEngineForGaplessPlaybackOfFormat: to allow users to properly reconfigure the chain?

[sbooth]

It's surprising that audioPlayerAVAudioEngineConfigurationChange isn't called when the player node is updated because there should be a change in either sample rate or channel count which I (perhaps incorrectly) assumed would cause the notification to be posted.

I think your idea for a hook is a good one because it would let the changes to the processing graph be grouped together.

[sbooth]

Please take a look at #401 and let me know if it works for you.

[jlama]

It's surprising that audioPlayerAVAudioEngineConfigurationChange isn't called when the player node is updated because there should be a change in either sample rate or channel count which I (perhaps incorrectly) assumed would cause the notification to be posted.

My interpretation of Apple's documentation is that AVAudioEngineConfigurationChangeNotification is only sent when hardware changes.

I tested your patch but there's still an issue:

The exception is raised in connect: when attempting to reattach the graph. It tries changing the EQ input but not its output which is still connected with the old format and now invalid.

if(playerNodeOutputConnectionPoint)
    [_engine connect:_playerNode to:playerNodeOutputConnectionPoint.node format:format];

Not sure how to handle this cleanly. Some ideas:

• if a user node is detected between the player and mixer, its output is always disconnected before calling the delegate audioPlayer:reconfiguredProcessingGraph:
• if a user node is detected, the new player node's output is left unconnected, the user must handle the reconnection in audioPlayer:reconfigureProcessingGraph: (note: reconfigure, not 'ed)

[sbooth]

Thank you for testing out the proposed PR and providing feedback.

I pushed additional changes that I hope will fix the issue.

[jlama]

Thanks, we are nearly there :)

So now adding an AudioUnit in the chain may look like this:

init() {
      self.audioPlayer = AudioPlayer()

      self.equalizer = AVAudioUnitEQ()
      self.equalizer.globalGain = -6
      self.equalizer.bypass = false

      self.audioPlayer.delegate = self
      self.audioPlayer.withEngine { engine in
          guard let mixerCP = engine.inputConnectionPoint(for: engine.mainMixerNode, inputBus: 0) else {
              logger.critical("AVAudioEngine missing main mixer node")
              return
          }
          guard let srcNode = mixerCP.node else {
              logger.critical("AVAudioEngine missing input node to main mixer node")
              return
          }

          let format = srcNode.outputFormat(forBus: 0)
          engine.attach(self.equalizer)
          engine.disconnectNodeInput(engine.mainMixerNode)
          engine.connect(srcNode, to: self.equalizer, format: format)
          engine.connect(self.equalizer, to: engine.mainMixerNode, format: format)
      }
}

func audioPlayer(_ audioPlayer: AudioPlayer, reconfigureProcessingGraph engine: AVAudioEngine) -> AVAudioNode {
    let format = audioPlayer.playerNode.renderingFormat
    engine.disconnectNodeOutput(self.equalizer)
    engine.connect(self.equalizer, to: engine.mainMixerNode, format: format)
    return self.equalizer
}

I suggest providing the new format as an argument to the reconfigureProcessingGraph function. Right now we need to access the playerNode.renderingFormat to retrieve it. playerNode.outputFormat(forBus: 0) still returns the old format.

It would look like this:

func audioPlayer(_ audioPlayer: AudioPlayer, reconfigureProcessingGraph engine: AVAudioEngine, withFormat format: AVAudioFormat) -> AVAudioNode {
    engine.disconnectNodeOutput(self.equalizer)
    engine.connect(self.equalizer, to: engine.mainMixerNode, format: format)
    return self.equalizer
}

[sbooth]

I'd actually considered that initially but then decided against it, but after seeing your code it definitely makes sense to include the format in the delegate call.

I guess the last thing to nail down is whether the delegate method should be called

1. audioPlayer:reconfigureProcessingGraph:forFormat:,
2. audioPlayer:reconfigureProcessingGraph:withFormat:, or
3. audioPlayer:reconfigureProcessingGraph:format:

[jlama]

Your call :)
I prefer either withFormat: or format:

[sbooth]

I think these are good changes. Thank you for raising the issue.

[jlama]

Thank you for your support and time!