Import CDSP YAML and EqAPO filters

[JWahle]

I pushed an implementation to import CDSP YAML and EqAPO filters directly into a filter step in the pipeline. This also includes a small backend change. The tooltips of the new buttons should describe the exact semantics properly.

Here are some exampe files: EqAPO.txt CdspYaml.txt

I am dicey, whether we should release the EqAPO import, because supporting all EqAPO filters would add much complexity, without much gain (if CDSP export gets implemented in AutoEQ). Implementing only a subset - like the current implementation - might create some confusion for the users and possibly more support tickets for you. What do you think?

[HEnquist]

This seems to work fine when used as intended. But there are problems when it's not, for example when trying to import a complete cdsp yaml file with pipeline and all. As it is now, I fear it would generate quite a lot of support requests. I'm trying to finish up v2.0 of both dsp and gui so it can finally be released. Smaller fixes etc are fine to push directly to the next20 branches, but a big thing like this should go via a PR. I think this was premature and should be rolled back in next20 for now.

Some points for discussion:

• How general should the import be? Now it imports all filters from the file and only the filters. Fine for the limited files exported from REW for example. But why not allow importing an entire pipeline with mixers etc? This would be needed to import for example Convolver config files (https://convolver.sourceforge.net/config.html)
• Where should the translation be done? I prefer to do things in Python, so I would vote for implementing only the import of CamillaDSP json configs in the gui. Others, like EqAPO and anything more added later, I would let the backend translate to the CamillaDSP json format.
• Where should the import functionality live in the ui? A more general solution would likely benefit from having its own tab (or perhaps a section in the files tab).

[JWahle]

This seems to work fine when used as intended. But there are problems when it's not...

True. For some reason I assumed, that REW would only export the filters. I just downloaded the 5.20.14-Beta and saw, that it exports the filters and pipeline attributes, so I can change the import to account for that. Changing that should make it much more robust and generate less support requests. Would you want to add some validation beforehand, or would it be fine to release then?

I think this was premature and should be rolled back in next20 for now.

I am optimistic, that I can get this to a releasable state very soon - assuming, we can agree on a workflow. If not, I would suggest just hiding the buttons and keeping the code as is.

How general should the import be?

I think there are several use cases for an import, so I am not sure, if we should think of THE import. There could be more than one option to import things, depending on what and where to import. Another import button for just importing a selection of filters without adding anything to pipeline steps could be located in the filters tab. A complete pipeline import could be located between the pipeline steps. For the use case of importing from REW or AutoEQ, the current workflow seems optimal to me, because they always just export filters for one filter step. Also the functionality should be easy to explain in a tooltip. I can change it quickly to a complete pipeline import, though. Not sure, that is the best thing from a usability perspective, though.

Where should the translation be done?

YAML to JSON conversion is done in the backend. Only the limited EqAPO parsing is done in the frontend - should we decide to keep the functionality, I can translate this to python, if you prefer that.

Where should the import functionality live in the ui? A more general solution would likely benefit from having its own tab (or perhaps a section in the files tab).

Unless we show a complete report of the import result or build a complete import UI, the pipeline is the best place for the user to see, what actually got imported.

[JWahle]

I just changed the import behavior when importing YAML files to expect a filters attribute on the top level. So, now filters from normal CDSP configs can be imported.

The missing validation in the backend might be a problem, though. Normal config validation is too strict to be used for this use case - we'll have to alter it, before we can reuse it.

When do you plan to do the 2.0 release, roughly?

[HEnquist]

This seems to work fine when used as intended. But there are problems when it's not...

True. For some reason I assumed, that REW would only export the filters. I just downloaded the 5.20.14-Beta and saw, that it exports the filters and pipeline attributes, so I can change the import to account for that. Changing that should make it much more robust and generate less support requests. Would you want to add some validation beforehand, or would it be fine to release then?

I think this was premature and should be rolled back in next20 for now.

I am optimistic, that I can get this to a releasable state very soon - assuming, we can agree on a workflow. If not, I would suggest just hiding the buttons and keeping the code as is.

The current implementation is built for importing the limited files exported from REW etc. It's far from as general as I had in mind. But it was merged without any prior discussion. People will try to use it to import things from their other config files, something it's not designed for and will handle poorly. I don't think it's very close to a releasable state, and I still thing it should be rolled back and moved to a PR.

And yes, some validation is required. It would be quite easy to run validation on only the parts of the config that are to be imported.

How general should the import be?

I think there are several use cases for an import, so I am not sure, if we should think of THE import. There could be more than one option to import things, depending on what and where to import. Another import button for just importing a selection of filters without adding anything to pipeline steps could be located in the filters tab. A complete pipeline import could be located between the pipeline steps. For the use case of importing from REW or AutoEQ, the current workflow seems optimal to me, because they always just export filters for one filter step. Also the functionality should be easy to explain in a tooltip. I can change it quickly to a complete pipeline import, though. Not sure, that is the best thing from a usability perspective, though.

There are certainly several use cases, but I still think there should be a single place to go for imports. Adding buttons here and there, like inside pipeline filter steps, between pipeline steps, in the filters and in the mixers tab, is confusing and clutters the ui.

Where should the translation be done?

YAML to JSON conversion is done in the backend. Only the limited EqAPO parsing is done in the frontend - should we decide to keep the functionality, I can translate this to python, if you prefer that.

Doing the translation in Python means it would be possible to reuse the same code to do translation in simple scripts, without the frontend.

Where should the import functionality live in the ui? A more general solution would likely benefit from having its own tab (or perhaps a section in the files tab).

Unless we show a complete report of the import result or build a complete import UI, the pipeline is the best place for the user to see, what actually got imported.

IMO you should be given a choice of what to import from the selected file. The user may want to import a few filters, and a mixer from a larger file. Then once the import is done, it should show a summary of what was imported. I'm not saying that everything needs to be available in a first released version, but the overall design and workflow should really be in place.

When do you plan to do the 2.0 release, roughly?

The sooner the better! It's long overdue.

[JWahle]

I just removed the filter import logic from the next20 branch of the GUI and put it in the filter_import branch.

[JWahle]

In the filter_import branch (backend+frontend), I implemented importing of Cdsp Yaml and convolver configs. The EqAPO import button is removed, for now. For the filter scale factors in the convolver configs, I used linear dB scale. Is this correct? For Reference: Specification, Examples Some configs to play around: crossover.txt, stereo.txt, surround_downmix.txt

@HEnquist would you consider this feature releasable

[HEnquist]

I'll take a look and try it.

[HEnquist]

I'm running filter_import for both frontend and backend and tried importing files. It's not really working, I just get:

Uncaught runtime errors:

ERROR
Cannot read properties of undefined (reading 'name')
TypeError: Cannot read properties of undefined (reading 'name')
    at http://localhost:3000/static/js/bundle.js:5658:124

This comes after I pick a config file, doesn't matter if it's camilla or convolver config file.

[JWahle]

Is this the port 5000/5005 problem? With the port fix, it works on my machine - I just checked out the branches fresh from GitHub.

[HEnquist]

I don't think it has anything to do with the port, all other parts of the gui are working. I'll give it another go as soon as I can.

[JWahle]

I forgot to mention, you have to switch to the filter_import branch on the frontend AND backend ;) This is what just caused the same issue for me.

