search

LI7XI / AudioAnalyzerDocs

Documentations for AudioAnalyzer Plugin.
0 stars 0 forks source link

Changes between AudioAnalyzer v1.1.5 to v2.0 discussion #2

Closed d-uzlov closed 3 years ago

d-uzlov commented 3 years ago

Next release version of the plugin is planned to break compatibility. As plugin contains a lot of options that aren't convenient to use but were preserved for the sake of compatibility, this next version will bring a lot of changes. I believe it would be best to isolate information of these changes in one issue. After next public release this issue should be closed, all further changes will have separate discussion threads.

I will post new comments here when I change anything. Also I will post my thoughts on possible changes. Everyone interested are welcome to make suggestions for what should be changed.

d-uzlov commented 3 years ago

Handler BandResampler: Options startCascade and endCascade are removed.

d-uzlov commented 3 years ago

Handler BandResampler has several properties that allow user to get number of bands and get band bounds and central frequency. They will be removed due to inconvenient syntax and vague use-cases.

Proposed replacement: Property BandsCount [no parameters] : integer — correspond to number specified in bands option of BandResampler. Property BandLowBound [params: integer index] : float — equivalent of current lower bound property. Index must be in range [0, BandsCount], where BandsCount would correspond to upper bound of last band.

I'm not sure if central frequency property is needed. It can be calculated as (BandLowBound(ind) + BandLowBound(ind+1)) / 2. Research is required if it would be inconvenient to do so in context of Rainmeter.

LI7XI commented 3 years ago

Handler BandResampler has several properties that allow user to get number of bands and get band bounds and central frequency. They will be removed due to inconvenient syntax and vague use-cases.

I went to check them but the log message is just empty, even when adding delay before logging. Btw i noticed that Resolve function is case sensitive, e.g. it won't work if first letter is capital.

In section variables, i suggest that we remove Value and HandlerInfo arguments. Don't get me wrong, they are useful in some cases, but the only time we used them practically is to get the Spectrogram image. But even then, we found it more convenient to use a child measure than section variables.

P.S I mean we leave section variables for hardware infos only, in our case: audio device, sample rate, etc.

Also in Value argument, i have some concerns regarding performance, idk, is it better to not use child measures at all, and use Value argument to retrieve the value from the handler directly? i mean which one is faster, child measures or section variables?

I want to suggest some conventions to add in the docs, for example:

I still think there is a better words than Processing but idk what it is. Also i thought about something like:

Processing=Main | Channel ... | OtherParameters ...
; or
Processing=Name Main | ...

And we can create a new processes that way. But it looks confusing.

LI7XI commented 3 years ago

Btw.. Are we talking about AudioAnalyzer 2.x.x?😃 since the plugin follows semantic versioning.

d-uzlov commented 3 years ago

I went to check them but the log message is just empty, even when adding delay before logging.

Something like [!Log "[&Measure001_Parent:resolve(handlerInfo, channel auto | handler resampler | data lower bound 10)]"] works for me.

Btw i noticed that Resolve function is case sensitive, e.g. it won't work if first letter is capital.

Yeah, it's a Rainmeter thing. I can do little about it. I believe I could change the case (like, make it work only with Resolve or RESOLVE or ReSoLvE), but I can't make it case-insensitive.

In section variables, i suggest that we remove Value and HandlerInfo arguments.

I don't see a reason to. Value allow you to skip child measure creation when you don't really need a measure, like when using Calc measure of Shape meter. HandlerInfo is just extensible way to get values from handlers. Physically I can transform this to fixed-feature set, but mentally it would be too painful for me to remove a good-working mechanism. It costs me nothing to support them because Child measures use the same thing, just with different syntax, so I don't see a reason to remove them.

Also in Value argument, i have some concerns regarding performance, idk, is it better to not use child measures at all, and use Value argument to retrieve the value from the handler directly? i mean which one is faster, child measures or section variables?

I believe they are roughly the same. Section variables way have some overhead on parsing each time, child measures have some overhead in Rainmeter itself. Overall I would say that it probably doesn't matter, because you loose much-much more on drawing meters, for example. And if you use spectrogram or waveform then you certainly loose incomparably more time on images.

Using child measure to get the image from handlers.

Yeah. Isn't it already there?

FFT -> BandResampler -> BandCascadeTransormer (because it makes more sense this way).

I think that full short guide to simple spectrogram/spectrum skin would be better, because such short advises won't really help users who don't know how this works.

I still think there is a better words than Processing but idk what it is. Also i thought about something like:

  1. I thought about replacing word "Processing" with something else generic. Like, "unit", "module". But I'm not sure if it will be confusing.
  2. Rainmeter doesn't give plugins a way to enumerate its options. There must be an option with fixed name that enumerates processings, and from there plugin can parse other options with dynamic names. It's possible to define all the processings in one option, without separating them into name-list and several named options, but I think if would be very inconvenient.

Btw.. Are we talking about AudioAnalyzer 2.x.x?😃 since the plugin follows semantic versioning.

Next will be 1.2. Version 2.x would be named AudioAnalyzer2. That is, if the plugin will ever need to break compatibility again. Though, I don't see how version number matters.

P.S. Though, I wonder if it would be better to make it AudioAnalyzer2.

d-uzlov commented 3 years ago

Proposal: link handlers in processing description.

Current syntax:

Processing-proc= … handlers fft, resampler, transformer …
Handler-resampler= source fft …
Handler-transformer source resampler …

Proposed syntax:

