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

add chunked sph file processing #367

Merged

Conversation

videodanchik
Copy link
Contributor

This MR aims to provide an optimal way for reading SPHERE files, avoiding reading the whole file, when offset and duration are provided. These parameters can be specified directly in sph2pipe command which helps to load only chunk of audio in memory during lhotse.audio.read_audio function call.

lhotse/audio.py Outdated
@@ -15,6 +15,7 @@
from typing import Any, Callable, Dict, Iterable, List, Mapping, NamedTuple, Optional, Sequence, Tuple, Union

import numpy as np
import soundfile as sf
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please stick to a local import, otherwise building the docs is going to fail (the import tries to load libsndfile.so into memory which is not available on read-the-docs servers).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, sorry, will move it back

@@ -231,8 +232,7 @@ def from_file(
else:
try:
# Try to parse the file using pysoundfile first.
import soundfile
info = soundfile.info(str(path))
info = sf.info(str(path))
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there a missing case for using sph_info?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, good catch.

lhotse/audio.py Outdated Show resolved Hide resolved
@pzelasko
Copy link
Collaborator

Thanks that looks good! Please address my comments and we'll merge it.

@videodanchik
Copy link
Contributor Author

Import and SPHERE file handling in Recording are fixed.

@videodanchik
Copy link
Contributor Author

videodanchik commented Aug 13, 2021

@pzelasko Can we merge this as I want to add Fisher English that will depend on this PR?

@pzelasko
Copy link
Collaborator

Hmm, the tests are failing because there is no sph2pipe in the CI. Let me try to add it, and then we can merge.

@pzelasko
Copy link
Collaborator

@videodanchik I made a PR #370 that makes it easy to install sph2pipe in a way that enables Lhotse to auto-discover it. I'll wait for Dan to say what he thinks; as soon as it's merged, we can try re-run the CI for your PR again with no code changes -- it should help.

@videodanchik
Copy link
Contributor Author

@pzelasko Oh, sorry that it caused this additional effort, I haven't even realized that you can't install sph2pipe via sudo apt install.

@pzelasko
Copy link
Collaborator

no worries, I think that it’s better for Lhotse users not to have to worry about how to find these esoteric programs…

@pzelasko
Copy link
Collaborator

Seems to work now, merging.

@pzelasko pzelasko merged commit 82286dc into lhotse-speech:master Aug 14, 2021
@pzelasko pzelasko added this to the v0.8 milestone Aug 14, 2021
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