Unused or incomplete code?

[wholder]

I've been slowly working my way through the code to try and get a feel for how it's organized and have noticed a few odd things that may be incomplete, or unused code. For example, the class FeatureRegistry has a MultiKeyMap object named computers which is later referenced only in this block of code:

  TargetFeatureComputer tfc = (TargetFeatureComputer) computers.get(mgr, features);
  if (tfc == null) {
    tfc = new TargetFeatureComputer(mgr, features);
  }
  return tfc;

However computers is only read from, never written so, IMO, this code needs to be changed to store the created TargetFeatureComputer object into the computers MultiKeyMap, or simplified to:

return new TargetFeatureComputer(mgr, features);

Wayne

[psibre]

Thanks for your feedback!

Please keep in mind that some of this code was written more than 15 years ago by people with limited programming experience, and even today, the project is not staffed with any full-time software engineers.

Having said that, we're more than happy to accept pull requests. The issue you raise here is certainly a reasonably low-hanging fruit, and a great candidate to improve the codebase, bit by bit (no pun intended).

[wholder]

When I get a bit more familiar with the code and there's a resolution to the Gradle build issue, I'd like to work on updating some of the more outdated code. I also need to figure out how to run the various units tests that seem so be built into a lot of the code. In addition, I've discovered that, due to how many of the modules are loaded dynamically using class.forName(), that I cannot really trust when InteiilJ analysis tools tells me some bit of code is unused. So far, I've learned to be cautious with any subclass of InternalModule. But, is there any kind of overview that details how some of these loading mechanisms work and which other bits of code are dynamically loaded?

Wayne

[psibre]

AFAIA, reflection is used mainly via the service loader mechanism for classpath discovery. MaryTTS was designed to work with new language and voice components "installed" by copying the jar files into the MARYBASE/lib directory. Still seems like an elegant solution today, but takes a moment to understand for developers who have not used that technique before.

The languages and voices can also pack their own InternalModules, so technically a voice could be packaged with its own variant of many modules for NLP and/or synthesis. (Which, incidentally, would have been a better way to provide novel unit selection features like the "statistical model cost" used for our Blizzard Challenge 2013 submission -- this feature was never used in the regular MaryTTS voices, yet redundant logic branches to handle sCost have since been cluttering nearly all parts of the marytts.unitselection package...)

[psibre]

P.S. Although now I realize that the statistical cost unitselection variant was actually developed quite a bit earlier (before I joined the project, in fact), around marytts/marytts@260ea4e2b2fa081537513d703a1bbea6afac3de0.

[wholder]

As an experiment, I decided to just hack away and see just how many things I could remove and still use the voice in the voice-cmu-rms-hsmm-5.2.jar file. After a lot of trial and error, I was able to remove a lot of code, as well as significantly reduce the set of support JAR files needed (excluding logging-related files that will be common to my other code) down to this set:

• emotionml-checker-java-1.1.jar
• jtok-core-1.9.3.jar
• opennlp-tools-1.9.2.jar

However, this required creating more compact, substitute code for some of the other libraries/classes needed, such as:

• Jama Matrix
• Apache MultiKeyMap
• Apache IOUtuls
• Apache StringUtils
• ICU4J (substituted English-only code for RuleBasedNumberFormat)

When I bundle this hacked-down, English-only version with my test code, I get a final JAR file of 9.8MB. This contrasts with the 36.1MB JAR file I previously got when building with the unmodified, original JAR files. In addition, the load/init time dropped from 2762ms to 500ms. I suspect there are additional reductions I can make in there resources files, but I haven't yet figured out an efficient way to determine the dependencies there.

This cut down version can still seem to use the various effects, such as Robot, Whisper, etc., but I think I may have broken the Rate effect, as it doesn't seem to do anything. Or, perhaps I don't understand what this effect is supposed to do, as it also doesn't seem to do anything when I try the same test with the unmodified MaryTTS jar files. Here's the test I tried:

  marytts.setAudioEffects("FIRFilter")
  say("Extreme caution is advised.");
  marytts.setAudioEffects("FIRFilter+Rate(durScale:0.5;);");
  say("Extreme caution is advised.");

To my ear, both sound the same. Should I expect to hear a difference?

Wayne

[psibre]

Great to hear you managed to prune away unused transitive dependencies and code!

However, please be aware that your efforts at reducing redundant or unused code might be more productively aimed at the master branch of this repository (in the form of pull requests), to avoid a lot of duplicate effort...

Regarding excluding unused transitive dependencies, this is easily accomplished using Gradle, see https://docs.gradle.org/6.3/userguide/dependency_downgrade_and_exclude.html#sec:excluding-transitive-deps

To simply strip unused classes and reduce the footprint of your fat Jar, you can use solutions like ProGuard, see https://www.guardsquare.com/en/products/proguard/manual/introduction

Regarding the DSP filters, the code for those has been unmaintained for years, and not all of them work as expected anyway.

[wholder]

*> However, please be aware that your efforts at reducing redundant or unused code might be more productively aimed * > at the master branch of this repository (in the form of pull requests), to avoid a lot of duplicate effort...

I do hope to be able to do that once I understand the structure of the code better. The process I use to pare things down was pretty brutal, as it wasn't simply a matter of removing class files. In many cases, I had to hack out portions of code across many different files through many, successive iterations, which made it hard to keep track of the overall effect on the code. However, now that I have something much smaller that works, I can use it as reference to compare back to the master source and look for areas where I can recommend changes.

