-
Notifications
You must be signed in to change notification settings - Fork 38
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
Automatic transcoding feature, RecordID, and $enter without args. #16
base: master
Are you sure you want to change the base?
Conversation
@capthndsme hey there, thanks for contributing! I'm a little busy with school atm, the changes look good on first glance (thanks to the detailed info), I'll go through it in detail sometime soon, hopefully before this month :) |
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.
This is a good enhancement! I really like the guild lock thingy 😄
There are just a few issues with the PR and I'm a bit iffy about the necessity of including metadata but overall, great work!
Also, the code formatting is a mess (including mine :P), I should probably add a pre commit hook to fix that.
`${__dirname}/../recordings/${recId}/rendered.mp3` | ||
] | ||
|
||
const ffmpegChild = spawn('ffmpeg', ffmpegArgs); |
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.
Since this module depends on ffmpeg-static
, you could do something like:
const ffmpeg = require('ffmpeg-static');
const ffmpegConvert = () {
require('child_process').execSync(`${ffmpeg} -f s16le -ar 48000 -ac 2\
-i recordings/${recId}/merged.pcm\
recordings/${recId}/rendered.mp3`);
}
This would not need ffmpeg
as a system level dependency.
const dispatcher = conn.play(__dirname + '/../sounds/drop.mp3'); | ||
dispatcher.on('finish', () => { console.log(`Joined ${voiceChannel.name}!\n\nREADY TO RECORD\n`); }); | ||
|
||
console.log(voiceChannel); |
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 think this line can be removed? It is polluting the console with rather useless information.
|
||
console.log(`Sliding into ${voiceChannel.name} ...`); | ||
voiceChannel.join() | ||
if (typeof(activeGuildRecorders[msg.member.voice.channel.guild.id]) === "undefined") { |
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.
It'll be best to put this if-else
block inside a try-catch
block because the bot crashes if a guild member is not in a voice channel and runs a command like !enter invalidVoiceChannelName
Can you update this to fit with the new updated command.js as this contains the cannot find channel bug |
Implemented the following features:
Automatic transcoding feature
You can now enter without args. This will join the user's current voice channel.
Added JSON metadata.