-
Notifications
You must be signed in to change notification settings - Fork 2
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
ffmpeg and DTX support #20
base: master
Are you sure you want to change the base?
Conversation
Hello, thanks for the PR. I am reviewing. |
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.
I have done a preliminary review on this PR.
I am very new to DTX (I have never played dtxmania before). Do you have a recommended/favorite DTX simfile?
if (process.platform.match(/^win/)) { | ||
yield execFile('qaac', ['-o', m4aPath, '-c', '128', wavPath]) | ||
} else { | ||
yield execFile('afconvert', [wavPath, m4aPath, '-f', 'm4af', |
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.
I have tried to use ffmpeg’s AAC encoder before, however the result doesn’t sound good at 128kbps. Apple’s proprietary encoder (afconvert
and qaac
) produces a much more superior sound at 128kbps. That’s why I don’t recommend using ffmpeg.
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.
I have compared qaac and ffmpeg output but did not hear any significant difference. Maybe my ears not so perfect :)
@@ -11,7 +11,7 @@ export class Directory { | |||
this._path = path | |||
} | |||
files(pattern) { | |||
return glob(pattern, { cwd: this._path }) | |||
return glob(pattern, { cwd: this._path, nocase: true }) |
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.
Thanks for this fix!
|
||
get buffer() { | ||
return this._buffer.slice(0, this._bufferPos) | ||
} |
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.
Have you tried using e.g. https://github.com/samcday/node-stream-buffer?
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.
Yes. WritableStreamBuffer works and very similar to mine.
But ReadableStreamBuffer useless - very slow and does not implement some required methods which fluent-ffmpeg calls.
} else if (buffer[0] === 0xFF && buffer[1] === 0xFB) { | ||
typeArgs = [ '-t', 'mp3' ] | ||
} else if (buffer[0] === 0x4F && buffer[1] === 0x67 && buffer[2] === 0x67 && buffer[3] === 0x53) { | ||
typeArgs = [ '-t', 'ogg' ] |
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.
Are you sure that the audio converter still works in the cases where mp3
/ogg
files have .wav
extension and vice versa? I do this manual format detection because many BMS archives have this problem.
Sorry for lack of automated testing in audio.js
...
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.
Not sure, but every file I have tried was succesfully detected. Btw I pass files to ffmpeg's pipe, so ffmpeg have no information about file extension at all. I believe that ffmpeg itself guesses format by head bytes.
} else if (buffer[0] === 0xFF && buffer[1] === 0xFB) { | ||
typeArgs = [ '-t', 'mp3' ] | ||
} else if (buffer[0] === 0x4F && buffer[1] === 0x67 && buffer[2] === 0x67 && buffer[3] === 0x53) { | ||
typeArgs = [ '-t', 'ogg' ] |
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.
Are you sure that the audio converter still works in the cases where mp3
/ogg
files have .wav
extension and vice versa? I do this manual format detection because many BMS archives have this problem.
Sorry for lack of automated testing in audio.js
...
Have no favorite simfile, but you can choose some |
Thank you for the pointer. Unfortunately, I had to remove the link as I have identified it to be associated with copyrighted music, which this project does not aim to support. (This is a reason O2Jam’s OJM/OJN is not supported either. It should have a community that creates original song content.) Fortunately, DTXMania seems to have an active community creating more original content every month: http://chanmori.net/dtxir/packages.php |
|
bemuse-indexer points to my branch as it not merged yet
Depends on bemusic/indexer#1