Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fluid.bufstats~ should treat a buffer of zeroes like any other equal values #308

Open
rconstanzo opened this issue Jun 26, 2022 · 7 comments
Labels
suggestion Possible enhancement

Comments

@rconstanzo
Copy link

As discussed in #198, there is a use case when loudness-weighting descriptors and analyzing digital silence where fluid.bufstats~ throws up a yellow Invalid Buffer error message.

I'm of the opinion that this is an engineering problem rather than a user problem where what the user is asking for (in this case, loudness-weighted descriptors) creates a technical edge case that they shouldn't have to think about (similar to rounding numbers for fft sizes, or scaling the internal envelope follower in ampslice so it doesn't start at negative infinity etc....).

I can't read through the code well enough to determine whether this is the case, but I would be surprised if AudioGuide/IRCAM approaches to loudness-weighting throw up similar errors when presented with digital silence.

@rconstanzo
Copy link
Author

I closed #198, but posted this code example there.

Basically there's audio you can't analyze unless you build in some edge-case detection stuff around the objects.

----------begin_max5_patcher----------
799.3ocuVssbaBCD8Y6uBM7LwitAH5S9+nSmLxfvVIXIFPD61LIe6ck.mFm3
1ZhcxKwQqVocOmytK5w4yhVY2q5hPeC8czrYONe1rfIugYiqmEsUtunV1EbK
ZqpqStVEEOrmSs2ErWoqqQ3EjCazzp5TFmzoslaq0FUgs2DbkN5goeqs2Uqb
gKlLZcvj6mMpgrJJB8iCWozUrQaVeaqpvMraFMaQdJgJDILVhPvSDwnLxBBI
gIH4rTFilQhQb7BbLhRWfe41fvqMGh9gbRWF.ic0c2jG4M8z749+DegrSmS1
5pZkaUHHOVIMqithrPBaAmCnkJvXlflJ.Rfm6QLAmNYfKtl.err3Zh1TA6rz
bFaxPO8i.ciZGb12g7U8UUp1mQNUmCzAL9zb.8uwAU0VoKJFtHe0xjIDBl+N
FIO+eyHj2yHrqHiTU2qKW.7BzM35dFsrSUCf.sUIMnkPlTpZ0OzgHSjo7jzj
6OnvHAFOkv4zbg.HuLNXLGuHAbLIICnLbRlm0l1TC9mBg0.XoUV2sQ1n7Dms
usPMTZsrpx0A7A.4NDMIEkxQIDJZIDA+V5eofwe4oAFtXiz.Db3+CiiFNxxF
6NUq2duQ67+1HKKgaDQeQkJfo4sVcIppV5LP69gz8ng67u.o6Tk1zz72qboS
oRmREeJRWgrNHY9u.3oeD9XkPaZ.VwtCcCLjXb4F85Md+Jp0MMdYfcRxldkI
aFkehAIBvHkgy3bLmmmiSow9hF+nDVxDH3rOE9s11W5qFeaWwqo3cZSocWnS
HTsCA3UlFZN1XaBqftmk2uSABfuixKBt1dUiRd+nvcTaygtjSJOruD44DMCr
gO84eBvYqNGM2Jbp.XdyaCCYk29wJ1.yOhigGOEi9SrKAEQaBOE7U9vF74j0
Dmab3mQbf5tKOPhyHPGmL1V3yY9cwWTf8ik9+gNm+Egw2lNufRxEE5ryFjjK
DjoenJygFBYSyCp1tQmCg.FVcmMP.h3vRsYXYn4OpU8f9f+gltHYKzV6fd59
1PZEsOkGMbTKPkld8HaBfCBYXPnwOwoQNfiv7x4OM+2.KncTUB
-----------end_max5_patcher-----------

@weefuzzy
Copy link
Member

Basically there's audio you can't analyze unless you build in some edge-case detection stuff around the objects.

I've said a couple of times now that the reason the object squawks on all-0-weights is that there isn't a sensible default action to take that covers all cases because those numbers could be anything, and a sensible thing to do ends up being task dependent. So we have to leave the responsibility to the caller to either avoid it or decide what to do. If you want to convince us to change it, then we'll need a proposal for a sensible default that would make sense in a very wide range of situations, not just one.

As for whether this is an edge case when dealing with loudness data, I don't think it is: it's inherent to using loudness as a control source that silent frames need to be safely dealt with. I can't think of anything I've done with loudness that hasn't needed this to be handled.

If all you need in this case is for silent frames to be unweighted rather than ditched, then add @outputlow 0.1 (or whatever number) to your bufscale~.

@rconstanzo
Copy link
Author

I've said a couple of times now that the reason the object squawks on all-0-weights is that there isn't a sensible default action to take that covers all cases because those numbers could be anything, and a sensible thing to do ends up being task dependent. So we have to leave the responsibility to the caller to either avoid it or decide what to do. If you want to convince us to change it, then we'll need a proposal for a sensible default that would make sense in a very wide range of situations, not just one.

I'm struggling to think of a case where having the object throw up an error and dump out dummy values is beneficial. Is there a type of weighting where having zeros treated as "all values are equal" would be bad, or even a less-than-ideal? The main (only?) use cases for having weights as inputs I can think of are loudness weighting (the main reason the object was created if I remember right) and pitch confidence. In either of those cases if your loudness/confidence are zeros (loudness more clearly so), the object should just pass on the stats unweighted as at the moment it spits out zeroes (which for loudness would be "pretty loud"), which would actually be bad data.

Just imagining a similar UX for other objects like setting the @minfreq of fluid.bufmelbands~ to something like @minfreq 200 and then worrying or building code for the potential edge case of analyzing something that has frequency content below that.

@weefuzzy
Copy link
Member

The main (only?) use cases for having weights as inputs I can think of are loudness weighting (the main reason the object was created if I remember right) and pitch confidence

These are by no means the only things one could use as weights, because the weights are simply some probability distribution-like bunch of numbers. They could be counts of some things, they could be the results of sampling some generative process. They could be arbitrary weightings tied to time indexes to reflect some domain specific knowledge the coder has about their data.

Loudness and pitch confidence were certainly motivating uses, but the weights were added as a general mechanism that happened to allow those specific things. All it requires is that the numbers in the weights can be made to sum to 1. So, all 0s and things that cross the 0 boundary are bad and require the caller to do some extra work reflecting their knowledge of the task at hand.

it spits out zeroes (which for loudness would be "pretty loud")

Or very quiet, depending on the loudness units. Which bufstats~ can't know.

Just imagining a similar UX for other objects like setting the @minfreq of fluid.bufmelbands~ to something like @minfreq 200 and then worrying or building code for the potential edge case of analyzing something that has frequency content below that.

This is beginning to feel needlessly snarky for something where you can literally get the behaviour you want by changing a single attribute value upstream. melbands::minfreq is just saying 'well, if you're making a filter bank, start here' – it's completely tied to the domain of the object, and interpretable. Even asking for garbage like @minfreq ∞ is easier to reason about than 'a probability distribution for which there is 0 probability of anything happening'.

@rconstanzo
Copy link
Author

So we can agree that when weights of zeros come in that that is problematic. The current solution is to spit out zeros as the output (regardless of the input), which I'm arguing would never be a preferable alternative to just spitting out the stats with no weights applied (another (IMO) "valid" interpretation of the weights).

At minimum, I would think that the current implementation would be such that a red warning should be issued and no output should be passed since that is more in line with the philosophy of the implementation: "there is a problem".

That was a post-flight late night post, so I appoligize for coming across extra sassily.

@weefuzzy
Copy link
Member

I'm much more amenable to treating it as an error, and it turns out that on its first commit, that was indeed what happened. I think it changed because without (yet) having programmatic error handling, it was deemed too disruptive to doing big batch analyses (though spitting out junk instead doesn't really delight me any more than it does you).

So, for me this bumps the need for programmatic error handling up the queue somewhat (because we have a motivating example), and once that's in place it's easy enough to justify just treating this as an error instead.

@weefuzzy
Copy link
Member

That was a post-flight late night post, so I appoligize for coming across extra sassily.

❤️

@jamesb93 jamesb93 added the suggestion Possible enhancement label Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
suggestion Possible enhancement
Projects
None yet
Development

No branches or pull requests

3 participants