-
Notifications
You must be signed in to change notification settings - Fork 305
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
project-s: MIDIファイルとMusicXMLファイルをインポートする機能を追加 #1018
project-s: MIDIファイルとMusicXMLファイルをインポートする機能を追加 #1018
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!
xmlの読み込み周り大変だったと思います、お疲れ様でした&ありがとうございます!!
レビューありがとうございます! また、MIDIインポートでテンポ・拍子の位置が変換できていなかったのと、 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sigprogramming @Hiroshiba
こちらレビュー遅くなり大変失礼いたしました…!
MIDI/MusicXML読み込みが問題なくできること確認いたしました!!(MusicXMLすごいです)
1点メモのみ追記いたしましたが、当面問題なさそうと考えております!
@sigprogramming (こちらレビュー遅くなり大変失礼いたしました…!) |
@romot-co 何度もレビューいただいて申し訳ないです…!ありがとうございました! |
const tempos = midi.header.tempos | ||
.map((tempo) => ({ | ||
position: convertToPosBasedOnRes(tempo.ticks), | ||
tempo: round(tempo.bpm, 2), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sigprogramming
昔のプルリクなのにすみません、ちょっとここの質問が!
こちらでbpmをroundしている意図はなんでしょう 👀
というのも、曲が長い場合ずれそうだな~と思ったためです。
もし表示がずれる感じであれば、たぶん致命具合は 表示がずれる < タイミングがずれる なのでround外したほうが良いのかなと思ったり、という感じです!
(文脈)
#2104 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
roundはSMF(スタンダードMIDIファイル)作成時のテンポ情報の誤差を無くす目的で行っています!
SMFでは、メタイベントのセットテンポイベントがテンポ情報を持っています。
このテンポ情報はBPMではなく「4分音符あたりのマイクロ秒」の形になっていて、SMF作成時、
が割り切れない場合に小数点以下が切り捨てられ、誤差が発生します。
例えば、BPM299のとき
となり、約0.0013の誤差が発生します。(BPM300以下ではこれが最大の誤差です)
BPMは300以下であることがほとんどなのと、少数第3位まで設定することはほとんどないと思うので、少数第3位で四捨五入(round(tempo.bpm, 2)
)してSMF作成時の誤差を無くしています。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
なーーーーるほどです!!!! 非常によくわかりました、ありがとうございます!!!
内容
MIDIファイルとMusicXMLファイルをインポートする機能を追加します。
MusicXMLを扱えるライブラリで導入しやすそうなものが見つからなかったので、ひとまずパース処理を実装しました。
簡単な楽譜で動作を確認していますが、読み込めない楽譜もあるかもしれません…
また、showImportFileDialogで、ファイルの種類を指定できるように変更しています。
レビューよろしくお願いいたします。
関連 Issue
ref VOICEVOX/voicevox_project#15
ref #981