Modify SpeechSynthesisDataset class, make it return text

[yaozengwei]

This PR is required by the TTS recipe k2-fsa/icefall#1372 in icefall. In this TTS recipe, we convert the transcript text to phonemes in training.

[pzelasko]

Looks good, but don't you prefer to have G2P inside SpeechSynthesisDataset to avoid spending the time running it inside the training loop?

[pzelasko]

This change is breaking, can you move return_cuts to some position towards the end of parameter list?

[yaozengwei]

OK.

[yaozengwei]

Looks good, but don't you prefer to have G2P inside SpeechSynthesisDataset to avoid spending the time running it inside the training loop?

Hi, @pzelasko, I prefer to do the text normalization and tokenization in separate training recipes, since they usually depend on different packages. @csukuangfj suggests doing this in data preparation stage, with converted phonemes saved to manifests.

[pzelasko]

Looks good, but don't you prefer to have G2P inside SpeechSynthesisDataset to avoid spending the time running it inside the training loop?

Hi, @pzelasko, I prefer to do the text normalization and tokenization in separate training recipes, since they usually depend on different packages. @csukuangfj suggests doing this in data preparation stage, with converted phonemes saved to manifests.

OK cool. It looks like there are some conflicts after merging the other PR with multi-speaker support, could you resolve them?

[yaozengwei]

Looks good, but don't you prefer to have G2P inside SpeechSynthesisDataset to avoid spending the time running it inside the training loop?

Hi, @pzelasko, I prefer to do the text normalization and tokenization in separate training recipes, since they usually depend on different packages. @csukuangfj suggests doing this in data preparation stage, with converted phonemes saved to manifests.

OK cool. It looks like there are some conflicts after merging the other PR with multi-speaker support, could you resolve them?

Ok. Thanks. I have some local changes and will resolve the conflicts later.

In addition, in current implementation, it will load the whole cuts set and generate a char-based vocabulary according to the given texts. But I think TTS recipes usually use phoneme tokens instead. Also, if might cause token-id-mapping mismatch bettwen training and test, when separately given two cuts sets for this class. Shall we remove this and make this calss return raw text and optionally pre-converted (phoneme) tokens?

lhotse/lhotse/dataset/speech_synthesis.py

Line 43 in 7061175

self.token_collater = TokenCollater(cuts, add_eos=add_eos, add_bos=add_bos)

[pzelasko]

In addition, in current implementation, it will load the whole cuts set and generate a char-based vocabulary according to the given texts. But I think TTS recipes usually use phoneme tokens instead. Also, if might cause token-id-mapping mismatch bettwen training and test, when separately given two cuts sets for this class. Shall we remove this and make this calss return raw text and optionally pre-converted (phoneme) tokens?

I see that this class accepts cuts in constructor, it's an outdated design we don't use anymore, so thanks for updating it. Instead, it would make sense to pass a tokenizer object/callable that (optionally) converts raw text into some tokens. What you're suggesting also works.

[yaozengwei]

@pzelasko Thanks. It is ready to be merged.

[yaozengwei]

@pzelasko @desh2608 Would you mind giving a review if you have time? I have fixed the test case.

[pzelasko]

Thanks, LGTM!