Skip to content

Speechmatics adapter update #190

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

Merged
merged 5 commits into from
Sep 24, 2019

Conversation

murezzda
Copy link
Contributor

Is your Pull Request request related to another issue in this repository ?
No

Describe what the PR does
The speechmatics STT-adapter is updated with the following points:

  1. Fixed a bug which led to the wrong assignment of speakers (33b7523)
  2. Added new paragraph breaking rules (f687b08 / 7503036):

A new paragraph is created when the speaker changes
or
when the word count of a single speaker paragraph exeeds 150 words and a sentence is finished (indicated by .?!)

State whether the PR is ready for review or whether it needs extra work
Ready for review.

@murezzda
Copy link
Contributor Author

Checks fail as example output of speechmatics adapter is changed, will provide new ground truth.

@murezzda murezzda force-pushed the speechmatics-adapter-update branch from ff8bf60 to 10a87b7 Compare September 24, 2019 12:56
Copy link
Contributor

@emettely emettely left a comment

Choose a reason for hiding this comment

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

The changes look fine to me :)
I’m not too across how speechmatics works, but the only bits that I think could be buggy is if the element.time doesn't exist for whatever reason, and you get a NaN after running float(element.time). I imagine this doesn't happen very often.

@emettely emettely closed this Sep 24, 2019
@emettely emettely reopened this Sep 24, 2019
@emettely emettely merged commit d199690 into bbc:master Sep 24, 2019
@emettely
Copy link
Contributor

Apologies @murezzda, @jamesdools pointed out: I meant to merge, but I closed it 🤦‍♂

@murezzda
Copy link
Contributor Author

Hi @emettely, thanks for the merge!

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