Submit streaming code and examples #43

ghjdegithub · 2024-10-29T00:34:06Z

No description provided.

…ompile success.)

# Conflicts: # sys/build.rs

fix typo

ghjdegithub · 2024-11-05T03:34:43Z

@thewh1teagle Didn't see your comment

thewh1teagle · 2024-10-29T18:30:59Z

examples/streaming_decode_files.rs

+///
+/// * `samples` - 样本数据
+/// * `sample_rate` - 采样率
+fn read_wave(filename: &str) -> (Vec<f32>, i32) {


Use internal function for that

Use internal function for that

This is only for testing, and the code logic is not suitable for the framework

It's useful to keep the example short and simple as possible with only the relevant info to dive in quickly. That's why it's useful to use the internal function like in other examples and even i don't use clap in most of the examples when not necessarily

This place is just to do safe packaging, not to do higher level packaging, and higher level to use those that were made before. The structs you made before could be structs that were wrapped with this safe.

thewh1teagle · 2024-10-29T18:31:15Z

src/recognizer.rs

+//!
+//! [sherpa-onnx]: https://github.com/k2-fsa/sherpa-onnx
+//! [onnxruntime]: https://github.com/microsoft/onnxruntime
+//! [Next-gen Kaldi]: https://github.com/k2-fsa/


Remove all comments

thewh1teagle · 2024-10-29T18:31:44Z

src/recognizer/offline_recognizer.rs

+///
+/// please refer to
+/// https://k2-fsa.github.io/sherpa/onnx/pretrained_models/offline-paraformer/index.html
+/// to download pre-trained models


Remove most of the comments in the files. you can add link to specific sherpa docs on top of the file

Remove most of the comments in the files. you can add link to specific sherpa docs on top of the file

Consider moving it to the entire crate document

I want to avoid maintain documentation as possible. We'll handle that in another pr/issue

thewh1teagle · 2024-10-29T18:31:54Z

src/recognizer/online_recognizer.rs

+/// Configuration for online/streaming models
+///
+/// Please refer to
+/// https://k2-fsa.github.io/sherpa/onnx/pretrained_models/online-transducer/index.html


thewh1teagle · 2024-10-29T18:32:25Z

src/stream/online_stream.rs

+    pub(crate) _marker: PhantomData<T>,
+}
+
+pub trait State {}


Why use trait?

Why use trait?

Check out the latest code.
Simplify the online_stream parameters in online_recognizer.

thewh1teagle · 2024-11-05T09:41:06Z

@ghjdegithub
Maybe now you can see?

ghjdegithub · 2024-11-05T09:43:55Z

@ghjdegithub

Maybe now you can see?

I saw it.

thewh1teagle · 2024-11-05T09:57:26Z

examples/streaming_decode_files.rs

