Skip to content

Commit

Permalink
feat!: renderの引数の範囲指定部分を各言語の慣習に合わせる (#879)
Browse files Browse the repository at this point in the history
`Synthesizer::render`の引数の範囲指定部分を次のようにする。

Rust API: `range: Range<usize>`
Python API: `start: int, stop: int`

以前の議論:
#870 (comment)
  • Loading branch information
qryxip authored Dec 1, 2024
1 parent d8e021e commit 1d24929
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 26 deletions.
21 changes: 9 additions & 12 deletions crates/voicevox_core/src/synthesizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,15 +468,15 @@ mod inner {
pub(super) async fn render(
&self,
audio: &AudioFeature,
start: usize,
end: usize,
range: Range<usize>,
) -> Result<Vec<u8>> {
// TODO: 44.1kHzなどの対応
if (start..end).is_empty() {
if range.is_empty() {
// FIXME: `start>end`に対してパニックせずに正常に空を返してしまうのでは?
// 指定区間が空のときは早期リターン
return Ok(vec![]);
}
let spec_segment = crop_with_margin(audio, start..end);
let spec_segment = crop_with_margin(audio, range);
let wave_with_margin = self
.render_audio_segment(spec_segment.to_owned(), audio.style_id)
.await?;
Expand Down Expand Up @@ -524,7 +524,7 @@ mod inner {
let audio = self
.precompute_render(audio_query, style_id, options)
.await?;
let pcm = self.render(&audio, 0, audio.frame_length).await?;
let pcm = self.render(&audio, 0..audio.frame_length).await?;
Ok(wav_from_s16le(
&pcm,
audio_query.output_sampling_rate,
Expand Down Expand Up @@ -1201,6 +1201,8 @@ mod inner {
形を考えると、ここの引数を構造体にまとめたりしても可読性に寄与しない"
)]
pub(crate) mod blocking {
use std::ops::Range;

use easy_ext::ext;

use crate::{
Expand Down Expand Up @@ -1310,13 +1312,8 @@ pub(crate) mod blocking {
}

/// 中間表現から16bit PCMで音声波形を生成する。
pub fn render(
&self,
audio: &AudioFeature,
start: usize,
end: usize,
) -> crate::Result<Vec<u8>> {
self.0.render(audio, start, end).block_on()
pub fn render(&self, audio: &AudioFeature, range: Range<usize>) -> crate::Result<Vec<u8>> {
self.0.render(audio, range).block_on()
}

/// AudioQueryから直接WAVフォーマットで音声波形を生成する。
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ class Synthesizer:
self,
audio: AudioFeature,
start: int,
end: int,
stop: int,
) -> bytes: ...
def synthesis(
self,
Expand Down
18 changes: 7 additions & 11 deletions crates/voicevox_core_python_api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -713,32 +713,28 @@ mod blocking {
Ok(AudioFeature { audio })
}

#[pyo3(signature=(audio, start, end))]
fn render<'py>(
&self,
audio: &AudioFeature,
start: usize,
end: usize,
stop: usize,
py: Python<'py>,
) -> PyResult<&'py PyBytes> {
if start > audio.frame_length() || end > audio.frame_length() {
if start > audio.frame_length() || stop > audio.frame_length() {
return Err(PyIndexError::new_err(format!(
"({}, {}) is out of range for audio feature of length {}",
start,
end,
audio.frame_length(),
"({start}, {stop}) is out of range for audio feature of length {len}",
len = audio.frame_length(),
)));
}
if start > end {
if start > stop {
return Err(PyValueError::new_err(format!(
"({}, {}) is invalid range because start > end",
start, end,
"({start}, {stop}) is invalid range because start > end",
)));
}
let wav = &self
.synthesizer
.read()?
.render(&audio.audio, start, end)
.render(&audio.audio, start..stop)
.into_py_result(py)?;
Ok(PyBytes::new(py, wav))
}
Expand Down
5 changes: 3 additions & 2 deletions docs/guide/dev/api-design.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@ VOICEVOX CORE の主要機能は Rust で実装されることを前提として
* [`StyleId`](https://voicevox.github.io/voicevox_core/apis/rust_api/voicevox_core/struct.StyleId.html)といった[newtype](https://rust-unofficial.github.io/patterns/patterns/behavioural/newtype.html)は、そのままnewtypeとして表現するべきです。
* 例えばPythonなら[`typing.NewType`](https://docs.python.org/ja/3/library/typing.html#newtype)で表現します。
* オプショナルな引数は、キーワード引数がある言語であればキーワード引数で、ビルダースタイルが一般的な言語であればビルダースタイルで表現すべきです。

<!-- TODO: `render`の引数について: https://github.com/VOICEVOX/voicevox_core/pull/870#discussion_r1835601477 -->
* `Synthesizer::render``range: std::ops::Range<usize>`を引数に取っています。`Range<usize>`にあたる型が標準で存在し、かつそれが配列の範囲指定として用いられるようなものであれば、それを使うべきです。
* ただし例えばPythonでは、`slice`を引数に取るのは慣習にそぐわないため`start: int, stop: int`のようにすべきです。
* もし`Range<usize>`にあたる型が標準で無く、かつRustの"start"/"end"やPythonの"start"/"stop"にあたる明確な言葉が無いのであれば、誤解が生じないよう"end_exclusive"のように命名するべきです。

0 comments on commit 1d24929

Please sign in to comment.