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

Live2D with Lipsync (Outdated) #117

Open
wants to merge 55 commits into
base: master
Choose a base branch
from
Open

Conversation

RaSan147
Copy link

@RaSan147 RaSan147 commented Oct 6, 2023

No description provided.

Changes:
1. updated model.motion(group, index, priority) to model.motion(group, index, priority, sound, volume, expression);
2. added model.stopSpeaking()
3. updated readme.MD with demos
4. Workflow will save files to dist (won't be gitignored)
5. Praying this new change doesn't break
voice volume expressions are now optional arg {name: value, ....}
Copy link
Owner

@guansss guansss left a comment

Choose a reason for hiding this comment

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

Sorry for the long wait! You did a great job on this PR and I really appreciate the effort you put into it.

I've added a few comments for things that need to be addressed before this can be merged.

.github/workflows/test.yml Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
Comment on lines +15 to +17
#### Feel free to support the Maintainer:
<a href="https://www.buymeacoffee.com/RaSan147" target="_blank"><img src="https://cdn.buymeacoffee.com/buttons/v2/default-yellow.png" alt="Buy Me A Coffee" style="height: 60px !important;width: 217px !important;" ></a>

Copy link
Owner

Choose a reason for hiding this comment

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

It's okay to put this in your fork, but not into the main repository.

Copy link
Author

Choose a reason for hiding this comment

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

Will make another fork 😅 copy pasted it in all my repo. Sorry.

@@ -97,16 +100,24 @@ import { Live2DModel } from 'pixi-live2d-display/cubism2';
import { Live2DModel } from 'pixi-live2d-display/cubism4';
```

#### Via CDN
#### Via CDN (lipsync patched)
Copy link
Owner

Choose a reason for hiding this comment

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

This and the (not lipsync patch) above is unnecessary after this feature is merged.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah will change that.

README.md Outdated
Comment on lines 114 to 120
<script src="https://cdn.jsdelivr.net/gh/RaSan147/pixi-live2d-display@v0.4.0-ls-2/dist/index.min.js"></script>

<!-- if only Cubism 2.1 -->
<script src="https://cdn.jsdelivr.net/npm/pixi-live2d-display/dist/cubism2.min.js"></script>
<script src="https://cdn.jsdelivr.net/gh/RaSan147/pixi-live2d-display@v0.4.0-ls-2/dist/cubism2.min.js"></script>

<!-- if only Cubism 4 -->
<script src="https://cdn.jsdelivr.net/npm/pixi-live2d-display/dist/cubism4.min.js"></script>
<script src="https://cdn.jsdelivr.net/gh/RaSan147/pixi-live2d-display@v0.4.0-ls-2/dist/cubism4.min.js"></script>
Copy link
Owner

Choose a reason for hiding this comment

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

These links should point to the main repository.

@@ -1,7 +1,7 @@
import { logger, remove } from '@/utils';

const TAG = 'SoundManager';
const VOLUME = 0.5;
const VOLUME = 0.9;
Copy link
Owner

Choose a reason for hiding this comment

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

This should not be changed. 0.9 would be too loud as a default volume.

Copy link
Author

Choose a reason for hiding this comment

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

However since the lip movement value is dependent on the audio volume, lowering it makes unusual movement (like mumbling)

Copy link
Owner

Choose a reason for hiding this comment

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

Is it possible to add a multiplier after getting the value? Some thing like amplitude *= 2

Copy link
Author

Choose a reason for hiding this comment

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

nope, like if you set volume to 0, there will be no response, no matter how the audio or multiplier is (Browser JS API, cant change).... But yeah multiplier can be really great, will try to add one.

Copy link
Author

Choose a reason for hiding this comment

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

tried, sending Amp variable to analizer is hard. Let it be...

@@ -39,6 +41,8 @@ export class SoundManager {

audio.volume = this._volume;
audio.preload = 'auto';
audio.autoplay = true;
Copy link
Owner

Choose a reason for hiding this comment

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

autoplay should not be true because we are not immediately playing the audio. When config.motionSync is on, we'll postpone it until the related motion/expression is ready.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the info

@@ -39,6 +41,8 @@ export class SoundManager {

audio.volume = this._volume;
audio.preload = 'auto';
audio.autoplay = true;
audio.crossOrigin = "anonymous";
Copy link
Owner

Choose a reason for hiding this comment

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

crossOrigin should not be set by default, otherwise if the audio's source is cross-origin but the server does not respond with a CORS header, the audio will be blocked by the browser.

It should be an option, for example, model.speak('audio.mp3', { crossOrigin: true })

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

@@ -33,9 +34,12 @@ export class Cubism2InternalModel extends InternalModel {
angleZParamIndex: number;
bodyAngleXParamIndex: number;
breathParamIndex: number;
mouthFormIndex: number;
Copy link
Owner

Choose a reason for hiding this comment

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

This property seems unused.

Copy link
Author

Choose a reason for hiding this comment

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

I do remember it using somewhere in cubism 2, cubism 4 has it by default. It takes the mouthForm id...

Comment on lines +249 to +251
updateFacialEmotion(mouthForm: number) {
this.coreModel.addParameterValueById(this.idParamMouthForm, mouthForm); // -1 ~ 1
}
Copy link
Owner

Choose a reason for hiding this comment

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

This is not related to lip sync, I suppose?

Copy link
Author

Choose a reason for hiding this comment

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

Thats the main thinggggg. 😅 It makes the lips move.

@RaSan147
Copy link
Author

RaSan147 commented Dec 7, 2023

Sorry for the late reply.
I'll make a branch with the changes you mentioned (also fixing a lot of merge conflict from the latest update)
Thanks for taking your time and looking into it. This project is awesome.

@RaSan147 RaSan147 changed the title Live2D with Lipsync (using audio file/link) Live2D with Lipsync (Outdated) Dec 14, 2023
RaSan147 and others added 4 commits December 14, 2023 19:05
Co-authored-by: Guan <46285865+guansss@users.noreply.github.com>
Co-authored-by: Guan <46285865+guansss@users.noreply.github.com>
Co-authored-by: Guan <46285865+guansss@users.noreply.github.com>
also remove cache buster and autoplay
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.

None yet

2 participants