+    let (samples, sample_rate) = read_wave(&args.wave_file);
+
+    println!("Initializing recognizer (may take several seconds)");
+    let config = OnlineRecognizerConfig {


Simplify configs with impl default like in other examples

rust's default means that there can be a default case, but this configuration obviously doesn't. The best practice should be to provide a series of new methods, such as new_ transducer, new_paraformer, etc. default, there really isn't a particular default standard.

I agree but even using new methods for the structus will be too much expressive for irrelevant fields. so maybe it's worth using default and the examples will show how to use it correctly + sherpa-onnx will throw error with info

thewh1teagle · 2024-11-05T10:00:46Z

src/recognizer/online_recognizer.rs

+/// Configuration for online/streaming transducer models
+///
+/// Please refer to
+/// https://k2-fsa.github.io/sherpa/onnx/pretrained_models/online-transducer/index.html


Link to models should placed in examples. Keep the source code clean

thewh1teagle · 2024-11-05T10:01:34Z

src/recognizer/online_recognizer.rs

+/// Configuration for online/streaming paraformer models
+///
+/// Please refer to
+/// https://k2-fsa.github.io/sherpa/onnx/pretrained_models/online-paraformer/index.html


Remove most of the comments keep tiny reference maybe just a link

thewh1teagle · 2024-11-05T10:03:17Z

src/recognizer/offline_recognizer.rs

+
+impl OfflineRecognizer {
+    /// Frees the internal pointer of the recognition to avoid memory leak.
+    fn delete(&mut self) {


Remove. You can free it directly in impl drop

This is a private method, and it is currently only used for drop.

You can remove the private delete methods and do the logic directly in drop

@thewh1teagle I'd recommend keeping it as you might use this private method when writing unit tests.

If we'll need the logic for unit tests we simply can call drop(...) that will call the drop implementation.

thewh1teagle · 2024-11-05T10:06:12Z

src/recognizer/online_recognizer.rs

+    }
+
+    /// Free the internal pointer inside the recognizer to avoid memory leak.
+    fn delete(&mut self) {


Remove but use in impl drop

thewh1teagle · 2024-11-05T10:06:55Z

src/stream/offline_stream.rs

+
+impl OfflineStream {
+    /// Frees the internal pointer of the stream to avoid memory leak.
+    fn delete(&mut self) {


Remove and keep in impl drop

thewh1teagle · 2024-11-05T10:08:19Z

src/stream/offline_stream.rs

+    }
+
+    /// Get the recognition result of the offline stream.
+    pub fn get_result(&self) -> Option<OfflineRecognizerResult> {


Return result instead of option with bail! Macro to emit errors

Return result instead of option with bail! Macro to emit errors

Is there an err type for the appropriate result at the moment

Currently we use eyre::{Result, bail}
We can improve it in future in another PR though, I don't see real benefit of using custom errors in the crates at this stage.

Currently we use eyre::{Result, bail} We can improve it in future in another PR though, I don't see real benefit of using custom errors in the crates at this stage.

In rust practice, if you're a library that returns result, it's a good idea to define your own err and result types. This way, sub-documents can see their own error type and related descriptions. The essence is to facilitate the use of the caller.

We'll handle it later. see #46

ghjdegithub · 2024-11-06T03:24:48Z

@thewh1teagle Can you just modify these PRs

thewh1teagle · 2024-11-06T05:16:49Z

@ghjdegithub
Please start with smaller, specific PRs from a dedicated branch in the future to make them easier to review and modify. I'll take a look at this one as soon as I have time. Thanks again for your contribution!

ghjdegithub · 2024-11-06T08:32:51Z

@ghjdegithub Please start with smaller, specific PRs from a dedicated branch in the future to make them easier to review and modify. I'll take a look at this one as soon as I have time. Thanks again for your contribution!

Next time I used the feature/XXX branch to get it done, so I'll be lazy this time

thewh1teagle · 2024-11-23T23:04:35Z

Next time I used the feature/XXX branch to get it done, so I'll be lazy this time

Yes, and smaller even tiny PRs :)

ghjdegithub added 6 commits October 21, 2024 13:52

example tts add param tts_rule_fsts

a891216

sherpa-rs-sys build.rs compile_error! add target-os cfg. (macos can c…

ccf78a5

…ompile success.)

Merge remote-tracking branch 'origin/main'

6e40a9c

# Conflicts: # sys/build.rs

Merge remote-tracking branch 'origin/main'

9d7c13d

Submit streaming code and examples

8964974

Submit streaming code and examples

18224b0

thewh1teagle mentioned this pull request Oct 29, 2024

Submit streaming code and examples #42

Closed

ghjdegithub added 2 commits October 30, 2024 10:10

streaming examples add a description note

084d09f

Add the sherpa struct of safety no-streaming and example

ac71ac5

ghjdegithub mentioned this pull request Oct 31, 2024

Add zipformer's online feature. #35

Open

solaoi and others added 2 commits November 4, 2024 23:00

fix typo

2633bef

Merge pull request #1 from solaoi/fix/typo

25d3dc9

fix typo

thewh1teagle reviewed Nov 5, 2024

View reviewed changes

Submit streaming code and examples #43

Are you sure you want to change the base?

Submit streaming code and examples #43

Conversation

ghjdegithub commented Oct 29, 2024

ghjdegithub commented Nov 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thewh1teagle commented Nov 5, 2024

ghjdegithub commented Nov 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghjdegithub commented Nov 6, 2024

thewh1teagle commented Nov 6, 2024

ghjdegithub commented Nov 6, 2024

thewh1teagle commented Nov 23, 2024