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.

Sign up for GitHub

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

Jump to bottom

CTC streaming decoder #2121

Merged
merged 1 commit into from
May 22, 2019
Merged

CTC streaming decoder #2121

merged 1 commit into from
May 22, 2019

Conversation

dabinat
Copy link
Collaborator

@dabinat dabinat commented May 20, 2019

Refactored the CTC decoder to work with streaming data.

@dabinat dabinat mentioned this pull request May 20, 2019
Closed
@kdavis-mozilla kdavis-mozilla requested a review from reuben May 20, 2019 06:23
eggonlea
eggonlea reviewed May 20, 2019
View reviewed changes
Copy link
Collaborator

@eggonlea eggonlea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI. I back ported it to 0.4.1 and can confirm it works great. Thanks!

There're just some trivial coding style issues, mostly trailing space/tab.

native_client/ctcdecode/ctc_beam_search_decoder.cpp Outdated Show resolved Hide resolved
native_client/deepspeech.cc Outdated Show resolved Hide resolved
reuben
reuben reviewed May 20, 2019
View reviewed changes
Copy link
Contributor

@reuben reuben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really good! I have a few initial comments and want to do a few more tests, but I don't expect more changes will be needed.

native_client/deepspeech.cc Outdated Show resolved Hide resolved
native_client/deepspeech.cc Outdated Show resolved Hide resolved
native_client/ctcdecode/ctc_beam_search_decoder.cpp Show resolved Hide resolved
native_client/ctcdecode/ctc_beam_search_decoder.cpp Outdated Show resolved Hide resolved
native_client/ctcdecode/ctc_beam_search_decoder.cpp Outdated Show resolved Hide resolved
@dabinat
Copy link
Collaborator Author

dabinat commented May 21, 2019

Ok, I've addressed all of the requests and also fixed some comments I forgot to update.

eggonlea added a commit to eggonlea/DeepSpeech that referenced this pull request May 21, 2019
@eggonlea
CTC beam search streaming decoder
f70f737 
Backported from PR mozilla#2121

Signed-off-by: Li Li <eggonlea@msn.com>
reuben
reuben approved these changes May 22, 2019
View reviewed changes
Copy link
Contributor

@reuben reuben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment, otherwise looks good.

native_client/ctcdecode/ctc_beam_search_decoder.cpp Outdated Show resolved Hide resolved
CTC beam search streaming decoder (+6 squashed commits)
d9a2694 
Squashed commits:
[2941b47] Fixed nits
[700572e] Restored old CTC decoder API
[5aaf75d] Fixed nits
[969d71a] Added a destructor for DecoderState
[af0be6e] Removed accumulated_logits
[9dcb7b4] CTC beam search streaming decoder
@dabinat
Copy link
Collaborator Author

dabinat commented May 22, 2019

I rebased it into a single commit. Does anyone else need to review it or can I merge it?

eggonlea
eggonlea approved these changes May 22, 2019
View reviewed changes
Copy link
Collaborator

@eggonlea eggonlea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for enabling streaming decoder!

@kdavis-mozilla
Copy link
Contributor

Merge away.

@dabinat dabinat merged commit 69538f2 into mozilla:master May 22, 2019
@lock
Copy link

lock bot commented Jun 21, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Jun 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@eggonlea eggonlea eggonlea approved these changes

@reuben reuben reuben approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dabinat @kdavis-mozilla @reuben @eggonlea