Processing-proc= … handlers fft, resampler(fft), transformer(resampler) …
LI7XI commented 3 years ago
Click to expand >Something like [!Log "[&Measure001_Parent:resolve(handlerInfo, channel auto | handler resampler | data lower bound 10)]"] works for me. oops, i forgot the word `Data` :P, also i wrote `HandlerName` instead of `handler`. The problem is, i wrote the whole documentation this way lol. --- >but I can't make it case-insensitive. Alright, just make it `Resolve` and we will mention that it's case-sensitive. --- >It costs me nothing to support them because Child measures use the same thing, just with different syntax, so I don't see a reason to remove them. >I believe they are roughly the same. Oh, alright then. I thought they are making more overhead than being a feature. --- >Yeah. Isn't it already there? Yep, i mean this should be the main way of doing it, i guess we fixed this already so never mind :P --- >I think that full short guide to simple spectrogram/spectrum skin would be better, because such short advises won't really help users who don't know how this works. I agree. I'm thought about making a whole new section just for examples, similar to tips section but for how to actually use multiple processes and how to retrieve more than one value from a single handler, etc.. --- >Like, "unit", "module". You are genius, what about `ProcessingUnits`? yeah it's kinda long but sounds more descriptive. But then, what about process description? `ProcessingUnit-Proc1= ... | ...` doesn't look good. --- >It's possible to define all the processings in one option, without separating them into name-list and several named options, but I think if would be very inconvenient. Just to let you know, the current way is perfect, the only concern i have is the word `Processing` because it sounds like a verb. >P.S. Though, I wonder if it would be better to make it AudioAnalyzer2. As i understand, semantic versioning works like the following: Major.Minor.BugFixes - Major update may come up with breaking changes. - Minor adds features without breaking changes (or breaking changes rarely happen in minor versions). - BugFixes, well, it's just for fixes. So i guess yeah, it's better to make it v2. Btw, are you planning to make a feature that require a lot of changes? it's better to take more time and do it now than later. --- >Proposed syntax: You mean getting rid of `Source` parameter? 😶 Um, tbh the current one looks better, actually it's pretty good (about the handlers). Also `Source` parameter make it easier to organize and see which goes where. One last thing, in Processings, `Handlers h1, h2, h3, ...`, what if `h3` is the source for `h2`? does it work? I mean does the arrangement in `Handlers` parameter actually matters? >And we can create a new processes that way. But it looks confusing. What do you think of this one?
d-uzlov commented 3 years ago

Alright, just make it Resolve and we will mention that it's case-sensitive.

Do you think that starting from capital letter is better? I'm always too lazy to press shift every time. Through, while I can't make it fully case-insensitive, I guess I can make 2 functions: resolve and Resolve. By the way, do you think that word "resolve" is fitting here, or should it be changed?

But then, what about process description? ProcessingUnit-Proc1= ... | ... doesn't look good.

It could be just unit-proc1= …

So i guess yeah, it's better to make it v2.

Well, then name must be AudioAnalyzer2, because Rainmeter doesn't have another way of breaking changes versioning. Alternatively name can be fully changed.

Btw, are you planning to make a feature that require a lot of changes? it's better to take more time and do it now than later.

Well, even current version of AudioAnalyzer (for example, that alpha1 version) is already incompatible with the old version. The main question is with naming.

What do you think of this one?

If you are talking about syntax like Processing=Name Main | … instead of Processing-Main=…, then it's impossible, because options must have unique names.

Um, tbh the current one looks better, actually it's pretty good (about the handlers). Also Source parameter make it easier to organize and see which goes where.

Well, I for one usually forget to change all the necessary things when adding or removing handlers. Let's say I have the following:

handler-transformer=source resampler
handler-spec=source transformer

It's not unusual to me to write something like this:

handler-transformer=source resampler
handler-time=source transformer
handler-spec=source transformer

and then spend some time debugging it, because time handler doesn't work, because I forgot to change source of spec handler, because source suboptions are scattered throughout all of the handler descriptions. Also it's very easy to change source of some handler but forget to remove it from handler list in processing description.

In new syntax it would look like this: changing from handlers fft, resampler(fft), transformer(resampeler), spec(transformer) to handlers fft, resampler(fft), transformer(resampeler), time(transformer), spec(time), which is at least guaranteed to be on one line, so it's easier to keep track of.

I mean does the arrangement in Handlers parameter actually matters?

Yes, order of handlers matters.

LI7XI commented 3 years ago

Do you think that starting from capital letter is better? I'm always too lazy to press shift every time.

Me too but i'm following the rainmeter convention, all options and parameters start with Upper-case letter, now i'm used to it.

By the way, do you think that word "resolve" is fitting here, or should it be changed?

Yes, resolving 1 or more arguments. It makes sense.

It could be just unit-proc1= …

I like it. So its something like:

ProcessingUnits= Proc1 | ...
Unit-Proc1= ... | ...

Well, then name must be AudioAnalyzer2, because Rainmeter doesn't have another way of breaking changes versioning. Alternatively name can be fully changed.

The file name? no, actually the main reason we are removing backward compatibility is the naming. Also no need to worry about upcoming changes, since we have MagicNumber right?

If you are talking about syntax like Processing=Name Main | … instead of Processing-Main=…, then it's impossible, because options must have unique names.

Oh i forgot, Processing must be something like Processing1=, Processing2=. But never mind, we will go with ProcessingUnits approach.

In new syntax ... keep track of.

I like the idea, but that would make the parameter way longer. In one of my visualizers i generate the handler dynamically, and if we added the length of specifying there source it would be even longer, of course it doesn't matter that much, but when looking for what went wrong, this is a nightmare.

Yes, order of handlers matters.

Another thing that annoys me is when i comment a handler description, currently:

Processing=Main
Processing-Main=Channels Auto | Handlers MainFFT, MainBR, MainBCT, MainTR, MainFinalOutput | TargetRate 4000 | Filter like-a

Handler-MainFFT=Type FFT
Handler-MainBR=Type BandResampler 
Handler-MainBCT=Type BandCascadeTransformer
; Handler-MainTR=Type TimeResampler 
Handler-MainFinalOutput=Type UniformBlur

If Handler-MainTR is commented, but it's still present in processing-main then suddenly nothing will work.

I know this might be intended, but if we used the new syntax, can't we do something like Handlers MainFFT, MainBR(mainfft), MainBCT(mainbr), MainTR1(mainbct), MainFinalOutput(maintr) and change the source for mainfinaloutput to maintbct and it works just fine? with additionally logging a message that there a handler that doesn't have a description?

Also, when there is a Handler-handlerName but that handlerName is not specified in Handlers parameter, can we just ignore this handler description? (if this sounds better then ignore the sentence before it)

d-uzlov commented 3 years ago

The file name? no, actually the main reason we are removing backward compatibility is the naming.