[HEnquist]

Finally got it to work, I need to use Firefox on Linux. In Safari and Chrome on MacOS, and Konqueror on Linux I get the error "Cannot read properties of undefined (reading 'name')". The line it's complaining about is here: https://github.com/HEnquist/camillagui/blob/52c0a52792e1d501b665fc7c04f9b7170f34a003/src/import/importpopup.tsx#L96

With Firefox on mac I get another error, "right-hand side of 'in' should be an object, got null".

[HEnquist]

I pushed a fix for the problem I mentioned above, as well as an issue where the collision check failed if the current config had a filters or mixers section that was null.

[HEnquist]

I looked at the Convolver translation and found some errors in the parsing of gain values. Convolver has a very weird way of defining this, but I think it's correctly handled now. Please check! I also ended up renaming many methods in the backend, to separate what is actually json and what is a python object. It was quite confusing when json could mean either a string with json content, or a Python object (which isn't json but has similar syntax).

[JWahle]

I looked at the Convolver translation and found some errors in the parsing of gain values. Convolver has a very weird way of defining this, but I think it's correctly handled now. Please check!

Yes, looks correct now. I wonder, how I could forget this while writing all the tests ;)

I also ended up renaming many methods in the backend, to separate what is actually json and what is a python object. It was quite confusing when json could mean either a string with json content, or a Python object (which isn't json but has similar syntax).

The distinction is definitely good.

Your GUI changes also look nice. I would prefer running the python tests from the backend sub directory, but other than that everything looks good to me.

[HEnquist]

I would prefer running the python tests from the backend sub directory,

To me it makes sense to launch the tests from the same directory I launch the application from, so the root directory here. That also has the advantage of not breaking if we change the structure from all files in a single dir to something smarter (which is probably about time since the number of files is steadily increasing).

Speaking of tests, I'm not a fan of unittest. IMO pytest is a much nicer framework to work with, with easier test definitions, much more helpful output on test failures etc. The current tests would be easy to port over, and I think it would make sense to do the switch now(ish), before implementing more tests.

[HEnquist]

see https://github.com/HEnquist/camillagui-backend/pull/51

[HEnquist]

I added some more tests in that pr, some general things for the api. There are still plenty of things that are not tested that could be added quite easily. The pytest-converted tests are picked up just fine by VS Code btw (haven't tried that with the unittest version).

[HEnquist]

As fas as I can tell, the import is working well now. I think it's time to release it, together with the small updates to the pipeline plot and the custom shortcuts. The table component can go in the next release (unless it's nearly done?).

[JWahle]

I added some more tests in that pr, some general things for the api. There are still plenty of things that are not tested that could be added quite easily. The pytest-converted tests are picked up just fine by VS Code btw (haven't tried that with the unittest version).

The pytest tests look good to me. I'll try, if I can just use them properly in IntelliJ.

As fas as I can tell, the import is working well now. I think it's time to release it, together with the small updates to the pipeline plot and the custom shortcuts. The table component can go in the next release (unless it's nearly done?).

Yes, let's not wait for the table component. It's getting a little messy with all the branching. If you're not releasing immediately, can you update the develop branches (of both GUI and backend) with the changes, that will go into the next release?