Make rustfft dependency optional

[Shnatsel]

Right now symphonia-core depends on rustfft unconditionally. But it brings a sprawling dependency tree with large amounts of unsafe code, making Symphonia harder to audit and adopt.

If IMDCT was an optional feature that only AAC and Vorbis codecs enabled, this would make adopting Symphonia for other codecs much easier.

Perhaps even the less optimized but safe IMDCT could be offered as an option - that might be beneficial in some domains.

[pdeljanov]

I think from a safety perspective the worries may be somewhat unfounded. We don't read f32s from files and then run them through the IMDCT directly. The values run through the IMDCT were either computed by the decoder or referenced from a lookup table. Illegal values from the file, if not guarded against, would panic the decoder before the IMDCT step. IMDCT size is also strictly controlled.

From a code bloat perspective, if none of the MDCT based codecs are enabled (AAC, Vorbis, Opus), then yes, extra time is spent compiling things which would never be linked in.

So maybe making symphonia-core's DSP features opt-in, or moving them to a separate crate altogether, is something we could consider for a 0.6.

I'm not a fan of making rustfft a top-level symphonia feature though since many people disable default features and would have to be very careful to re-enable it. Because I don't believe it poses a safety risk, and that maintaining our own FFT is a burden, I don't think it should gated.

[Shnatsel]

Fair enough re: security risks. It's just that packaging Symphonia or adopting it in an organization is complicated by large dependency tree, so not enabling IMDCT when it's not needed (and not compiling all the extra dependencies) could be beneficial.

[pdeljanov]

Learned about cargo tree to see the dependency tree. Seems like symphonia-core is pretty light even with rustfft. This doesn't negate our previous conversation, just an observation.

symphonia-core v0.5.2 (/home/philip/Development/Libraries/Symphonia/symphonia-core)
├── arrayvec v0.7.2
├── bitflags v1.3.2
├── bytemuck v1.7.3
├── lazy_static v1.4.0
├── log v0.4.14
│   └── cfg-if v1.0.0
└── rustfft v6.1.0
    ├── num-complex v0.4.2
    │   └── num-traits v0.2.14
    │       [build-dependencies]
    │       └── autocfg v1.1.0
    ├── num-integer v0.1.45
    │   └── num-traits v0.2.14 (*)
    │   [build-dependencies]
    │   └── autocfg v1.1.0
    ├── num-traits v0.2.14 (*)
    ├── primal-check v0.3.3
    │   └── num-integer v0.1.45 (*)
    ├── strength_reduce v0.2.4
    └── transpose v0.2.2
        ├── num-integer v0.1.45 (*)
        └── strength_reduce v0.2.4
    [build-dependencies]
    └── version_check v0.9.4

[atoktoto]

I had to make rustfft optional in #244 because it's not yet no_std itself.

[pdeljanov]

I made rustfft optional in 24e941f by gating it behind SIMD flags. For the 0.5.x series we'll leave SIMD off by default since rustfft has a higher MSRV so I'd consider enabling it by default to be non-semver compliant.