-
Notifications
You must be signed in to change notification settings - Fork 291
WIP-1: Create asr pipeline and paraformerASR constructor. #2804
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
base: master
Are you sure you want to change the base?
Conversation
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.
Please create/provide JIRA ticket to access if we need arch review for these changes.
In Jira ticket, we expect description of reason of these changes.
| namespace genai { | ||
|
|
||
| // forward declaration | ||
| class ASRPipelineImplBase; |
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.
what is about Whisper pipeline? Will it work with ASRPipeline? I expect -yes. Please add tests for this.
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.
Agree, ASRPipeline should work with Whisper model. Once we add ASRPipeline I expect whisper pipeline to be deprecated.
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.
Yes, I think Whisper pipeline will work with ASRPipeline. Let me also work on this to integrate with Whisper. @rkazants also mentioned tests for whisper. could you also show me the current tests for Whisper pipeline? I can refer to the current implementation. Thanks
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.
| namespace ov { | ||
| namespace genai { | ||
|
|
||
| ParaformerImpl::ParaformerImpl(const std::filesystem::path& model_dir, |
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.
how do you get IRs for paraformer models? Do exporting and inference work in optimum-intel? If no, then we must add it for the full consistent user experience.
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.
The peraformer IRs upstream to huggingface. Here is the link.
https://huggingface.co/apinge/paraformer-zh-int8-ov
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.
No, that is not how it should work. All supported models must pass optimum-cli tool conversion.
This is our deployment pipeline. If not optimum-intel support, model is not enabled and no claim in our documentation.
Create jira ticket for the corresponding task.
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.
Just to make it explicit. The ticket was created: CVS-174743
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.
Pull Request Overview
This PR creates the foundational structure for an ASR (Automatic Speech Recognition) pipeline with a focus on integrating Paraformer ASR functionality.
- Introduces ASRPipeline class with a base implementation interface
- Adds ParaformerImpl as a concrete implementation of the ASR pipeline
- Provides a factory method for creating Paraformer ASR instances
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/cpp/include/openvino/genai/speech_recognition/asr_pipeline.hpp | Public API header defining ASRPipeline class and paraformerASR factory method |
| src/cpp/src/speech_recognition/asr_pipeline.cpp | Implementation of ASRPipeline constructors and paraformerASR factory method |
| src/cpp/src/speech_recognition/asr_pipelinee_impl_base.hpp | Base class interface for ASR pipeline implementations |
| src/cpp/src/speech_recognition/paraformer_impl.hpp | Header for ParaformerImpl class extending the base interface |
| src/cpp/src/speech_recognition/paraformer_impl.cpp | Implementation of ParaformerImpl constructor and destructor |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| #pragma once | ||
|
|
||
| #include "asr_pipelinee_impl_base.hpp" |
Copilot
AI
Oct 8, 2025
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.
Corrected spelling of 'pipelinee' to 'pipeline' in include filename.
| #include "asr_pipelinee_impl_base.hpp" | |
| #include "asr_pipeline_impl_base.hpp" |
| ASRPipelineImplBase(const std::filesystem::path& model_dir, | ||
| const std::string& device, | ||
| const ov::AnyMap& properties = {}) {}; |
Copilot
AI
Oct 8, 2025
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.
[nitpick] Constructor should be explicit and the empty body should be on a separate line for better readability.
| ASRPipelineImplBase(const std::filesystem::path& model_dir, | |
| const std::string& device, | |
| const ov::AnyMap& properties = {}) {}; | |
| explicit ASRPipelineImplBase(const std::filesystem::path& model_dir, | |
| const std::string& device, | |
| const ov::AnyMap& properties = {}) | |
| {} |
| const ov::AnyMap& properties) { | ||
|
|
Copilot
AI
Oct 8, 2025
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.
Constructor is empty but should initialize m_impl member variable or provide implementation details.
| const ov::AnyMap& properties) { | |
| const ov::AnyMap& properties) | |
| : m_impl(std::make_shared<ParaformerImpl>(model_dir, device, properties)) { | |
| assert(m_impl != nullptr); |
Do you have any existing JIRA link i can refers to? I can create new one based on this. Thanks |
Please consult with @EfimovIlia, @moslex about this JIRA tickets creation. |
Description
This PR intends to integrate ASR pipeline which include paraformer ASR.
WIP-1: Create ASRPipeline and paraformerASR constructor.