Metering module for TAAE2

This discussion was created from comments split from: Presenting The Amazing Audio Engine 2.
«1

Comments

  • @Michael Should I attempt to make a stereo "/Taps/AEMeteringModule" or is there a better/simpler way?

  • I created AEMeteringModule.h and AEMeteringModule.m files and hacked the sample app to display the metering output (in labels - only tested on iPhone 6 device).

    The github branch can be found here: https://github.com/jcypher/TAAE2/tree/metering

    I'd love some feedback. I haven't done much DSP coding yet, so if anyone who is competent in that could check logic AEMeteringModuleProcess(...) function, that would be appreciated.

  • edited April 2016

    @leothiessen said:
    @Michael Should I attempt to make a stereo "/Taps/AEMeteringModule" or is there a better/simpler way?

    Good question! Right now I'm thinking it might be best implemented in AEDSPUtilities; something like

    void AEDSPMeter(const AudioBufferList * bufferList, int channels, float ** outAverage, float ** outPeak);
    

    That way it can be used on the output buffer list, as well as stack items, as you might want to successively mix stack items to the output, and then meter the output at the end.

    But then it could be used in an AEModule, as well, to make it easier.

  • @leothiessen said:
    I created AEMeteringModule.h and AEMeteringModule.m files and hacked the sample app to display the metering output (in labels - only tested on iPhone 6 device).

    The github branch can be found here: https://github.com/jcypher/TAAE2/tree/metering

    I'd love some feedback. I haven't done much DSP coding yet, so if anyone who is competent in that could check logic AEMeteringModuleProcess(...) function, that would be appreciated.

    Lovely! Looks great - as I mentioned on the commit, I think this is fine as-is; the metering is so simple, it doesn't really need to be refactored out into an AEDSPUtility.

  • @Michael said:

    @leothiessen said:
    @Michael Should I attempt to make a stereo "/Taps/AEMeteringModule" or is there a better/simpler way?

    Good question! Right now I'm thinking it might be best implemented in AEDSPUtilities; something like

    void AEDSPMeter(const AudioBufferList * bufferList, int channels, float ** outAverage, float ** outPeak);
    

    That way it can be used on the output buffer list, as well as stack items, as you might want to successively mix stack items to the output, and then meter the output at the end.

    But then it could be used in an AEModule, as well, to make it easier.

    @leothiessen
    The strategy should probably be:

    1. Create a circular buffer module
    2. meter the circular buffer on a separate background thread
    3. display results on yet another thread (main)

    That has the least impact on the audio-aorta, especially important if you intend to inject several metering nodes into the stream (for example multiple inputs and an overal output.)

  • @32BT said:
    The strategy should probably be:

    1. Create a circular buffer module
    2. meter the circular buffer on a separate background thread
    3. display results on yet another thread (main)

      That has the least impact on the audio-aorta, especially important if you intend to inject several metering nodes into the stream (for example multiple inputs and an overal output.)

    Hmm, that sounds like a lot of overhead for not very much gain. I doubt copying onto a circular buffer is any faster than the two vDSP calls, and then we've got to manage all the extra machinery. I'm pretty sure @leothiessen's implementation is the best way.

  • @Michael said:

    @32BT said:
    The strategy should probably be:

    1. Create a circular buffer module
    2. meter the circular buffer on a separate background thread
    3. display results on yet another thread (main)

      That has the least impact on the audio-aorta, especially important if you intend to inject several metering nodes into the stream (for example multiple inputs and an overal output.)

    Hmm, that sounds like a lot of overhead for not very much gain. I doubt copying onto a circular buffer is any faster than the two vDSP calls, and then we've got to manage all the extra machinery. I'm pretty sure @leothiessen's implementation is the best way.

    Huh?

    That can't be correct. A simple non-locking, 2-complement, unmanaged circularbuffer is enough and only needs to blindly copy samples in its renderloop. If that isn't a lot faster than any vDSP call, then there is something wrong with the copy routine.

    Also note that you might want to be metering more than just the latest levels, a lissajous representation for example. If you add multiple metering types, you would stack multiple vDSP calls for no other reason than..., well..., laziness! ;-)

  • edited April 2016

    Perhaps that might be left as an 'exercise for the reader', then =)

    I personally don't think all that additional code complexity is worth whatever time is saved. Given the speed of those vector ops, I honestly don't think the gains are worth it. Sure, if one's doing really complex analysis then it might be worthwhile, and TPCircularBuffer's there to help if that happens. It might even be worth making an AEModule type that provides a foundation for this kind of thing - basically, a bufferer + background processor with an arbitrary implementation. But for this basic metering, I think it's overkill.

  • Okay, fair enough. Also, I'm willing to contribute some milage, so we eventually have multiple options = flexibility.

  • edited April 2016

    I'm finding the output of the AEMeteringModule pretty wild/high-spikes (you can see it in the hack I did in the sample project on my 'metering' branch). For the 'avg' value I can sometimes get values as high as 1.7 and I think over 2 or maybe even 3 on the peak. Not all the time though. It happens when I blow in the mic and stuff on my iPhone 6 - but it seems kinda weird that it would spike so high. The numbers seem a lot wilder than what I am getting on my in-progress project that currently has TAAE1 in it (using its built in input metering, which I then convert to a ratio). I may be doing something wrong though, I'll have to check it out tomorrow.

    @32BT - that is interesting but a little outside of my ability for now

    @Michael - should I clean up the details on the AEMeteringModule and do a pull request? (I've never done a pull request - I presume it can be done for a file or two?)

    Thanks for the feedback, much appreciated.

  • This could be happening because of (maybe?) some kind of normalization happening in the mixer unit within TAAE1. TAAE2 uses 32-bit float everywhere, and mixing consists of adding buffers together, so it can sum to more than 1.0. That's all fine and intended, though.

  • @Michael said:
    Perhaps that might be left as an 'exercise for the reader', then =)

    I personally don't think all that additional code complexity is worth whatever time is saved. Given the speed of those vector ops, I honestly don't think the gains are worth it. Sure, if one's doing really complex analysis then it might be worthwhile, and TPCircularBuffer's there to help if that happens. It might even be worth making an AEModule type that provides a foundation for this kind of thing - basically, a bufferer + background processor with an arbitrary implementation. But for this basic metering, I think it's overkill.

    Sounds like a great idea, a module to do off-the-render-thread capturing of audio. Could be used for analysis (metering, FFT, etc) as well as custom disk writing or streaming to network etc.

  • Yeah, could be quite useful!

  • @j_liljedahl - I've updated the AEMeteringModule.h/AEMeteringModule.m files to allow for variable number of audio channels. Do these look right?

  • Close - you'll need to override - (void)rendererDidChangeChannelCount;, and update your float* buffers. But because that could crash the render thread (freeing the previous buffer before you set the new one, at the same time as it's being interacted with on the render thread), you'll need to use AEManagedValue to do the memory management for you.

    To do that, create two AEManagedValue @properties in the class extension, to replace the two float * instance properties, and use the 'pointerValue' property of those instances to assign/access the pointers on the main thread. On the audio thread, use AEManagedValueGetValue. Don't free() once you've allocated - AEManagedValue will handle that for you.

  • Didn't think of that! (I thought about another thing though, see the github comments).

    However, you can also use a single AEManagedValue that holds a struct with both float pointers in it.

  • You could, but it's a little more complicated because you'd either need to write your own free block (to release the two pointers and the struct), or you'd need to make it one contiguous allocation, with the struct, followed by first buffer, followed by second. Too complex, better just to use two AEManagedValues.

  • Isn't writing such a free-block quite straight forward?

  • Yeah, pretty much =)

  • Would it be more suitable to name this class something AELevelMonitoringModule?

  • I thought metering was fine, too, but either way is fine.

  • @Michael - ok, I just figured "metering" may imply something more than what I was doing (perhaps something with a history that could be graphed, etc.).

    I created these today: AELevelMonitoringModule.h / AELevelMonitoringModule.m - I've not tested them yet (getting a little late here), but if you have a minute could you tell me if I'm on the right track?

    I've incorporated use of a struct wrapped in an AEManagedValue to provide a access to it to the realtime audio thread & more safely free the float[] pointers. I also tried to provide a more accurate average/peak using the "reset" approach from TAAE1.

  • Cool!

    Some line-by-line comments (cos I can't figure out how to do it inline on Github):

    • 80: No need for the NULL assignments here, can save a few lines
    • 91: By the time it's dealloced, you can assume there's no more rendering happening, so no need for this dealloc. Consequently, no need for the 'monitoringEnabled' member
    • 96, 100, 104, etc: Style note: monitorValue.pointerValue, not [monitorValue pointerValue]
    • 134: Do you mean AEBufferStackGet?
    • 138: No need for this check, it'll always be non-NULL
    • 140: I think you mean abl, not context->output. Also you're not determining the start index, which could cause problems
    • 146: A memset(0) might be more efficient here, both in time and lines of code

    Some general thoughts:

    Is there a valid use case for specifying a sub-range of channels? I'd argue that it might be adding too much complexity here, which means more technical debt. I think I'd prefer to see it just monitor all the channels of the top buffer, maybe even with a limit of 2 (which makes the accessors easier - you could even have a method that returns a struct by value!).

    A mistake that I made in the past with TAAE1 was trying to do too much, to catch all the possible use cases, and it left me with a crippling amount of technical debt. Maybe this already has the balance right, but we need to make features really fight to survive =)

  • @Michael said:
    Cool!

    Some line-by-line comments (cos I can't figure out how to do it inline on Github):

    Comments are available in the commits, not in the file. Don't know why, would make things a lot easier, but I guess comments are stored with the commit for history tracking.

    Some general thoughts:

    Is there a valid use case for specifying a sub-range of channels? I'd argue that it might be adding too much complexity here, which means more technical debt. I think I'd prefer to see it just monitor all the channels of the top buffer, maybe even with a limit of 2 (which makes the accessors easier - you could even have a method that returns a struct by value!).

    +1

  • edited April 2016

    @32BT said:

    @Michael said:
    Some general thoughts:

    Is there a valid use case for specifying a sub-range of channels? I'd argue that it might be adding too much complexity here, which means more technical debt. I think I'd prefer to see it just monitor all the channels of the top buffer, maybe even with a limit of 2 (which makes the accessors easier - you could even have a method that returns a struct by value!).

    +1

    I agree with just monitoring all channels of the top buffer, and no range-selection. But I don't think a hard-coded limit of 2 channels make sense, someone might very well want to work with 4 channel or 5.1 audio etc.. So you'd still need a maxNumberOfChannels passed to the initializer, but could have the default init use 2 as max channel count. Also, if this is read-only, the float arrays can be allocated at init time, and there's no need for AEManagedValue.

    And then, you could even get the values by having getters that return the actual float pointers. float *AEMonitorGetPeakLevels(monitorModule) and int AEMonitorGetMaxChannelCount(monitorModule).

    Oh, I also would prefer to have the linear values, not converted to dB. That's better done in the actual UI part that displays such a meter, me thinks.

  • edited April 2016

    @j_liljedahl said:
    But I don't think a hard-coded limit of 2 channels make sense

    Fair enough, I think this is right. Make the default limit 2, but have another initializer that takes a max.

    And then, you could even get the values by having getters that return the actual float pointers. float *AEMonitorGetPeakLevels(monitorModule) and int AEMonitorGetMaxChannelCount(monitorModule).

    I'd advise against that part, because they could change at any point. Could lead to some weird behaviour?

    Oh, I also would prefer to have the linear values, not converted to dB. That's better done in the actual UI part that displays such a meter, me thinks.

    Yeah, that's probably a good idea. AEDSPUtilities has ratio to dB conversion functions, so it's pretty easy.

  • @Michael said:

    @j_liljedahl said:

    >

    And then, you could even get the values by having getters that return the actual float pointers. float *AEMonitorGetPeakLevels(monitorModule) and int AEMonitorGetMaxChannelCount(monitorModule).

    I'd advise against that part, because they could change at any point. Could lead to some weird behaviour?

    Well, they would be valid as long as the object is valid, since they are alloced on init and released in dealloc. But sure, some might store the pointer somewhere and then the object is deallocated. Copying into a user supplied array would be safer, use a bit more cpu and memory, and need the user to make sure he supplies an array with sufficient size.

  • @j_liljedahl said:
    Well, they would be valid as long as the object is valid, since they are alloced on init and released in dealloc. But sure, some might store the pointer somewhere and then the object is deallocated. Copying into a user supplied array would be safer, use a bit more cpu and memory, and need the user to make sure he supplies an array with sufficient size.

    Oh, that too - I was just thinking the values could change at any time, including midway through iterating the array on the main thread. Could produce some odd results in certain circumstances.

  • @Michael said:

    @j_liljedahl said:
    But I don't think a hard-coded limit of 2 channels make sense

    Fair enough, I think this is right. Make the default limit 2, but have another initializer that takes a max.

    And then, you could even get the values by having getters that return the actual float pointers. float *AEMonitorGetPeakLevels(monitorModule) and int AEMonitorGetMaxChannelCount(monitorModule).

    I'd advise against that part, because they could change at any point. Could lead to some weird behaviour?

    Oh, I also would prefer to have the linear values, not converted to dB. That's better done in the actual UI part that displays such a meter, me thinks.

    Yeah, that's probably a good idea. AEDSPUtilities has ratio to dB conversion functions, so it's pretty easy.

    +1 to the entire discussion

    weird behaviour for concurrent fetching: a mono signal distributed over a stereobuffer may display unequal levels...

    Another minor argument for using a ringbuffer:
    https://github.com/32Beat/TAAE2/blob/develop/TAAESample/Classes/AERingBufferModule.m

Sign In or Register to comment.