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

Fix combine_karaoke #127

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

Conversation

moi15moi
Copy link

This PR resolves: Ristellise/AegisubDC#36

I have no idea what the person who created the combine_karaoke method was thinking. To me, the previous results make no sense, so I changed its behavior to match what I think it should do. I have listed many examples to show what I think the result should be.

Example 1 - The line has no collision and the timing of the second line is after the first line:

Dialogue: 0,0:21:18.86,0:21:21.00,ED,,0,0,0,,{\k50}Ko{\k47}no {\k51}ki{\k26}mo{\k40}chi
Dialogue: 0,0:21:22.78,0:21:25.26,ED,,0,0,0,,{\k34}ko{\k20}{\k49}i {\k24}de{\k54}su {\k67}ka

Before this PR (different k-time compared to the two lines):

Dialogue: 0,0:21:18.86,0:21:25.26,ED,,0,0,0,,{\k214}{\k50}Ko{\k47}no {\k51}ki{\k26}mo{\k40}chi{\k248}{\k34}ko{\k20}{\k49}i {\k24}de{\k54}su {\k67}ka

After this PR (same k-time)

Dialogue: 0,0:21:18.86,0:21:25.26,ED,,0,0,0,,{\k50}Ko{\k47}no {\k51}ki{\k26}mo{\k40}chi {\k178}{\k34}ko{\k20}{\k49}i {\k24}de{\k54}su {\k67}ka

Example 2 - Lines have timing collision:

Dialogue: 0,0:21:18.86,0:21:21.00,ED,,0,0,0,,{\k50}Ko{\k47}no {\k51}ki{\k26}mo{\k40}chi
Dialogue: 0,0:21:19.78,0:21:25.26,ED,,0,0,0,,{\k34}ko{\k20}{\k49}i {\k24}de{\k54}su {\k67}ka

Before this PR (different k-time compared to the two lines):

Dialogue: 0,0:21:18.86,0:21:25.26,ED,,0,0,0,,{\k50}Ko{\k47}no {\k51}ki{\k26}mo{\k40}chi {\k34}ko{\k20}{\k49}i {\k24}de{\k54}su {\k67}ka

After this PR (same k-time)

Dialogue: 0,0:21:18.86,0:21:25.26,ED,,0,0,0,,{\k50}Ko{\k47}no {\k51}ki{\k26}mo{\k40}chi {\k-122}{\k34}ko{\k20}{\k49}i {\k24}de{\k54}su {\k67}ka

Example 3 - The line has no collision and the timing of the first line is after the second line:

Dialogue: 0,0:21:40.86,0:21:50.86,ED,,0,0,0,,{\k50}Ko{\k47}no {\k51}ki{\k26}mo{\k40}chi
Dialogue: 0,0:21:19.78,0:21:25.26,ED,,0,0,0,,{\k34}ko{\k20}{\k49}i {\k24}de{\k54}su {\k67}ka

Before this PR (different k-time compared to the two lines):

Dialogue: 0,0:21:40.86,0:21:50.86,ED,,0,0,0,,{\k1000}{\k50}Ko{\k47}no {\k51}ki{\k26}mo{\k40}chi{\k548}{\k34}ko{\k20}{\k49}i {\k24}de{\k54}su {\k67}ka

After this PR (different k-time compared to the two lines):

Dialogue: 0,0:21:40.86,0:21:50.86,ED,,0,0,0,,{\k50}Ko{\k47}no {\k51}ki{\k26}mo{\k40}chi {\k-3108}{\k34}ko{\k20}{\k49}i {\k24}de{\k54}su {\k67}ka

But, please note that Join (concatenate) also doesn't work in this third case!

@moi15moi
Copy link
Author

moi15moi commented Apr 22, 2024

I just saw this PR: Aegisub#49
Prior to this PR, Join (as karaoke) was working as what I am proposing now.

But, from the doc

Join (as karaoke)
Does the inverse of Split (by karaoke), i.e. the same as Join (concatenate) but inserts \k tags with the timing of each source line in the joined line.
Split (by karaoke)
Splits the line into one new line per syllable, as delimited by karaoke override tags (\k and its relatives). The timing of the first line will start at the original line’s start time and end at that time plus the length of the first syllable; the following lines will start at the end of the previous and last for the duration of the syllable.

So, I guess Join (as karaoke) isn't suppose to do what I want.
Should I modify Join (concatenate) and detect if the second line has a \k, \kf, \K or \ko, then I do what I was thinking?
Edit: If I do that, it will also resolve #55

@arch1t3cht arch1t3cht force-pushed the feature branch 3 times, most recently from 914ad0e to 0e44a34 Compare May 21, 2024 12:02
@moi15moi
Copy link
Author

@arch1t3cht I never got an answer from you ^^'

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.

Problem when Join (as karaoke)
1 participant