search

arch1t3cht / Aegisub

Cross-platform advanced subtitle editor, with new feature branches. Read the README on the feature branch.
http://www.aegisub.org
Other
800 stars 34 forks source link

Fix combine_karaoke #127

Open moi15moi opened 7 months ago

moi15moi commented 7 months ago

This PR resolves: https://github.com/Ristellise/AegisubDC/issues/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 commented 7 months ago

I just saw this PR: https://github.com/Aegisub/Aegisub/pull/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

moi15moi commented 3 months ago

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