Well, that's because "_1_1" suffix is very inconvenient. Suffix "2" could be more tolerable.

Also no need to worry about upcoming changes, since we have MagicNumber right?

Well, MagicNumber can be easily used to to adapt skins for small changed, but if changes are major then a significant portion of the sources must be duplicated for different plugin behavior. Even 1.1.5 version partially had this, for example.

Oh i forgot, Processing must be something like Processing1=, Processing2=.

I don't think that using numbers (that are hard to remember, easy to mess up, hard to support) are better than human-readable names.

I like the idea, but that would make the parameter way longer. In one of my visualizers i generate the handler dynamically, and if we added the length of specifying there source it would be even longer, of course it doesn't matter that much, but when looking for what went wrong, this is a nightmare.

It's not like there will be more options. You already have to manage sources in handler description. With new syntax you will just have to do it in one place, instead of many separated places.

If Handler-MainTR is commented, but it's still present in processing-main then suddenly nothing will work.

Yes, because processing description is the place where you specify how the processing will work. Plugin knows nothing about it's options, because Rainmeter doesn't have any way for a plugin re enumerate them. You can add or remove however many you want garbage options with arbitrary names, but if you have specified something in the processing description, then there must be a corresponding option.

I know this might be intended, but if we used the new syntax, can't we do something like Handlers MainFFT, MainBR(mainfft), MainBCT(mainbr), MainTR1(mainbct), MainFinalOutput(maintr) and change the source for mainfinaloutput to maintbct and it works just fine? with additionally logging a message that there a handler that doesn't have a description?

It already works that way.

Also, when there is a Handler-handlerName but that handlerName is not specified in Handlers parameter, can we just ignore this handler description? (if this sounds better then ignore the sentence before it)

Yes, plugin knows nothing about it's options.

d-uzlov commented 3 years ago

Change: Parent measure now have BlockCaptureLoudnessChange option. Type: enum. Possible values: Never, ForApp. Default value: Never. When set to ForApp, sets application volume level to maximum and prevents its change. Doesn't support dynamic variables.

LI7XI commented 3 years ago

Well, that's because "_1_1" suffix is very inconvenient. Suffix "2" could be more tolerable.

