abstract post-processing filter factory via context - #12425
Conversation
| // These filters do not accept settings (nor are settings recorded in ros_writer) | ||
| (void *)&settings; | ||
|
|
||
| if( rsutils::string::nocase_equal( name, "Decimation Filter" ) ) |
There was a problem hiding this comment.
How is this code better than before?
Performance is worse (comparing all strings)
If you make a string mistake it compile but dont work
Magic strings are used (no header that group all)
There was a problem hiding this comment.
The code compared strings before, too.
Yes, before they were "hard-coded" in that they were generated from the extension names. But we don't want to use the extension names. If you prefer, I can compare block names and fall back to "automatic" extension name comparison which was case-sensitive, but I don't think that's readable or easy to understand at all.
This is the factory. It centralizes the knowledge of processing blocks, and lets us add processing blocks dynamically in different modules. I like this much better than having extension names spread all over the place, many times not in any human-preferable way ("Hdr Merge" rather than "HDR Merge").
I can change to fit your preference, but the factory idea is much better.
There was a problem hiding this comment.
As far as performance, I can change to a map if you like. But we compared all strings before, too. This happens once during device creation.
Today, if you want to return "recommended processing blocks" (AKA recommended filters in the API), then you need to return the actual instantiation of the blocks.
This means that everyone needs to know how to instantiate them: the rosbag reader; the DDS device; etc.
The problem is that, if we add new filters (or remove old ones), the code needs to be updated everywhere.
The problem is that there's no abstraction of a factory that can instantiate a filter by name, that would allow a piece of code to know about the factory rather than the block implementations.
This adds such a factory, with the goal of separating the DDS/ROS/whatever code from post-processing (which may not even exist!).
pp_block_factorypp== post-processing, to denote these are used for post-processingblockdesignation because it returnsprocessing_block_interfaceprocessing_block_factorythat's used for more-internal non-filter stuff, so I couldn't call it what it should be calledrscore_pp_block_factorywhich thecontextknows aboutcontext::create_pp_block()- the context will know what factories exist and how to use them; nobody else shouldcreate_pp_block()accepts JSON settings that can be used to serialize settings; these aren't used right nowTracked on [LRS-967]