Skip to content

Fix and add functions in speaker utils#4138

Merged
tango4j merged 16 commits intomainfrom
fix_speaker_utils
May 25, 2022
Merged

Fix and add functions in speaker utils#4138
tango4j merged 16 commits intomainfrom
fix_speaker_utils

Conversation

@tango4j
Copy link
Collaborator

@tango4j tango4j commented May 9, 2022

What does this PR do ?

This PR fixes bugs for certain cases and adds few functions. Previously, write_rttm2manifest function had following issues:

(1) Offset is not considered
(2) Occasionally, the function was returning the segments outside the given (offset, offset+duration) range.

The newly written part is compared with the original code, and confirmed to

Collection: [Note which collection this PR will affect]

ASR

Changelog

  • Add specific line by line info of high level changes in this PR.

Added following functions in speaker_utils.py.
These functions are needed to extract oracle VAD stamp from manfiest file, considering the offset and duration.
In addition, these functions are designed for the future use for streaming diarization.

get_uniq_id_with_dur()
getSubRangeList()
getMinMaxOfRangeList()
getMergedRanges()
int2fl()
fl2int()
combine_int_overlaps()
combine_float_overlaps()
getOverlapRange()
isOverlap()

and modified following functions:
write_rttm2manifest() - to resolve existing issues
segments_manifest_to_subsegments_manifest() - Now, if include_uniq_id = True, the function adds uniq_id to the created subsegment manifest file. This is for using segmentation files for truncated (chopped) diarization dataset, where uniq_id is overlapped if not specified. (One audio file creates multiple 30~40sec utterances and these short utterances should be identifed too.)

Details can be found in the docstring in each function definition.

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you add or update any necessary documentation?
  • Does the PR affect components that are optional to install? (Ex: Numba, Pynini, Apex etc)
    • Reviewer: Does the PR have correct import guards for all optional libraries?

PR Type:

  • New Feature
  • Bugfix
  • Documentation

If you haven't finished some of the above items you can still open "Draft" PR.

Who can review?

Anyone in the community is free to review the PR once the checks have passed.
Contributor guidelines contains specific people who can review PRs to various areas.

Additional Information

  • Related to # (issue)

tango4j added 3 commits May 9, 2022 12:27
Signed-off-by: Taejin Park <tango4j@gmail.com>
Signed-off-by: Taejin Park <tango4j@gmail.com>
Signed-off-by: Taejin Park <tango4j@gmail.com>
@tango4j tango4j changed the title Fix speaker utils Fix and add functions in speaker utils May 9, 2022
@tango4j tango4j requested a review from nithinraok May 9, 2022 20:05
tango4j added 8 commits May 9, 2022 16:41
Signed-off-by: Taejin Park <tango4j@gmail.com>
Signed-off-by: Taejin Park <tango4j@gmail.com>
Signed-off-by: Taejin Park <tango4j@gmail.com>
Signed-off-by: Taejin Park <tango4j@gmail.com>
Signed-off-by: Taejin Park <tango4j@gmail.com>
@NVIDIA-NeMo NVIDIA-NeMo deleted a comment from lgtm-com bot May 10, 2022
@NVIDIA-NeMo NVIDIA-NeMo deleted a comment from lgtm-com bot May 10, 2022
@NVIDIA-NeMo NVIDIA-NeMo deleted a comment from lgtm-com bot May 10, 2022
@NVIDIA-NeMo NVIDIA-NeMo deleted a comment from lgtm-com bot May 10, 2022
@NVIDIA-NeMo NVIDIA-NeMo deleted a comment from lgtm-com bot May 10, 2022
@NVIDIA-NeMo NVIDIA-NeMo deleted a comment from lgtm-com bot May 10, 2022
@titu1994
Copy link
Collaborator

/blossom-ci

Copy link
Member

Choose a reason for hiding this comment

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

Can you replace this piece of code with #4125 for getting uniq name.

This avoids issues if basename has dot in it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Replaced with the line Yang has changed.

Copy link
Member

Choose a reason for hiding this comment

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

same here as mentioned above for uniqname fix

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This line is also replaced with the line Yang pushed in PR #4125.

Copy link
Member

Choose a reason for hiding this comment

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

specified file -> specified manifest file

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added "manifest'.

Copy link
Member

@nithinraok nithinraok left a comment

Choose a reason for hiding this comment

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

LGTM

@tango4j tango4j merged commit 7075644 into main May 25, 2022
@nithinraok nithinraok deleted the fix_speaker_utils branch May 25, 2022 20:57
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.

3 participants

Comments