Plz, without a suffix, that would annoy me to death. :( Mainly because it doesn't look cool.

but if changes are major then a significant portion of the sources must be duplicated for different plugin behavior.

Are you planning (in long term) to implement a feature that require massive changes?

I don't think that using numbers (that are hard to remember, easy to mess up, hard to support) are better than human-readable names.

I know, i was talking about the rainmeter way of doing it, e.g. Shape meter.

With new syntax you will just have to do it in one place, instead of many separated places.

Yeah but when using Source it looks more clear what goes where, but the idea looks cool overall.

2 things:

  1. Is it easy to implement this? so we can test it out and see how it works.
  2. Can we keep both ways? at least for testing purposes (i mean specifying the source using the parameter or using the parentheses (the new syntax)).
LI7XI commented 3 years ago

Parent measure now have BlockCaptureLoudnessChange option.

Documented. But i think the name is a bit long, what about PreventVolumeChange? since we will explain what this option does anyway.

Btw, i started taking a second look on the documentations and collected few notes:

Todo:

Questions:

I will add more notes when i find them. For now, the only pages i updated: Home, Introduction, and Why using AudioAnalyzer.

For the introduction page, it needs a rewrite, but i don't know what to write there, also you mentioned there is AudioLevel2, what is that? you mean AudioLevelBeta?

d-uzlov commented 3 years ago

Are you planning (in long term) to implement a feature that require massive changes?

Well, to be honest, it's not like I have a long term plan at all. Also, I can freely add new features without breaking compatibility. Problems appear when some of the old features are badly designed, and I must somehow either work around this bad design or add new feature with different syntax and support both old and new feature.

I know, i was talking about the rainmeter way of doing it, e.g. Shape meter.

Well, I don't think that all of the Rainmeter design choices are good. In fact, I do know many things that could be implemented much better even without adding complexity. Although I'm not blaming Rainmeter devs for this, as it's always easier to see such things in hindsight, I don't think that Rainmeter design choices is something to take as an example. For example, this naming of name1=, name2=. Hard to use, hard to read, easy to break.

  1. Is it easy to implement this? so we can test it out and see how it works.

Yeah, it's easy. I think any changes to the syntax are easy, as long as they only affect how options are parsed, and not how something works.

  1. Can we keep both ways? at least for testing purposes (i mean specifying the source using the parameter or using the parentheses (the new syntax)).

Well, we can do it temporarely.

Documented. But i think the name is a bit long, what about PreventVolumeChange? since we will explain what this option does anyway.

Well, I wanted to use something like this, but PreventVolumeChange sounds like Rainmeter will block volume that you hear, not one that it captures.

  • Remove the word "callback" from Callback-OnUpdate and Callback-OnDeviceChange in parent options.

I think that it's easier to understand with the word callback. Also, Callback-OnUpdate is deprecated.

  • Since there will be 255RGB support, what about calling that option Use8BitRGB instead of UseLegacyRGB?

Well, plugin always uses 8-bit RGB. And I really want to make sure that everyone understands that range [0, 255] is legacy.

  • What's the difference between Callback-OnUpdate and rainmeter's OnUpdateAction?

OnUpdateAction is fired when rainmeter updates measure. Callback-OnUpdate is fired when values are updated. When you are using a separate thread, these are different things.

also you mentioned there is AudioLevel2, what is that? you mean AudioLevelBeta?

No. IIRC, on Rainmeter forum in AudioLevel thread there was a post of an improved version of the AudioLevel (they later renamed it into AudioLevel2 because it conflicted with built-in plugin version), that were able to draw waveform. That's kinda where I got an idea of writing images to disk.

LI7XI commented 3 years ago

Well, I don't ... take as an example.

Yep i know.

For example, this naming of name1=, name2=. Hard to use, hard to read, easy to break.

It works well for simple stuff, like MeasureName MeasureName2 or Shape Shape2. But in case of a plugin it would be confusing.

Well, I wanted to use something like this, but PreventVolumeChange sounds like Rainmeter will block volume that you hear, not one that it captures.

I thought the same but is there an alternate for the word "block" and "capture"? something that points to the audio control panel.

I think that it's easier to understand with the word callback.

But it feels redundant, any option that starts with "On" is known to be used with bangs. At least in OnDeviceChange.

Also, Callback-OnUpdate is deprecated.

Idk if it's still useful but should we remove it? or keep it and point out the difference between it and OnUpdateAction?

Callback-OnUpdate is fired when values are updated. When you are using a separate thread, these are different things.

Fired "only" when using separate thread? or with ui thread as well but then it acts like OnUpdateAction?

d-uzlov commented 3 years ago

I thought the same but is there an alternate for the word "block" and "capture"? something that points to the audio control panel.

Well, I can live with something like LockCaptureVolumeOnMax, for example. I can't really think of any simple alternative to word "capture", though. If we were to use Windows WASAPI terminology, I'm sure we could describe it in other words, but it would sure be much more confusing.

But it feels redundant, any option that starts with "On" is known to be used with bangs. At least in OnDeviceChange.

I would gladly trade slight verbosity in exchange for readability. This callback- prefix really makes it easy to find and recognize options that set callbacks.

Also, Callback-OnUpdate is deprecated.

Idk if it's still useful but should we remove it? or keep it and point out the difference between it and OnUpdateAction?

When I tested it, it increased CPU usage several times. There is nothing wrong with it, and I still sometimes use it for testing, but I think end-users shouldn't even know about it.

Fired "only" when using separate thread? or with ui thread as well but then it acts like OnUpdateAction?

Fired even when not using separate thread. I didn't check Rainmeter docs thoroughly, but in case of uiThread it probably acts the same as OnUpdateAction. The main purpose of this option was to allow creation if skins that would only be updated when there really is new audio data. Mainly to reduce CPU usage. Unfortunately, it didn't work as I expected, but I only found it out after the 1.1.5 release.

LI7XI commented 3 years ago

If we were to use Windows WASAPI terminology, I'm sure we could describe it in other words, but it would sure be much more confusing.

Alright,LockCaptureVolumeOnMax sounds good.

When I tested it, it increased CPU usage several times.

Yeah same here, i did an experiment on updating measures and meters using this option (!UpdateMeasure/MeterGroup !Redraw) but CPU usage increased and the skin kept lagging even at Update=50, while using OnUpdateAction caused less lag. I did this long time ago so i don't remember the exact results.

There is nothing wrong with it, and I still sometimes use it for testing, but I think end-users shouldn't even know about it. The main purpose of this option was to allow creation if skins that would only be updated when there really is new audio data. Mainly to reduce CPU usage. Unfortunately, it didn't work as I expected, but I only found it out after the 1.1.5 release.

I suggest we remove it.

Plugin knows nothing about it's options, because Rainmeter doesn't have any way for a plugin re enumerate them.

B..bb...but.. how the plugins knows there is an option to read values from? >-< I mean the plugin will look for an option named Something right? in case of it's required as example.

Btw, can you take a look at this?

LI7XI commented 3 years ago

HandlerName option in child measures doesn't work, even when adding ProcessName option. Here is the skin:

```ini [Rainmeter] Update=32 BackgroundMode=2 SolidColor=0,0,0,100 [MeasureAudio] Measure=Plugin Plugin=AudioAnalyzer Type=Parent MagicNumber=104 Processing=Main Processing-Main=Channels Auto | Handlers MainFFT, MainResampler, MainTransform, MainFilter, MainMapper | filter like-a Handler-MainFFT=Type FFT | BinWidth 5 | OverlapBoost 10 | CascadesCount 3 Handler-MainResampler=Type BandResampler | Source MainFFT | Bands log 10 20 2000 Handler-MainTransform=Type BandCascadeTransformer | Source MainResampler | MinWeight 0.01 | TargetWeight 10 Handler-MainFilter=Type TimeResampler | Source MainTransform | Attack 100 Handler-MainMapper=Type ValueTransformer | Source MainFilter | Transform db map[from -50 : -0] clamp [MeasureBand0] Measure=Plugin Plugin=AudioAnalyzer Type=Child Parent=MeasureAudio Channel=Auto Index=0 HandlerName=MainMapper [MeasureBand1] Measure=Plugin Plugin=AudioAnalyzer Type=Child Parent=MeasureAudio Channel=Auto Index=1 HandlerName=MainMapper [MeasureBand2] Measure=Plugin Plugin=AudioAnalyzer Type=Child Parent=MeasureAudio Channel=Auto Index=2 HandlerName=MainMapper [MeasureBand3] Measure=Plugin Plugin=AudioAnalyzer Type=Child Parent=MeasureAudio Channel=Auto Index=3 HandlerName=MainMapper [MeasureBand4] Measure=Plugin Plugin=AudioAnalyzer Type=Child Parent=MeasureAudio Channel=Auto Index=4 HandlerName=MainMapper [MeasureBand5] Measure=Plugin Plugin=AudioAnalyzer Type=Child Parent=MeasureAudio Channel=Auto Index=5 HandlerName=MainMapper [MeasureBand6] Measure=Plugin Plugin=AudioAnalyzer Type=Child Parent=MeasureAudio Channel=Auto Index=6 HandlerName=MainMapper [MeasureBand7] Measure=Plugin Plugin=AudioAnalyzer Type=Child Parent=MeasureAudio Channel=Auto Index=7 HandlerName=MainMapper [MeasureBand8] Measure=Plugin Plugin=AudioAnalyzer Type=Child Parent=MeasureAudio Channel=Auto Index=8 HandlerName=MainMapper [MeasureBand9] Measure=Plugin Plugin=AudioAnalyzer Type=Child Parent=MeasureAudio Channel=Auto Index=9 HandlerName=MainMapper [MeterStyles] X=15r Y=0 W=10 H=100 BarColor=255,255,255,255 BarOrientation=Vertical [MeterBand0] Meter=Bar MeasureName=MeasureBand0 X=0 Y=0 W=10 H=100 BarColor=255,255,255,255 BarOrientation=Vertical [MeterBand1] Meter=Bar MeterStyle=MeterStyles MeasureName=MeasureBand1 [MeterBand2] Meter=Bar MeterStyle=MeterStyles MeasureName=MeasureBand2 [MeterBand3] Meter=Bar MeterStyle=MeterStyles MeasureName=MeasureBand3 [MeterBand4] Meter=Bar MeterStyle=MeterStyles MeasureName=MeasureBand4 [MeterBand5] Meter=Bar MeterStyle=MeterStyles MeasureName=MeasureBand5 [MeterBand6] Meter=Bar MeterStyle=MeterStyles MeasureName=MeasureBand6 [MeterBand7] Meter=Bar MeterStyle=MeterStyles MeasureName=MeasureBand7 [MeterBand8] Meter=Bar MeterStyle=MeterStyles MeasureName=MeasureBand8 [MeterBand9] Meter=Bar MeterStyle=MeterStyles MeasureName=MeasureBand9 ```
d-uzlov commented 3 years ago

I suggest we remove it.

Yeah, that's what I suggest too.

B..bb...but.. how the plugins knows there is an option to read values from? >-<

Well, I guess I made it sound too strict. A plugin can request an option with a certain name. To do so, it must calculate the name somehow. The point is, if you remove handler name from handler list in processing description, then it doesn't matter if you did anything with the corresponding option, because plugin can't read it without knowing its name.

Btw, can you take a look at this?

I will see what I can do about sound compression and dynamic offsets. I will search for an appropriate window function.

d-uzlov commented 3 years ago

HandlerName option in child measures doesn't work, even when adding ProcessName option. Here is the skin:

Oh, apparently I renamed it to just Handler long time ago, and forgot to update the docs. Whoops.

LI7XI commented 3 years ago

A plugin can request an option with a certain name. To do so, it must calculate the name somehow.

Oh, now i got it :D

I know this might be intended ... have a description?

What i meant there is, can we change the handler name in the Processing (something like HandlerName1) so the plugin can't find it, but still keep the process working? Right now if the handler description is not specified, the process will simply not work (i know this is intended).

I will see what I can do about sound compression and dynamic offsets. I will search for an appropriate window function.

Thanks but tbh i don't get it, not about what you said. It's about what sorcery after effects does until it can create a correct FFT visualization. I mean, it doesn't have FFT limitations, it outputs a high res in both high and low frequencies, while still updating them both really fast, at same time.

Oh, apparently I renamed it to just Handler long time ago, and forgot to update the docs. Whoops.

HandlerName seems more descriptive, is there a reason for the change? 😅

d-uzlov commented 3 years ago

What i meant there is, can we change the handler name in the Processing (something like HandlerName1) so the plugin can't find it, but still keep the process working? Right now if the handler description is not specified, the process will simply not work (i know this is intended).

Well, as you said, this is intended behaviour. Processing description that can't find specified handler is ill-defined. Previously there were no such check, and in case an essential handler was missing, there was an array or errors in the log ("handler X didn't find its source Q", "handler Y didn't find its source X", "handler Z didn't find its source Y"), nothing worked either way, and the part that still worked consumed CPU for nothing, because the rest didn't work. If user said that processing should use a handler, and this handler is not found or ill-defined, then user has made a mistake, and plugin is trying to help user to find this mistake early.

I think you use it the wrong way. If you want to remove handler from a processing unit, you can remove it's name from the handler list. Handler description option can hang in there however long you need. If you feel that it's too much hustle to edit the handler list the remove and re-add names, then I guess it's possible to add some special syntax to temporarily disable a handler without removing its name from the list, but I'm not sure how useful this would be.

I mean, it doesn't have FFT limitations, it outputs a high res in both high and low frequencies, while still updating them both really fast, at same time.

Oh. Who knows. I have no idea. It's probably using something like what I call FFT cascades, but better. The fact that I didn't find anything about such things in the google doesn't mean that some proprietary plugins don't use this. Or maybe even something open sourced use this, and I just don't know how to google properly.

HandlerName seems more descriptive, is there a reason for the change? 😅

I don't remember. I changed it long time ago. It was even before 1.1.5 release, if I'm not mistaken. It was probably because new variant is shorter, and there isn't really anything but name that you can think of using here.

d-uzlov commented 3 years ago

Change: option renamed: blockCaptureLoudnessChange → LockCaptureVolumeOnMax Change: option renamed: UnusedOptionsWarning → LogUnusedOptions Change: option renamed: Processing → ProcessingUnits Change: option ProcessingUnits uses , as separator instead of |. Example: ProcessingUnits=unit1, unit2 Change: option renamed: Processing-<name> → unit-<name> Change: new syntax for handler linking.

Change: sub-option with their own named sub-options use different separators:

Change: BandResampler: bands option now use syntax similar to that of transforms

Change: BandResampler: new handler info requests:

Change: error detection is stricter:

d-uzlov commented 3 years ago

1.2-alpha2 version, which includes all of the changes above, is posted in the Releases page of the plugin repository

d-uzlov commented 3 years ago

Change: sub-option with their own named sub-options use different separators:

The reason for this change is that rainmeter doesn't have much constraints on what a name of a meter/measure can be. You can name a measure [3], and then when you write something like kaiser[3] it won't be parsed correctly. So [] is not the best choice for separating parameters.

However, I'm not sure if parentheses are the best choice. Parentheses are also used in math operations. While the plugin doesn't restrict usage of parentheses inside a sub-option, it can potentially be confusing. For example, it's valid to write this: bands log(count 300, min 20, max (10^4) * 2) But if you make a mistake, then something like bands log(count 300, min 20, max (10^4)) * 2) won't be parsed correctly. It will give an error message in the log, leading to non-parsable option, though.

An alternative would be to use curly braces {}. I'm not sure if they are the better choice.

LI7XI commented 3 years ago
> Change: option renamed: `blockCaptureLoudnessChange` → `LockCaptureVolumeOnMax` > Change: option renamed: `UnusedOptionsWarning` → `LogUnusedOptions` > Change: option renamed: `Processing` → `ProcessingUnits` > Change: option `ProcessingUnits` uses `,` as separator instead of `|`. Example: `ProcessingUnits=unit1, unit2` > Change: option renamed: `Processing-` → `unit-` > Change: sub-option with their own named sub-options use different separators: This includes: transforms, filters, windowFunction > Change: BandResampler: `bands` option now use syntax similar to that of transforms > Change: BandResampler: new handler info requests: > Everything in the old syntax is removed, new requests are: ... All done. --- >Change: option ProcessingUnits uses , as separator instead of |. Example: ProcessingUnits=unit1, unit2 IMHO the pipe symbol looks better. I mean we use it to separate parameters, and parameters are different from each other. But we use commas to to set related arguments together, handler names as an example (`HandlerName1, HandlerName2`) --- >Change: BandResampler: bands option now use syntax similar to that of transforms >Example: bands log(count 300, min 20, max 20000) I suggest to rename Min to `FreqMin`, and `Max` to `FreqMax`. It would be recognizable on the fly and it's more descriptive. --- >Change: new syntax for handler linking. >1.2-alpha2 version, which includes all of the changes above, is posted in the Releases page of the plugin repository Under testing. --- >Change: error detection is stricter: Now if any handler or channel can't be parsed properly, the whole plugin doesn't work. Will be mentioned in tips discussion. >It was probably because new variant is shorter, and there isn't really anything but name that you can think of using here. `for i = 0; i < infinity ; cout << "Plz rename it to HandlerName"`. It's easier to understand and explain. --- >An alternative would be to use curly braces {}. I'm not sure if they are the better choice. Nope, parentheses are good. Also i like the (parentheses instead of brackets) idea a lot. >But if you make a mistake, then something like bands log(count 300, min 20, max (10^4)) * 2) won't be parsed correctly. >It will give an error message in the log, leading to non-parsable option, though. Hmm, since it will give an error message, i guess it would be easier to spot the issue if that message tells you exactly what parameter causes it. Here is what i think, if the plugin somehow can read the outer parentheses first, like: `Log -> () <-` lvl 1 parentheses `Log (ArgName1 -> () <- , ArgName2 ... )` Lvl 2 parentheses And there is something like `Log(ArgName1 (1+1)), ArgName2 ...)` Then the plugin would know there is an issue in `ArgName1` and simply tells you about which argument. Idk how that could be done but it will make it so much easier to hunt mistakes.
d-uzlov commented 3 years ago

