feat: voice recording #1113
feat: voice recording #1113
Conversation
wrong place
Not working yet.
TrapDrap
changed the title
|
The file structure and placements are not final! |
TrapDrap
changed the title
Co-authored-by: teaishealthy <76410798+teaishealthy@users.noreply.github.com>
Co-authored-by: teaishealthy <76410798+teaishealthy@users.noreply.github.com>
|
I was also wondering if there's a better way of doing my silence writer, right now I'm relying on calling write multiple times in a while loop with bigger to smaller silence frames, I was wondering whether there's a way to create a bytes-like object that can just write the silence frame x times without having to call write x times. It would also not have to cache the 3.5mb or so of silence frames for it to work. The silence is always the same data, so it would be a lot simpler if there's a better way I can write x amount of this data without creating it in memory (multiplication does that, a silence for a minute or two will result in like 100mb of ram spike) |
Co-authored-by: Emre Terzioglu <50607143+EmreTech@users.noreply.github.com>
Co-authored-by: Emre Terzioglu <50607143+EmreTech@users.noreply.github.com>
Co-authored-by: Emre Terzioglu <50607143+EmreTech@users.noreply.github.com>
…m/TrapDrap/_nextcord into TrapDrap-nextcord-voice-recording
cuz apparently its done by style, didnt know wops! Changes to be committed: modified: nextcord/voice_recording/core.py
Changes to be committed: modified: nextcord/voice_recording/core.py
Changes to be committed: modified: nextcord/voice_recording/core.py
Changes to be committed: modified: nextcord/voice_recording/core.py
…m/TrapDrap/_nextcord into TrapDrap-nextcord-voice-recording
Changes to be committed: modified: nextcord/voice_recording/exporters.py
…m/TrapDrap/_nextcord into TrapDrap-nextcord-voice-recording
Changes to be committed: modified: nextcord/voice_recording/errors.py
modified: nextcord/voice_recording/core.py modified: nextcord/voice_recording/errors.py modified: nextcord/voice_recording/exporters.py modified: nextcord/voice_recording/opus.py
- add examples & audio merging
- merging does not work with .wav yet dont know why
- merging needs to be moved from example into exporters
- add option `prevent_leakage` to `Connectable.connect`
- refine prevent leakage to properly account for latency
- move ffmpeg formats dictionary to shared
new file: examples/voice_recording.py
modified: nextcord/abc.py
modified: nextcord/voice_recording/core.py
modified: nextcord/voice_recording/exporters.py
modified: nextcord/voice_recording/shared.py
Changes to be committed: modified: examples/voice_recording.py
…m/TrapDrap/_nextcord into TrapDrap-nextcord-voice-recording
Changes to be committed: modified: examples/voice_recording.py
Changes to be committed: modified: examples/voice_recording.py
…m/TrapDrap/_nextcord into TrapDrap-nextcord-voice-recording
There was a problem hiding this comment.
I think the way you're using (reimplementing) temp files is fundamentally flawed. Instead of this whole tmp_type junk and needing to overwrite filenames, keep a lookup table of buffers (!) and let the user bring their own buffer. If it walks like a buffer, it is a buffer. When you actually need to encode with a muxer that needs to be able to seek back, check if you got a file buffer, if not write your data to real temp file and then call ffmpeg.
| if tmp_type == TmpType.File: | ||
| self.buffer = open_tmp_file(guild_id, user_id, "wb+") | ||
| elif tmp_type == TmpType.Memory: |
There was a problem hiding this comment.
| if tmp_type == TmpType.File: | |
| self.buffer = open_tmp_file(guild_id, user_id, "wb+") | |
| elif tmp_type == TmpType.Memory: | |
| if tmp_type is TmpType.File: | |
| self.buffer = open_tmp_file(guild_id, user_id, "wb+") | |
| elif tmp_type is TmpType.Memory: |
| # slots only work on python 3.10 with dataclasses | ||
| @dataclass(**{"slots": True} if PYTHON_VERSION >= (3, 10) else {}) | ||
| class OpusFrame: |
There was a problem hiding this comment.
just hardcode the slots...
| # processes | ||
| self.process: Optional[Thread] = None | ||
| self.auto_deaf: bool = auto_deaf if isinstance(auto_deaf, bool) else True | ||
| self.tmp_type: TmpType = tmp_type or TmpType.File |
There was a problem hiding this comment.
what's the point? when will tmp_type ever be falsy
| async def voice_connect(self, deaf=None, mute=None) -> None: | ||
| await self.channel.guild.change_voice_state( # type: ignore | ||
| channel=self.channel, | ||
| self_deaf=(deaf if deaf is not None else (bool(self.auto_deaf))), |
There was a problem hiding this comment.
| self_deaf=(deaf if deaf is not None else (bool(self.auto_deaf))), | |
| self_deaf=(deaf if deaf is not None else self.auto_deaf), |
self.auto_deaf is alredy bool?
| await self.channel.guild.change_voice_state( # type: ignore | ||
| channel=self.channel, | ||
| self_deaf=(deaf if deaf is not None else (bool(self.auto_deaf))), | ||
| self_mute=mute or False, |
There was a problem hiding this comment.
| self_mute=mute or False, | |
| self_mute=bool(mute), |
| class FFmpeg: | ||
| """A utility class containing audio conversion methods using FFmpeg""" | ||
|
|
||
| @staticmethod | ||
| def memory_tmp_conv(audio_format: str, writer: AudioWriter) -> bytes: | ||
| try: | ||
| return subprocess.Popen( | ||
| f"{ffmpeg_default_arg} {audio_format} pipe:1", | ||
| stdin=subprocess.PIPE, | ||
| stdout=subprocess.PIPE, | ||
| creationflags=FLAG, | ||
| ).communicate(_read_and_delete(writer.buffer))[0] | ||
| except FileNotFoundError as e: | ||
| raise NoFFmpeg( | ||
| "FFmpeg is not installed or aliased improperly. Unable to launch `ffmpeg` command." | ||
| ) from e | ||
|
|
||
| @staticmethod | ||
| def file_tmp_conv(audio_format: str, writer: AudioWriter) -> None: | ||
| try: | ||
| data = _read_and_delete(writer.buffer) # must read first to delete file | ||
| process = subprocess.Popen( | ||
| f"{ffmpeg_default_arg} {audio_format} {TMP_DIR}/{writer.guild_id}.{writer.user_id}.tmp", | ||
| stdin=subprocess.PIPE, | ||
| stdout=subprocess.PIPE, | ||
| creationflags=FLAG, | ||
| ) | ||
| process.communicate(data) | ||
| except FileNotFoundError as e: | ||
| raise NoFFmpeg( | ||
| "FFmpeg is not installed or aliased improperly. Unable to launch `ffmpeg` command." | ||
| ) from e | ||
|
|
There was a problem hiding this comment.
why is this a class? it has zero object members. it should just be a new module.
| opus_frame: OpusFrame = self.decode_queue.pop(0) | ||
|
|
||
| except IndexError: | ||
| sleep(0.001) |
There was a problem hiding this comment.
make the decode_queue an actual queue.Queue and use it's methods
| if recordable: | ||
| voice = RecorderClient( | ||
| client, self, auto_deaf, tmp_type, filters, periodic_write_seconds, prevent_leakage | ||
| ) |
There was a problem hiding this comment.
So users can't use a custom RecorderClient class if they want to? I would prefer if there were two parameters, maybe called play_cls and record_cls, where you could specify either class or not specify to use the default classes.
| @@ -0,0 +1,151 @@ | |||
| # SPDX-License-Identifier: MIT | |||
There was a problem hiding this comment.
Since there are plans to introduce a custom cache into Nextcord, could this file be moved somewhere else or be renamed to prevent confusion?
There was a problem hiding this comment.
Now conflicts with #1185 because of this.
| from time import sleep | ||
| from typing import TYPE_CHECKING, Dict | ||
|
|
||
| import nextcord as nc # `import as` to prevent circular import |
There was a problem hiding this comment.
I don't get the logic behind this? It makes sense in the errors module, but not here...
| Formats.MP3: ("mp3", "mp3"), | ||
| Formats.MP4: ("mp4", "mp4"), | ||
| Formats.M4A: ("m4a", "ipod"), | ||
| Formats.MKA: ("mka", "matroska"), | ||
| Formats.MKV: ("mkv", "matroska"), | ||
| Formats.OGG: ("ogg", "ogg"), | ||
| "mp3": ("mp3", "mp3"), | ||
| "mp4": ("mp4", "mp4"), | ||
| "m4a": ("m4a", "ipod"), | ||
| "mka": ("mka", "matroska"), | ||
| "mkv": ("mkv", "matroska"), | ||
| "ogg": ("ogg", "ogg"), |
There was a problem hiding this comment.
This is pretty redundant. Forcing users to use the Formats enum and/or using string values in that enum would prevent the need for this.
There was a problem hiding this comment.
Looking further, it looks like users are generally forced to use the Formats enum. Then why is this duplication here?
|
|
||
| def __init__( | ||
| self, | ||
| client=None, |
|
|
||
| raise TypeError("Each user must be of type `int`, `User`, or `Member`") | ||
|
|
||
| def add_blocked(self, user: Union[int, User, Member]) -> None: |
There was a problem hiding this comment.
I feel these various wrapper methods around blocklist and allowlist aren't necessary since they're public attributes.
| if uid in self.allowlist: | ||
| return True | ||
|
|
||
| return False if uid in self.blocklist else not self.allowlist |
There was a problem hiding this comment.
| return False if uid in self.blocklist else not self.allowlist | |
| return (uid in self.blocklist) or (not self.allowlist) |
| from .core import OpusFrame, RecorderClient | ||
|
|
||
|
|
||
| class DecoderThread(Thread, nc.opus._OpusStruct): |
There was a problem hiding this comment.
Could we avoid using threads here?
|
Would this feature be able to save to a file as it's recording or does it save in RAM as its recording? if the bot crashes partway through a (6 hour) recording the recording would be lost if it is not saving a temp file to disk. |
Summary
This PR aims to implement voice recording to nextcord.
This is mostly fresh code, with some decoding & decrypting parts taken from the previous dead PR on voice recording.
Note that this is still a work-in-progress, it is not yet fully finalized, most features are working but may be unstable, and some options may be missing.
Next steps would be: