Closed mixxxbot closed 2 years ago
Commented by: l0rdt Date: 2012-01-05T13:40:49Z Attachments: [ReplayGain patch](https://bugs.launchpad.net/mixxx/+bug/912258/+attachment/2658437/+files/ReplayGain patch)
Commented by: rryan Date: 2012-01-05T17:56:15Z
Hm! Looks like a good move. If multiple analyser queues are running at once (the main track analysis queue and the library 'Analyze' section) then this would likely cause problems since you're right, replay-gain isn't re-entrant since it uses all those static variables.
Commented by: asantoni Date: 2012-03-12T22:48:14Z
Is this why we sometimes see the waveform "pop" as the gain is being automatically adjusted?
Commented by: rryan Date: 2012-03-13T05:00:49Z
Since this could explain some segfaults we've seen 1.10.1 is reasonable.
Commented by: rryan Date: 2012-05-14T20:38:27Z
Thanks Vittorio!
I noticed that every process() call you allocate a new buffer. This will have significant performance implications since that will allocate 8192 bytes every process() call. Beyond that I don't think it was necessary since AnalyserRg has its own internal buffers so you don't need to create a new one. This will also put a lot of pressure on the heap which could be causing priority inversion with the engine and other parts of MIxxx.
One additional thing I saw you did was hand-unroll a lot of loops. This is the sort of thing that GCC does for us (and it's incredibly rar a programmer can even come close to what GCC can do) so there's no sense in competing with GCC here ;)
I made both of these changes to your patch and committed to 1.10. I think the heap-spray issue may be part of why analysis causes xruns in trunk so I'm hopeful that this change will help that a lot.
Commented by: rryan Date: 2012-05-14T20:47:24Z
Sorry, not 8192 bytes, 8192 floats so 32kb... roughly 132MB in total for a 12 minute 48khz track.
On Mon, May 14, 2012 at 4:38 PM, RJ Ryan
Thanks Vittorio!
I noticed that every process() call you allocate a new buffer. This will have significant performance implications since that will allocate 8192 bytes every process() call. Beyond that I don't think it was necessary since AnalyserRg has its own internal buffers so you don't need to create a new one. This will also put a lot of pressure on the heap which could be causing priority inversion with the engine and other parts of MIxxx.
One additional thing I saw you did was hand-unroll a lot of loops. This is the sort of thing that GCC does for us (and it's incredibly rar a programmer can even come close to what GCC can do) so there's no sense in competing with GCC here ;)
I made both of these changes to your patch and committed to 1.10. I think the heap-spray issue may be part of why analysis causes xruns in trunk so I'm hopeful that this change will help that a lot.
-- You received this bug notification because you are a member of Mixxx Development Team, which is subscribed to Mixxx. https://bugs.launchpad.net/bugs/912258
Title: ReplayGain weird behaviour: a possible fix
To manage notifications about this bug go to: https://bugs.launchpad.net/mixxx/+bug/912258/+subscriptions
Issue closed with status Fix Released.
Reported by: l0rdt Date: 2012-01-05T13:40:49Z Status: Fix Released Importance: High Launchpad Issue: lp912258 Tags: replaygain Attachments: [ReplayGain patch](https://bugs.launchpad.net/bugs/912258/+attachment/2658437/+files/ReplayGain patch)
I fear that the original ReplayGain lib we are using is all but thread-safe. While working on the vamp branch, I wrote some code which seems to me more suitable for using in analysers. Please find attached a patch for the current trunk.