Change: list of color spaces changed

Change: new syntax for specifying color space for single color value:

Change: new option: DefaultColorSpace

Change: option renamed: handler in Child measure → handlerName Also note, that long deprecated ValueId option is no longer parsed in child measure.

Option parsing has become stricter in this version. Log messages are now more informative in some cases.

d-uzlov commented 3 years ago

I suggest to rename Min to FreqMin, and Max to FreqMax. It would be recognizable on the fly and it's more descriptive.

I will rename it later.

Here is what i think, if the plugin somehow can read the outer parentheses first, like:

Log -> () <- lvl 1 parentheses Log (ArgName1 -> () <- , ArgName2 ... ) Lvl 2 parentheses

And there is something like Log(ArgName1 (1+1)), ArgName2 ...)

Then the plugin would know there is an issue in ArgName1 and simply tells you about which argument. Idk how that could be done but it will make it so much easier to hunt mistakes.

Well, that's part of the problem with using parentheses both for math expressions and as option delimiters. I don't think there is a simple and/or reliable way to detect and report issues like opt(arg1() ) , arg2). Plugin sees this as opt(arg1() ) , arg2), and arg2) is a separate invalid value. Plugin will report that it can't parse an option past this point, but it can't tell that you have made a mistake in arg1: for this you first need to correctly parse the outer option first. If we were to use different separators, like {}, then this would be parsed correctly, and then it could report an issue with certain inner option.

