Skip to content
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

[runtime] Fixed failed compilation without ITN. Now, compiling ITN is mandatory. #2444

Merged
merged 1 commit into from
Mar 27, 2024

Conversation

roney123
Copy link
Contributor

In the original version, it didn't work without itn. but now a further separation of references 'wetext' can ensure that it works without itn.

@xingchensong
Copy link
Member

不传itn的fst就可以了

@roney123
Copy link
Contributor Author

@xingchensong 使用的时候是不传fst就可以。
这个是runtime编译的时候,如果选择不使用itn时,会不加载wetext第三方库,但是在post_processer的代码中,直接#include了wetext,会编译不通过。
现在改的版本,选择不使用itn时,会不加载wetext第三方库,在post_processer的代码中,也不会#include “wetext”

@roney123 roney123 changed the title fix itn, support not use itn(wetext) [runtime] fix itn, support not use itn(wetext) Mar 26, 2024
@xingchensong
Copy link
Member

wetext这个库体积很大吗?不把他编进去的主要好处是什么

@roney123
Copy link
Contributor Author

不是体积大小的原因。
一是目前在cmake配置中ITN是可选,但是选非会编译不成功。
二是公司内部有一套自己的ITN了,再加一套感觉会有些奇怪。
但是比较尴尬的是,post_processer跟decoder结合的比较紧,现在post_processer中又必须有wetext,如果只想用decoder的话,好像没有别的办法了。
我现在pr的这个像是对目前版本的一个fix,如果有办法能把wetext像websocket、grpc一样灵活,就更好了

@xingchensong
Copy link
Member

我倾向于直接改cmakelist,把itn这个可选性删掉,变成必选项。

因为即便按照现在这种方式修改了,配套自己的itn还是要改代码的。

@roney123
Copy link
Contributor Author

按照我们目前的方案,是不需要修改wenet代码,只在外层增加代码就行,方案大概是这样的:
对外接口、音频前处理(重采样、VAD)、wenet(decoder及必须关联的模块)、文本后处理(ITN、punct)
这里wenet也可以替换为kaldi,VAD、ITN和punct与解码器不关联,因为这些都会因为接口请求的参数而改变。
我有一个建议,在接入wetext或者punct时,可不可以放在一个新的不与decoder关联的模块,比如叫text_post_processor中,这样可以保持decoder的独立性和简单性。在使用wenet时,也就可以只用decoder而不用对wenet的魔改,这样也方便跟随wenet的新版本。

另外,还有个小问题,现在使用itn后,单字的时间戳是不是对应不上了…

@xingchensong
Copy link
Member

如果是这样的话,把itn编译进去(但是不使用),也不用魔改wenet代码

@roney123
Copy link
Contributor Author

是可以这么做,但是必须编译的话,又感觉不够简洁了,而我又暂时没想出别的简单方案
我还是先把ITN非选编译失败这个bug修了吧

@roney123 roney123 changed the title [runtime] fix itn, support not use itn(wetext) [runtime] Fixed failed compilation without ITN. Now, compiling ITN is mandatory. Mar 27, 2024
@xingchensong
Copy link
Member

thx。把wetext当成boost一样的基础库来看就不觉得编译进去不简洁了,反而placeholder的方式显式增加了代码注入

@xingchensong xingchensong merged commit 2c26ae4 into wenet-e2e:main Mar 27, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants