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

ffmpeg and DTX support #20

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

mugabe
Copy link

@mugabe mugabe commented Apr 17, 2018

  • DTX indexing support
  • Recursive search for audio files (as DTX allows to put them in subdirectories)
  • Transcoding XA audio format
  • Use ffmpeg instead of sox, qaac and afconvert
  • Use buffers for transcoding, no temporary files needed

bemuse-indexer points to my branch as it not merged yet

Depends on bemusic/indexer#1

@dtinth
Copy link
Member

dtinth commented Apr 17, 2018

Hello, thanks for the PR. I am reviewing.

Copy link
Member

@dtinth dtinth left a 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',
Copy link
Member

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.

Copy link
Author

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 })
Copy link
Member

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)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

@mugabe mugabe Apr 17, 2018

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' ]
Copy link
Member

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...

Copy link
Author

@mugabe mugabe Apr 17, 2018

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' ]
Copy link
Member

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...

@mugabe
Copy link
Author

mugabe commented Apr 17, 2018

Have no favorite simfile, but you can choose some from here.

@dtinth
Copy link
Member

dtinth commented Apr 17, 2018

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

@mugabe
Copy link
Author

mugabe commented Apr 18, 2018

  • SoX, qaac, afconvert returned
    afconvert has not tested as I have no mac with dev environment

  • Added config for select encoders
    By default configured with sox and qaac

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

Successfully merging this pull request may close these issues.

2 participants