LI7XI commented 3 years ago

I got this log message "unexpected (handlerSpecificData == nullptr), tell the developer about this error" when using these settings:

```ini ProcessingUnits=Main Unit-Main=Channels Auto | Handlers FFT, Resampler(FFT), CascadeTransformer(Resampler), UniformBLur(CascadeTransformer), ReMaper(UniformBLur), Transformer(UniformBLur), Spectrogram(Transformer), Waveform | Filter none Handler-FFT=Type FFT | BinWidth 10 | OverlapBoost 40 | CascadesCount 4 | WindowFunction Hann ; -------------------------------------------------------------------------------------- Handler-Resampler=Type BandResampler | Bands Linear(Count 150, Min 20, Max 20000) | CubicInterpolation true ; -------------------------------------------------------------------------------------- Handler-CascadeTransformer=Type BandCascadeTransformer | MixFunction Average | MinWeight 0.01 | TargetWeight 2 | ZeroLevelMultiplier 1 ; -------------------------------------------------------------------------------------- Handler-UniformBLur=Type UniformBlur | Radius 1 | RadiusAdaptation 1 ; -------------------------------------------------------------------------------------- Handler-ReMaper=Type TimeResampler | Attack 20 | Decay 60 | Granularity ([#UpdateRate]*2) | Transform dB, Map(From -41 : -0, to [#MinHeight] : [#MaxHeight]), Clamp(Min [#MinHeight], Max [#MaxHeight]) ; -------------------------------------------------------------------------------------- Handler-Transformer=Type ValueTransformer | Transform Clamp ; -------------------------------------------------------------------------------------- Handler-Spectrogram=Type Spectrogram | Folder [#@]Images/ | Length 300 | Resolution 35 | SilenceThreshold -70 | Stationary false | BorderSize 1 | DefaultColorSpace sRGB | MixMode sRGB255 | FadingRatio 0.2 | BorderColor @sRGB255 255, 171, 92 | Colors 0.0: @hsl 217,0.38,0.11 ; 1: @hsl 29, 0.96, 0.62 ; -------------------------------------------------------------------------------------- Handler-Waveform=Type Waveform | Folder [#@]Images/ | Width 300 | Height 200 | Stationary false | BorderSize 1 | BorderColor 255, 64, 89 | Resolution 0.6 | Connected true | DefaultColorSpace sRGB255 | BackgroundColor @hsl 237,0.34,0.20 | WaveColor 255, 64, 89 | LineColor @sRGB 0.5,0.5,0.5 | FadingRatio 0.2 | LineDrawingPolicy Never | SilenceThreshold -70 Threading=Policy SeparateThread | UpdateRate ([#Fps]*3) LockCaptureVolumeOnMax=Never ```

