Skip to content

WaveData::Read unspecified streams; unsupport 8 and 32 bit formats#1546

Merged
danpovey merged 3 commits intokaldi-asr:kaldi_52from
kkm000:wav-read
Apr 17, 2017
Merged

WaveData::Read unspecified streams; unsupport 8 and 32 bit formats#1546
danpovey merged 3 commits intokaldi-asr:kaldi_52from
kkm000:wav-read

Conversation

@kkm000
Copy link
Contributor

@kkm000 kkm000 commented Apr 15, 2017

This continues and supersedes #338 by @Minhua722.

Still missing tests, will add some over the weekend.

@danpovey
Copy link
Contributor

Thanks.
Please make this PR against the kaldi_52 branch; it will help test it more before making it official.

@kkm000
Copy link
Contributor Author

kkm000 commented Apr 16, 2017

@danpovey, do you want to cherry-pick the commits after they are merged into master into a separate branch and a PR? The structure of the branch kaldi_52 is quite complex and non-obvious, and it is divergent in respect to master.

@kkm000
Copy link
Contributor Author

kkm000 commented Apr 16, 2017

@danpovey Or do you mean just rebase my changes onto kaldi_52 and make the PR against kaldi_52 only?

@kkm000 kkm000 changed the title WIP: WaveData::Read unspecified streams; unsupport 8 and 32 bit formats WaveData::Read unspecified streams; unsupport 8 and 32 bit formats Apr 16, 2017
@kkm000
Copy link
Contributor Author

kkm000 commented Apr 16, 2017

Ah, and FWIW, #1547 is yet unmerged, so accepting this PR into master first will bring it into that open PR as well.

Oops, it goes the other way round. I think I understand now what you want me to do with my PR.

@kkm000 kkm000 changed the base branch from master to kaldi_52 April 16, 2017 06:26
@kkm000
Copy link
Contributor Author

kkm000 commented Apr 16, 2017

Rebased to kaldi_52

@danpovey
Copy link
Contributor

Thanks! So this is ready to merge?

@kkm000
Copy link
Contributor Author

kkm000 commented Apr 17, 2017

Yes, please. Tests only cover basic positive cases (expected wave data format, no malformed files etc.), but at the very least I verified that the new functionality worked.

@danpovey danpovey merged commit 0799b4d into kaldi-asr:kaldi_52 Apr 17, 2017
@kkm000 kkm000 deleted the wav-read branch April 17, 2017 20:25
danpovey added a commit that referenced this pull request Apr 22, 2017
@danpovey
Copy link
Contributor

I have reverted this PR from the command line since the 'revert' button from github didn't work.

@kkm000 kkm000 restored the wav-read branch April 26, 2017 22:42
Skaiste pushed a commit to Skaiste/idlak that referenced this pull request Sep 26, 2018
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