However, as I mentioned before, the weird error messages I get when I try to run some of the Gradle scripts (error: unmappable character for encoding ASCII final String letters), have me worried about being able to properly test any changes I make to the code before submitting a pull request. In fact, I'm not really sure what script, or set of scripts I should run to test any changes I might make. Advice? Any progress on understanding root cause of these error messages?

> To simply strip unused classes and reduce the footprint of your fat Jar, you can use solutions like ProGuard

Unfortunately, in several cases, such as the number pronunciation code, I would up writing completely new code, as the code in the library was entangles with many other classes that duplicated functionality already available in Java. One big area of duplication was in String-related functions where several off the libraries each had their own take on various, basic string operations.

There's also quite a bit of unused code in the marytts.util.string.StringUtils class where at least 17 methods show as being completely unused elsewhere in the code. Actually, the number of unused methods is more like 25+, as more dependent functions in the same class show as unused once these 17 are removed. Ditto for the marytts.util.MaryUtils class where 11 methods show as unused but, again, if you delete these unused methods, more dependent methods pop up as unused. This is a pattern I saw thoughout the code where unused functions in one place tends to hide unused code in other places. From my point of view, this would be good place to start in cleaning up the code, especially as it's so easy to spot with InteiilJ's analysis tools. That is, unless there is some reason why this code is still in the project? Is this code fair game for removal, or is some code being kept for possible future use?

> Regarding the DSP filters, the code for those has been unmaintained for years, and not all of them work as expected anyway.

That's unfortunate. The filters were one of the features I liked best about the system, as I have several ideas as to how they can be used to good effect in a game. My thought was that the "Rate" effect was used to speed up, or slow down the rate of speech, which would be a very useful feature. If so, I may look into trying to get it working again. Can you provide any more info on how the various effects were supposed to work? Was the Rate filter intended as a way to control the rate of speech?

Some other questions

I'm still having trouble understanding how the various pieces of the speech synthesis chain fit together and what each part does. In particular, what do the HTSCARTReader, MaryCARTReader and WagonCARTReader classes do? The comments in the source files say things like "IO functions for CARTs in MaryCART format"... But what are CARTs?

Also, I don't know if this is possible, but I was hoping to find a way to generate a parallel stream of data (or a special type of audio stream) that included metadata about the phonemes being spoken at ay given moment. The idea was to potentially use this to drive lip and facial animation for an game character. I can approximate this in a crude, Robbie the Robot kind of way by using the amplitude of the speech waveform, but I'm curious to see if having access to phoneme data might make it possible to select proper jaw and lip positions.

Wayne

On Sat, May 23, 2020 at 10:04 PM psibre [email protected] wrote:

Great to hear you managed to prune away unused transitive dependencies and code!

However, please be aware that your efforts at reducing redundant or unused code might be more productively aimed at the master branch of this repository (in the form of pull requests), to avoid a lot of duplicate effort...

Regarding excluding unused transitive dependencies, this is easily accomplished using Gradle, see

https://docs.gradle.org/6.3/userguide/dependency_downgrade_and_exclude.html#sec:excluding-transitive-deps

To simply strip unused classes and reduce the footprint of your fat Jar, you can use solutions like ProGuard, see https://www.guardsquare.com/en/products/proguard/manual/introduction

Regarding the DSP filters, the code for those has been unmaintained for years, and not all of them work as expected anyway.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/marytts/marytts/issues/984#issuecomment-633180043, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIDM4QORLPTJOEH2TBQV4TRTCTFBANCNFSM4NBBPJAA .

[psibre]

Thanks for your thoughts! My two cents:

1. The (possibly) redundant effort I alluded to seems quite real -- many of the problems you mention (duplicate string handling code, etc.) should have been fixed since v5.2 was released. I really do recommend that you base your efforts on the current master branch, not on v5.2.

2. The filter implementations are indeed a rare beast -- there aren't many DSP libraries written in Java. But unfortunately, the author of that code left the MaryTTS project about 12 years ago, and the code itself is written in a very low-level style, and hard to maintain. But please feel free to contribute fixes where the filters are not working as expected.

[wholder]

many of the problems you mention (duplicate string handling code, etc.) should have been fixed since v5.2 was released. I really do recommend that you base your efforts on the current master branch, not on v5.2.

I'm confused. I started with the code in this Github project. Is this not the latest code?

Wayne

[psibre]

No, the confusion is mine! =)

When you mentioned voice-cmu-rms-hsmm-5.2.jar above, I somehow assumed that you were pulling that in as a dependency for your application, and would therefore be stripping away redundant classes from the contents of the marytts-lang-en-5.2.jar, marytts-runtime-5.2.jar files directly, etc.

If you're actually overriding transitive dependencies for voice-cmu-rms-hsmm...

https://github.com/marytts/gradle-marytts-voicebuilding-plugin/blob/ce8181b4852d65843a6ca833c38198c3f588037f/src/main/groovy/de/dfki/mary/voicebuilding/VoicebuildingBasePlugin.groovy#L46-L61

...by including a clone of the current master branch in a composite build or similar, than that's of course awesome!

It's hard for me to understand how you're integrating MaryTTS into your project at this point, but I look forward to hearing more about the outcome of your efforts!