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

Added French lipsyncing module #67

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

Anarbb
Copy link

@Anarbb Anarbb commented Oct 31, 2024

Let me know if anything is wrong with it, I will be creating more in the future.

@met4citizen
Copy link
Owner

Great! Thank you for contributing. I don’t speak French myself, but I can run some basic tests on this next week.

One potential issue I noticed involves letters with diacritics. For example, you included the rule "[É]=E" in the ruleset for the letter "E". However, that rule will never be triggered because when the code encounters a part that starts with the letter "É", it looks only for a ruleset "É", never the ruleset "E". In other words, you'll likely need to create separate rulesets for letters with diacritics. Alternatively, if some diacritics never impact the visemes, you could filter those diacritics out in the preProcessText method (as it is done in the English version).

@met4citizen
Copy link
Owner

met4citizen commented Nov 3, 2024

I just ran some tests on your French module and didn't find any coding errors. Good job! - The rules, however, need some adjustments.

The first issue is the one I already anticipated. For instance, the word "CAFÉ" produces the visemes "kk aa FF". The "E" (or maybe "E I"?) is missing because there are currently no rulesets for letters with diacritics.

The second issue concerns the rule order within each ruleset and the fact that rules are processed sequentially in the order they're defined. For example, in the "A" ruleset, the first rule is "[A]=aa", which matches all segments starting with "A". As a result, any rules defined after it will not get any matches. To fix this, the rule "[A]=aa" should be moved to the end of the "A" ruleset and all the exceptions to be placed before it in proper order.

If you have time to address these issues, I'd be happy to merge this and give you proper attribution. I can also add 'fr' option to the test app.

I should emphasize that since I don't speak French, I'm unable to verify the accuracy of the rules or confirm the correctness of the numbers-to-text conversion. At some point, it would be interesting to test the class against some open-source French phoneme dataset to assess its accuracy, but that can wait. Visually, the lip-sync already looks convincing.

@Anarbb
Copy link
Author

Anarbb commented Nov 5, 2024

Yeah my apologies for those issues. I am working on the lip-syncing modules for a company project on top of the actual project I am building and I have to make ones for other languages too so I don't have much time to 100% verify everything, that's my bad. I'll try to get those fixed for a better accuracy when I get the time to do so. I have currently achieved lip syncing for Arabic, Dutch (Netherlands) and Spanish. I can add those later when they're verified from QA. Appreciate the feedback!

@met4citizen
Copy link
Owner

Sounds good! - Over the past few months, I've received several requests for new lip-sync language modules, so I'm sure I'm not the only one who will appreciate your contribution. The rules don't have to be (and can't be) 100% accurate; the English rules certainly aren't.

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