Skip to content

[src] WaveData::Read streams of unspecified length#1590

Merged
danpovey merged 2 commits intokaldi-asr:kaldi_52from
kkm000:wav-read
May 3, 2017
Merged

[src] WaveData::Read streams of unspecified length#1590
danpovey merged 2 commits intokaldi-asr:kaldi_52from
kkm000:wav-read

Conversation

@kkm000
Copy link
Contributor

@kkm000 kkm000 commented Apr 28, 2017

Slightly modified work of @Minhua722 lifted from PR #338. Key changes:

  • Only 16-bit signed linear PCM sample format is supported. For everything else, there is SoX.
  • class WaveInfo separated from WaveData, and now used by wav-to-duration.
  • Added basic wave reader tests.

Closes: #88
Closes: #338
Closes: #1567

Slightly modified work of @Minhua722 lifted from PR kaldi-asr#338. Key changes:

 * Only 16-bit signed linear PCM sample format is supported. For
   everything else, there is SoX.
 * class WaveInfo separated from WaveData, and now used by wav-to-duration.
 * Added basic wave reader tests.

Closes: kaldi-asr#88
Closes: kaldi-asr#338
Closes: kaldi-asr#1567
@danpovey
Copy link
Contributor

Thanks! I'll try to test again, and merge, in the next few days

if (strcmp(tmp, expected))
KALDI_ERR << "WaveData: expected " << expected << ", got " << tmp;
}
// A utility class closure for reading wave header
Copy link
Contributor

Choose a reason for hiding this comment

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

'closure' is not a concept that will be familiar to many readers; can you please rename this class and remove 'closure' from the comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Hope the new name works. I would generally avoid calling anything "HeaderReader."

BaseFloat duration = wave_data.Duration();
const WaveInfo &wave_info = wav_reader.Value();
if (wave_info.IsStreamed())
KALDI_ERR << "Error: member " << key << " has no manifest duration. "
Copy link
Contributor

Choose a reason for hiding this comment

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

can you please change 'manifest duration' -> 'duration in header'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@kkm000
Copy link
Contributor Author

kkm000 commented Apr 29, 2017

I am on a trip, will do on Tuesday!

@danpovey
Copy link
Contributor

danpovey commented May 3, 2017

Thanks! Merging.

@danpovey danpovey merged commit 010c556 into kaldi-asr:kaldi_52 May 3, 2017
@kkm000 kkm000 deleted the wav-read branch June 21, 2017 01:16
Skaiste pushed a commit to Skaiste/idlak that referenced this pull request Sep 26, 2018
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