I will upload the full skin soon.

LI7XI commented 3 years ago
>Option parsing has become stricter in this version. Log messages are now more informative in some cases. Is it possible to add a different log message for overridden parameters? For example, i used `Color` parameter in `Spectrogram`, but then i forgot and added `BaseColor` parameter, i got a log message about unused options, at first i thought the option was removed or i misspell it, then i remembered it's ignored when using `Color` parameter. It would be easier if this case have a different log message. --- >If we were to use different separators, like {}, then this would be parsed correctly, and then it could report an issue with certain inner option. {} will complicate the syntax, and we are trying to make it simpler, also it's unusual for this kind of usage. >Plugin will report that it can't parse an option past this point, but it can't tell that you have made a mistake in arg1: for this you first need to correctly parse the outer option first. What about reading it as a string before parsing? more like what code linters do. I know this is out of the plugin's scope, but when parsing as a string, you can use some regex magic to find the first from left and the first from right parentheses, and after that you can see in which argument the problem occurred using the `,` separator. (I know how smart i am, thank me later uwu)
LI7XI commented 3 years ago

Change fft implementation from kiss fft to pffft.

Btw i'm surprised that the plugin was using kiss fft, i thought its using pffft already. I wonder what benefits pffft has over kiss fft? 👀 i read that pffft is faster and more lightweight, is there more things (i mean your reasons to switch)?

LI7XI commented 3 years ago

Hello, when using waveform i noticed that the wave line is a bit jagged.

image

the settings i used:

Handler-Waveform=Type Waveform | Folder [#@]Images/ | Width 300 | Height 200 | Stationary false | BorderSize 1 | BorderColor 255, 64, 89 | Resolution 0.6 | Connected true | DefaultColorSpace sRGB255 | BackgroundColor @hsl 237,0.34,0.20 | WaveColor 255, 64, 89 | LineColor @sRGB 0.5,0.5,0.5 | FadingRatio 0.2 | LineDrawingPolicy Never | SilenceThreshold -70

Is there a reason for that or just because i used kinda big resolution in the handler?

d-uzlov commented 3 years ago

I got this log message "unexpected (handlerSpecificData == nullptr), tell the developer about this error" when using these settings:

Is this consistent, or you are only getting this occasionally? To be honest, I can't really do much about this. This message should probably contain more meta info about what happened, but this is an unexpected event, and I don't know what to log to understand what happened, because sometimes I can't do this even with debugger. Though, in the current dev build I was able to consistently get an app crash with these settings, which is certainly something to work on. Yay.

Is it possible to add a different log message for overridden parameters?

I will add such note for color option in spectrogram. Tell me if you encounter other similar places that require special log messages.

What about reading it as a string before parsing? more like what code linters do. I know this is out of the plugin's scope, but when parsing as a string, you can use some regex magic to find the first from left and the first from right parentheses, and after that you can see in which argument the problem occurred using the , separator.

For this you first have to parse it. The most basic thing you need to do is to separate something like name1(), name2((1+2))^3), name3(4) into name1(), name2((1+2))^3) and name3(4). To separate the values, you have to parse them. To parse them, you have to have parentheses correctly placed. As a fun experiment, you can try to drop an extra } into some C++ code, run compiler and check the results.

Btw i'm surprised that the plugin was using kiss fft, i thought its using pffft already.

Well, original AudioLevel used KissFft, and I just didn't bother changing it.

I wonder what benefits pffft has over kiss fft? 👀 i read that pffft is faster and more lightweight, is there more things (i mean your reasons to switch)?

It's seems to be few times faster. That's all.

Hello, when using waveform i noticed that the wave line is a bit jagged. Is there a reason for that or just because i used kinda big resolution in the handler?

Well, waveform doesn't use any antialiasing techniques. It's just what your native monitor resolution looks like. Yay for retina 300+ dpi some 20+ years later, when someone except for Apple make PC monitors with decent resolution. I tried to add antialiasing earlier, but it had some issues, and since I didn't think it improved image too much and was kinda cumbersome to support, I eventually removed it when I was fixing some bugs in waveform, I guess. I guess you can make a bigger image and then downsample it with Rainmeter Image meter options, if you want antialiasing.

LI7XI commented 3 years ago

which is certainly something to work on. Yay.

Sorry😂

Tell me if you encounter other similar places that require special log messages.

Sure.

As a fun experiment, you can try to drop an extra } into some C++ code, run compiler and check the results.

I understand, but please, don't call that nightmare a fun experiment... "You missed a semi-colon in line 35" WTBlyat, my program is only 20 lines lol.

I guess you can make a bigger image and then downsample it with Rainmeter Image meter options, if you want antialiasing.

Alright, i'll try it out. But i'm afraid about performance, since making the handler generate a higher res image will translate to a larger file size, which means rainmeter will read a big image from disk, and my hard drive will suffocate.

LI7XI commented 3 years ago

Hey, i found few things that need fixing:

P.S about Processing option in child measures, you said:

If Proc is not specified, then the plugin will try to find a process that contains the handler specified in Handler argument.

I guess that's unnecessary since we decided that it would be the best to forbid using the same handler name in several processings, so Proc option will likely be removed both from here and from child measure.

So i guess never mind, i'll remove it.

d-uzlov commented 3 years ago
  • Ignore Index option in child measure when the handler name doesn't provide index range. For example when HandlerName=Loudness and Index=3 or other than 0, the index should be treated as 0.

Why would you use Index option on handlers with only 1 value in the first place? I would argue that such Child-type measure is ill-formed, so it shouldn't work. Umm, I guess I'll make checks for index more restrictive, so that when someone specifies incorrect index, plugin stops working until skin refresh.

LI7XI commented 3 years ago

Why would you use Index option on handlers with only 1 value in the first place?

