Default to capturing mic + all system audio. - #39
Conversation
There was a problem hiding this comment.
Pull request overview
Updates ownscribe’s out-of-the-box audio capture behavior to better match the primary “record a meeting you’re in” use case by enabling microphone capture and recording all system audio without prompting by default, while adding an explicit CLI opt-out.
Changes:
- Change default audio config to
mic = trueandcapture_mode = "all"(avoids missing the user’s own voice and avoids the source picker prompt). - Update CLI to support
--mic/--no-micand to reject the--no-mic+--mic-devicecombination. - Refresh tests and README examples to reflect the new defaults and flags.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_pipeline.py | Updates recorder-construction expectations to match new default mic + capture mode. |
| tests/test_config.py | Adjusts default-config assertions for mic and capture_mode. |
| tests/test_cli.py | Updates CLI parsing tests for --no-mic and adds coverage for invalid flag combinations. |
| src/ownscribe/config.py | Changes default audio settings in both TOML template and dataclass defaults. |
| src/ownscribe/cli.py | Implements --mic/--no-mic tri-state override behavior and validates incompatible combinations. |
| README.md | Updates usage examples and config snippet to match the new default capture behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if mic is False and mic_device: | ||
| raise click.UsageError("--no-mic and --mic-device cannot be used together.") | ||
| if mic is not None: | ||
| config.audio.mic = mic | ||
| if mic_device: | ||
| config.audio.mic = True | ||
| config.audio.mic_device = mic_device |
| 1. Capture system audio and your microphone until you press Ctrl+C (or auto-stop after 5 minutes of silence) | ||
| 2. Transcribe with WhisperX | ||
| 3. Summarize with your local LLM | ||
| 4. Save everything to `~/ownscribe/YYYY-MM-DD_HHMMSS/` |
| def test_no_mic_flag(self): | ||
| runner = CliRunner() | ||
| with _mock_config(), mock.patch("ownscribe.pipeline.run_pipeline") as mock_run: | ||
| result = runner.invoke(cli, ["--mic"]) | ||
| result = runner.invoke(cli, ["--no-mic"]) | ||
| assert result.exit_code == 0 | ||
| config = mock_run.call_args[0][0] | ||
| assert config.audio.mic is True | ||
| assert config.audio.mic is False |
paberr
left a comment
There was a problem hiding this comment.
Thanks a lot for the work!
Besides the Copilot & my comments, I think the README needs some updating:
- line 45 still mentions the old default and
--mic - I'm not sure if the permissions described in 68 are still correct or if there's a microphone prompt too?
- The mute hint "press 'm'" is on the
--no-micexample where it doesn't actually apply.
| if mic is False and mic_device: | ||
| raise click.UsageError("--no-mic and --mic-device cannot be used together.") | ||
| if mic is not None: | ||
| config.audio.mic = mic |
There was a problem hiding this comment.
Blocking: I think --no-mic doesn't actually disable the mic when mic_device is set in config.toml:
--no-micsetsconfig.audio.mic = False, butconfig.audio.mic_devicekeeps its value from the config file.CoreAudioRecorder.start()re-enables the mic from the device name alone (if self._mic or self._mic_device: cmd.append("--mic")).- So the helper is still launched with
--mic --mic-device "USB Mic"and we record the mic anyway.
Possible fix: clear config.audio.mic_device when --no-mic is given, e.g.
if mic is not None:
config.audio.mic = mic
if mic is False:
config.audio.mic_device = ""(same as CoPilot's comment below I just noticed)
| def test_no_mic_flag(self): | ||
| runner = CliRunner() | ||
| with _mock_config(), mock.patch("ownscribe.pipeline.run_pipeline") as mock_run: | ||
| result = runner.invoke(cli, ["--mic"]) | ||
| result = runner.invoke(cli, ["--no-mic"]) | ||
| assert result.exit_code == 0 | ||
| config = mock_run.call_args[0][0] | ||
| assert config.audio.mic is True | ||
| assert config.audio.mic is False |
There was a problem hiding this comment.
Two small things on the tests here, non-blocking:
- With
test_mic_flagrenamed, we no longer test that an explicit--micoverridesmic = falsefrom the config file. Maybe keep a--mictest next to the--no-micone? - There's also no test that
--mic-devicealone impliesmic = True— that implication now sits on its own line incli.pyand could regress silently.
Once the mic_device interaction above is fixed, a test for config-set mic_device + --no-mic would be a nice-to-have too.
Most users record meetings they're participating in, so mic=true and capture_mode="all" out of the box avoids missing your own audio and skips the source picker prompt. --no-mic allows opting-out. With --no-mic, --mic-device is rejected, as there's no sensible behavior for this combination. Fix paberr#36.
The helper was started with --mic whenever a mic_device was set, so --no-mic (or mic = false in config.toml) still recorded the microphone when a device name remained configured. The device name now only applies while mic capture is on.
Cover the mute key and --no-mic where the mic is mentioned, add the Microphone permission macOS now asks for, and fix the output directory name, which is YYYY-MM-DD_HHMM plus the title slug.
Capturing the mic by default means an unavailable microphone stops the whole recording, so name the flag that gets the run going again.
e051859 to
188ed03
Compare
|
Thanks a lot again @aaugustin ! I just had some time and put out the remaining fixes myself. |
|
Thank you! Apologies for dropping the ball, I went down a rabbit hole in another open source project of mine in the recent weeks. |
Most users record meetings they're participating in, so mic=true and
capture_mode="all" out of the box avoids missing your own audio and
skips the source picker prompt.
--no-mic allows opting-out. With --no-mic, --mic-device is rejected,
as there's no sensible behavior for this combination.
Fix #36.