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 audio support to DataPack #585

Merged
merged 7 commits into from
Jan 12, 2022
Merged

Conversation

mylibrar
Copy link
Collaborator

@mylibrar mylibrar commented Jan 8, 2022

This PR fixes #582.

Description of changes

  • Add new dependency: soundfile
    • soundfile>=0.10.3 is inserted to setup.py and docs/requirements.txt
    • CI workflow main.yml is updated with sudo apt-get install -y libsndfile1-dev as required by soundfile for Linux.
  • Update DataPack with audio support
    • Add _audio for payload and sample_rate for metadata
    • Corresponding operations are also added
  • Add AudioReader to load audio files
    • User can provide a directory with a file extension, and the reader will automatically walk through the directory and load all the files with the specified suffix.

Test Conducted

A unit test for AudioReader is added. It builds and runs an audio processing pipeline for automatic speech recognition (ASR) in order to jointly test AudioReader and DataPack.

@codecov
Copy link

codecov bot commented Jan 8, 2022

Codecov Report

Merging #585 (b004259) into master (818564c) will increase coverage by 0.08%.
The diff coverage is 93.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #585      +/-   ##
==========================================
+ Coverage   79.78%   79.86%   +0.08%     
==========================================
  Files         227      229       +2     
  Lines       16163    16239      +76     
==========================================
+ Hits        12896    12970      +74     
- Misses       3267     3269       +2     
Impacted Files Coverage Δ
forte/data/readers/audio_reader.py 88.00% <88.00%> (ø)
tests/forte/data/readers/audio_reader_test.py 94.87% <94.87%> (ø)
forte/data/data_pack.py 78.27% <100.00%> (+0.45%) ⬆️
forte/data/readers/__init__.py 100.00% <100.00%> (ø)
forte/data/base_pack.py 77.35% <0.00%> (+1.28%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 818564c...b004259. Read the comment docs.

Copy link
Member

@hunterhector hunterhector left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, looks like it works OK.

Some other small suggestions:

  1. We are at a point where we have quite a few data samples in the data_samples folder. Could you add a README.md in that folder? This time let's add a description of what the audio_reader_test folder contains.
  2. Once we add this PR we would need to start documenting the feature, so from the start, let's consider adding a new markdown file in the docs folder, start to serve as a tutorial for the audio project. Then we can make a link from https:/asyml/forte/wiki, and from the root README.md

docs/requirements.txt Show resolved Hide resolved
forte/data/data_pack.py Show resolved Hide resolved
"""
import os
from typing import Any, Iterator
import soundfile
Copy link
Member

Choose a reason for hiding this comment

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

so this reader would depend on the soundfile dependency, which means it needs to be in our core requirement. So what happens would be if we do pip install forte and from forte.data.readers.misc_reader import xxx, this would fail (since this is in the __init__.py)

We need to think of a better way to place this reader. Any suggestions

forte/data/data_pack.py Show resolved Hide resolved
setup.py Show resolved Hide resolved
@hunterhector hunterhector merged commit 0a16879 into asyml:master Jan 12, 2022
@mylibrar mylibrar deleted the localtest branch January 12, 2022 23:17
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.

Add support for audio data to DataPack
2 participants