Not me, i mean in case someone did it by mistake. We discussed that before and you agreed on it :[

d-uzlov commented 3 years ago

Not me, i mean in case someone did it by mistake.

Well, in case this is a mistake, it would certainly be much better to detect and fix it early. I see this as an improvement.

d-uzlov commented 3 years ago

v2.0 was released. Change: BandResampler: bands: name change: min → freqMin Change: BandResampler: bands: name change: max → freqMax Change: TimeResampler: granularity → UpdateRate. Value must be in range [1, 200], default value 20 Change: Spectrogram: resolution → UpdateRate. Value must be in range [1, 200], default value 20 Change: Waveform: resolution → UpdateRate. Value must be in range [1, 200], default value 20 Change: transforms: clamp: syntax: old syntax clamp(min 1, max 2), new syntax clamp(to 1 : 2) (now it is the same as map transform syntax) Change: BandCascadeTransformer: zeroLevelMultiplier: option removed Change: BandCascadeTransformer must have BandResampler as source. Handler of any other type between BandResampler and BandCascadeTransformer will cause an error. Change: UniformBlur: RadiusAdaptation: option removed Change: old syntax of specifying source for handler is removed (you can no longer specify source in handler description, it must be specified in processing unit description) Change: new syntax for specifying handler source:

Change: resolve: status string is no longer avaiable Change: resolve: option renamed: handler → handlerName. As Child measures also use resolve under the hood, this change also applies to them. Change: invalid index in resolve value is now forbidden and will result in full parent measure stop until skin refresh. Same for some other errors, which previously were silently suppressed or only caused warning in the log. Change: MagicNumber: option removed

d-uzlov commented 3 years ago

This was the last change for 2.0 release. And it's unlikely that there will be major changes anytime soon. I got a full-time job, and I probably won't have enough time for the work on the plugin. Of course, there will be bug fixes if any bugs are found, but I don't think plugin will get any new features in the foreseeable future, unless someone makes a pull request.

d-uzlov commented 3 years ago

Release 2.0.1: quick fix for device lists Change: resolves device list input, device list output, device list were removed. Change: resolve deviceList added.

Change: onDeviceListChange is also fired on initialization

LI7XI commented 3 years ago

Change: Spectrogram: resolution → UpdateRate. Value must be in range [1, 200], default value 20 Change: Waveform: resolution → UpdateRate. Value must be in range [1, 200], default value 20

I guess resolution is more descriptive, based on what it actually does to the image.

Change: new syntax for specifying handler source:

While it's veeery help full, it looks a bit confusing. I can't think of a better symbols though.😅 I'll test it out and document it.

I got a full-time job, and I probably won't have enough time for the work on the plugin.

Congratulation!! ^^ Although i hope that you still have time to reply questions :[

d-uzlov commented 3 years ago

I guess resolution is more descriptive, based on what it actually does to the image.

I wanted to make it uniform across the plugin. I think it's quite confusing that plugin used both milliseconds and Hz (rate) for defining how often things are updating. And resolution options were quite obscure: you couldn't really understand which value you need until you try it. Even if updateRate options will be somewhat confusing to someone, they aren't worse than the old solution.

While it's veeery help full, it looks a bit confusing. I can't think of a better symbols though.😅

Well, there is this → handy symbol, but I'm afraid that very few people use custom keyboard layouts.

The repeating of every Handler name had been bugging me ever since the syntax change (linking handlers in unit description instead of handler description). I was planning to fix it eventually. Maybe there could be a better syntax, but we have what we have.

Although i hope that you still have time to reply questions :[

Sure. I still have some free time, just not a lot of it.

LI7XI commented 3 years ago

Even if updateRate options will be somewhat confusing to someone, they aren't worse than the old solution.

I understand your point but the term "resolution" was very fitting in the context of these handlers. UpdateRate will cause more confusion when mentioning stationary parameter. When stationary true, does updateRate makes more sense than resolution?

Well, there is this → handy symbol, but I'm afraid that very few people use custom keyboard layouts. Maybe there could be a better syntax, but we have what we have.

Yep i thought the same. What about bitwise-like symbols (>>)?

d-uzlov commented 3 years ago

When stationary true, does updateRate makes more sense than resolution?

These options are not related at all. stationary is about in which places the image is updated, not how often this happens.

What about bitwise-like symbols (>>)?

Why not a single '>'? I was considering omitting '-' from the arrow. Either way, I don't think that people who aren't familiar with programming will natively understand what this notation means. 'fft>resampler': fft is more than resampler? WTF does this mean? 'fft>>resampler': fft is much more than resampler? WTF does this mean? Arrow symbol, on the other hand, will probably be understood by everyone.

LI7XI commented 3 years ago

Well, i guess it's better to stick with -> notation😹

LI7XI commented 3 years ago

Hello, About chaining handlers, is it possible to ignore white spaces? something like FFT -> Resampler doesn't work.

LI7XI commented 3 years ago

Here is why i prefer "resolution" :(

AudioAnalyzer latest: UpdateRate 200

image

AudioAnalyzer alpha: Resolution 0.6

image

Doesn't matter what i do, i can't achieve the same look using UpdateRate :((((((

LI7XI commented 3 years ago

Btw, UpdateRate 200 means there will be 200 writes to disk per second? or that depends on UpdateRate in threading option? or Update in rainmeter section?

You see... the problem with using the term UpdateRate every where especially that it has range from 1 to 200 make it really confusing.

Is UpdateRate in threading option only determines at which rate "values" (numerical calculations) gonna update?

d-uzlov commented 3 years ago

Hello, About chaining handlers, is it possible to ignore white spaces? something like FFT -> Resampler doesn't work.

Oh, that's a bug. I will look into it.

Doesn't matter what i do, i can't achieve the same look using UpdateRate :((((((

resolution 0.6 correspond to updaterate 1000/0.6 == 1666.666.

Btw, UpdateRate 200 means there will be 200 writes to disk per second? or that depends on UpdateRate in threading option? or Update in rainmeter section?

Plugin only writes something on disk when all of the following is true:

  1. The image has updated since last write.
  2. Rainmeter requested update from the measure.

Update rate in threading determines how often plugin tries to update its values. Update rate in handlers determines how often they generate new values.

You see... the problem with using the term UpdateRate every where especially that it has range from 1 to 200 make it really confusing.

Yeah, that's a bug. There certainly shouldn't be such limitations on handler update rate. Threading has this limitation because it doesn't make sense to update faster, but handlers operate on different data.