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

small issues with midi plugout #20

Closed
3 tasks done
mmitch opened this issue Dec 21, 2018 · 5 comments
Closed
3 tasks done

small issues with midi plugout #20

mmitch opened this issue Dec 21, 2018 · 5 comments
Assignees

Comments

@mmitch
Copy link
Owner

mmitch commented Dec 21, 2018

Checking out issue #19 I ran the midi plugout for the first time in a long time and found some small issues:

  • The plugout name in gbsplay -o list says MIDI sound driver but it is a file writer like stdout. This should be a simple rename.
  • The generated filename is gbsplay-1.mid. When you run it again, that file is silently overwritten. We should either output a warning, disallow the overwrite or sequentially name the output files (there is already a -1 in there, so using -2 and so on should not be too surprising to the user).
  • Silence detection does not work properly: When running gbsplay -o midi examples/nightmode.gbs, output stops after 2 seconds because gbsplay runs into the default silence timeout. -T has to be set to export more than 2 seconds. I'd guess silence detection does not work because no waveform is generated.
@mmitch
Copy link
Owner Author

mmitch commented Dec 21, 2018

D'oh: The number part of the filename is the subsong number, so we can't increment it for subsequent exports.
The easiest way would be to add some documentation regarding filename creation and overwriting of existing files.

@mmitch
Copy link
Owner Author

mmitch commented Dec 21, 2018

task 1 fixed with e1872f6

@mmitch
Copy link
Owner Author

mmitch commented Dec 22, 2018

task 2 fixed with 729d1f8 (documentation only, no changes)

@mmitch
Copy link
Owner Author

mmitch commented Dec 22, 2018

@ranma
I have tracked part 3 of this issue down to these two lines in gb_flush_buffer()

Two plugouts (iodumper and midi) don't implement the write callback because they don't need it.
This enables the shoutcut highlighted above and gb_flush_buffer() returns eartly.
Unfortunately, this also means that lminval, lmaxval, rminval and rmaxval don't get updated, which leads to the silence detection kicking in immediately in gbs_step()

In the midi output this also manifests as the global volume being empty in the status line during conversion, while the per-channel-volumes are displayed correctly.

What should we do?

  1. remove the shortcut in gb_flush_buffer (this increases computation time during conversion - but who cares?)
  2. add empty writer implementations to both plugins (but then the shortcut from 1. is never used and we can simply implement 1. straight away)
  3. disable the silence timeout when using those two plugins (with a new flag like we did for stdout/verbosity)
  4. add a note to the manpage that says you have to set -T or output will be cut off (I don't like this one)

What do you think?

ranma added a commit that referenced this issue Jan 4, 2019
This was added in commit d8a5303 but it
interferes with plugout_midi since it breaks the silence detection
(#20).
@mmitch
Copy link
Owner Author

mmitch commented Jan 5, 2019

Thanks!

@mmitch mmitch closed this as completed Jan 5, 2019
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