Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gr-audio: not a single unit test #2539

Open
marcusmueller opened this issue Jun 7, 2019 · 2 comments
Open

gr-audio: not a single unit test #2539

marcusmueller opened this issue Jun 7, 2019 · 2 comments

Comments

@marcusmueller
Copy link
Member

Testing audio sink/source might be hard, because we've not defined a contract that says "needs to be constructable, can fail in start()" or something. But: we do need to test at least these sources/sinks that we know can be instantiated under all circumstances.

@marcusmueller
Copy link
Member Author

persists

@marcusmueller
Copy link
Member Author

Testing audio infrastructure is hard. Here's the key points I've gathered. First, for reference, quick architectural rundown of gr-audio

  • one pair of unified blocks audio_{sink,source}, with six different backends (alsa, oss, portaudio, jack, osx, windows)
  • which backend is chosen is selected automatically using a hard-coded priority list, unless explicitly specified in config file
  • the blocks take four parameters: sampling rate, device name, OK to block and number of channels

Regarding testing these: While we hide these behind the same block, the actual implementations work differently, have different semantics even in basic things and need individual testing.

  • The automatic source selection is troublesome. If the configured backend is not available on the executing platform, one gets a warning, but there's no programmatic way (aside from using a debugging logger and watching the logs) to know that you're not using the backend you specified.

    • the fact that we can specify device strings, but not the backend to use with the device string, is stupid. Why would an ALSA device name be useful for the Windows backend? In the context of testability, it means that one needs to fiddle with configuration backends in order to isolate the audio blocks from the actual sound system of the test platform.
  • We're doing the platform abstraction ourselves – which is a valid choice, but puts the onus of making sure things not only compile but work on different platforms on us. Curiously, we do have one backend that itself does the platform abstraction already: portaudio. It might be worth considering throwing out all the complexity we currently incur by just focusing on that backend, and demoting the others to "you have to explicitly select this; we're less likely to support it" backends. I don't see us supporting any special features of the individual platforms through our specific backends that portaudio would not expose

  • Different provisions for nr of audio channels make comparing the platforms hard:

    • alsa
      • sink: in principle as many as hardware supports but special case for single-input block, where mono stream gets copied to stereo, if hardware can't do mono
      • source: as sink
    • jack
      • sink: as many as jack allows
      • source: as many as jack allows
    • oss
      • sink: stereo only backend, if mono input, then copy (like win)
      • source: stereo only backend (no matter what the actual hardware supports). if mono is connected, ignore R channel
    • osx
      • sink: as many channels as the hardware allows, if fewer connected: copy the last supplied to all remaining channels
      • source: as many channels as the hardware allows, if fewer connected: ignore the remaining channels
    • portaudio (uses check_topology for getting the number of streams, set up sound system)
      • sink: backend as many as device supports, if fewer connected, ignore remainder
      • source: backend as many as device supports, if fewer connected, drop remainder
    • windows
      • sink: stereo only on backend, if mono connected, copy to both channels
      • source: mono only, if stereo connected copy to both channels
  • the "OK to block" on should not exist in its current form; it does different things for different backends, and setting it to "false" leads to incorrect behaviour, according to code comments, on some platforms:

    • sinks:
      • portaudio sink just drops all samples on the floor if at first it doesn't succeed handing over samples to portaudio
      • alsa sink tries again until getting rid of the samples works (might lead to spinning?)
      • jack, oss flat out ignore the flag
      • OSX sink puts things from the input ring buffer into a buffer queue if not OK to block
      • windows sink prints "aO", drops all current samples on the floor
    • hence ok_to_block==false for sinks can't be tested on portaudio, windows, and is not useful to test on jack, oss
    • sources: Lots of comments assuming single-threaded scheduling. Not how any of this works;
      • portaudio source assumes it should just fill as much buffer as it got with zeros
      • alsa, jack, oss, windows source ignore the flag
      • osx source just returns 0 from work if it couldn't fill the whole output buffer, although it got the samples from the sound system (so, probably, produces lots of dropped audio)
    • hence there's not a single sensible use case for ok_to_blockwith sources, far as I can tell. Hence, no testing necessary. **
  • ALSA at least does allow one to set up loopback devices via .asoundrc, but the semantics of when these become available for passing through data and whether and when silence is inserted: haven't figured that one out

personal assessment here is that having this many cornercase behaviours without tests is a bit dangerous. Windows and OSS sinks seems to be mostly worse than the portaudio sink; would plain remove it. Same goes for the ok_to_block = false codepaths, altogether. The sink with the most "trustworthy" code looks to me to be the jack sink. OSS is obsolete on Linux, but still necessary on FreeBSD, OpenBSD (unless we want to implement sndio), if it wasn't for the portaudio backend. Don't know whether that works on these, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant