RFC: Streamline implementation overrides

[MarekPikula]

Background and motivation

While preparing initial patches for RISC-V support (as mentioned in #368), I noticed that overriding function implementations requires a lot of manual copy-pasting, leading to code duplication and increased maintenance effort.

This PR proposes a streamlined approach to declaring function overrides. I applied it to two functions in Silk (silk_NSQ and silk_VQ_WMat_EC) for x86 to demonstrate its benefits and gather feedback on whether such a refactor would be accepted upstream.

If approved, I can extend this approach to all overridable functions across all architectures, including Celt (which seems to use a similar override mechanism – please correct me if I'm wrong).

Proposal

1. To reduce code duplication, function declaration can be set in a single macro (inmain_overrides.h), which can also be used for function type definition for use in the implementation array (e.g., for silk_NSQ function, there is now SILK_NSQ_DECL(impl) macro, which can be used to declare the function for all backends).
2. The arch argument in the main macro definition is moved to the front, allowing for the use of macro variadic arguments, reducing the copy-paste of argument lists for each implementation.
3. To declare a single backend override, there is a new macro OVERRIDE_IMPL_SINGLE, which references the selected backend implementation.
4. Accordingly, for the multi-backend override, there is a new macro OVERRIDE_IMPL_ARRAY, which addresses the override array created with OVERRIDE_IMPL_ARRAY_DECL.

This approach significantly reduces manual (and error-prone) copy-paste work, minimizes the risk of errors when developing new backends (e.g., RISC-V in our case), and improves maintainability. Now, function definition changes require modification in only one place, reducing the likelihood of regressions across platforms.

Example

To enable the override of a function foo in Silk:

1. Add function declaration in main_overrides.h in the form of FOO_DECL(impl, ...) macro and function type definition with typedef FOO_DECL(t, *const);.
2. Add the declaration to main.h:

   FOO_DECL(c);
   #ifndef foo
   #define foo(...) OVERRIDE_IMPL_SINGLE(foo, c, __VA_ARGS__)
   #endif
   

3. Change the existing function definition to FOO_DECL(c) { ... }.
4. Change all function references to include arch as the first argument.

Now, you can add arch-specific implementation (an example for x86 SSE):

1. Add function override in main_sse.h:
   1. For single function override:

      #define foo(...) OVERRIDE_IMPL_SINGLE(foo, sse4_1, __VA_ARGS__)
      

   2. For RTCD override:

      extern OVERRIDE_IMPL_ARRAY_DECL(foo);
      #define foo(...) OVERRIDE_IMPL_ARRAY(foo, __VA_ARGS__)
      

2. Add RTCD map definition in x86_silk_map.c:

   OVERRIDE_IMPL_ARRAY_DECL(foo) = {
     foo_c,                  /* non-sse */
     foo_c,
     foo_c,
     MAY_HAVE_SSE4_1( foo ), /* sse4.1 */
     MAY_HAVE_SSE4_1( foo )  /* avx */
   };
   

3. Add arch-specific function definition in foo.c with

   FOO_DECL(sse4_1)
   {
       ...
   }
   

[MarekPikula]

Hi @jmvalin @rillian, could you take a look at this (or ping some other person who could make a comment)?

[jmvalin]

I didn't actually create that function pointer setup, so I'm not sure of all the reasoning behind it. What I'd suggest though is to make sure you run the randomized build test to check that your patch doesn't break anything. See my previous comment at https://github.com/xiph/opus/pull/379#issuecomment-2637381867 for more details.

[mark4o]

Unfortunately __VA_ARGS__ is a C99 feature. Opus depends only on C90 in order to be compatible with some older embedded/DSP toolchains.

[MarekPikula]

Thanks for a comment @mark4o. I didn't realize C90 was the required standard for Opus. Maybe it would be nice to enforce it with --std=c90 in the build config? I have an idea how to do the refactor without __VA__ARGS__. I'll push a new proposal later this week.