search

sachac / subed

subed is a subtitle editor for Emacs
177 stars 16 forks source link

Add ability to proportionally scale subtitles. #41

Closed brownts closed 3 years ago

brownts commented 3 years ago

This capability provides scaling of subtitles (proportionally, based on their current spacing) over a different (extended or contracted) time range. This can be helpful to correct synchronization issues in existing subtitle files.

rndusr commented 3 years ago

Fancy! I was thinking about adding something like this myself, but it seemed too complicated. Turns out it really is complicated!

I found a bug you should be able to reproduce with these subtitles:

1
00:01:00,000 --> 00:01:02,000
a

2
00:01:03,000 --> 00:01:04,000
b

3
00:01:06,000 --> 00:01:07,000
c

When I press C-u 1000 C-M-x, I end up with this:

1
00:01:00,000 --> 00:01:02,000
a

2
00:01:13,500 --> 00:01:14,500
b

3
00:01:07,000 --> 00:01:08,000
c

Notice how subtitle 2 travelled unexpectedly 10.5 seconds into the future.

brownts commented 3 years ago

Oops...I guess I accidentally invented subtitle time travel...how embarrassing! :wink:

Thanks for the test case, it was quite useful. I think the reason I missed this is that I was testing with start times that were close to zero. This shows a definite problem in the time shift calculation, but I know where it went wrong. I'll push a fix for this shortly.

rndusr commented 3 years ago

Thanks for the fix. But I found yet another bug.

1
00:00:43,233 --> 00:00:45,861
a

2
00:00:51,675 --> 00:00:54,542
b

3
00:01:00,717 --> 00:01:02,378
c

4
00:01:02,452 --> 00:01:05,216
d

Mark subtitles 1, 2 and 3 and press C-M-x.

I'm getting: Wrong type argument: number-or-marker-p, nil

I think this is because 4 is too close to 3 and it's not included in the marked region.

If you have the time, it would probably be a good idea to write tests to cover all these cases.

brownts commented 3 years ago

I fixed this additional issue and added a suite of tests to test out the functionality.

rndusr commented 3 years ago

So many tests. Thank you. Great work.

This seems to be working pretty well. One issue I have is that emacs-nox doesn't differentiate between C-M-x and C-M-S-x, but I don't think there's a lot one can do about that.

brownts commented 3 years ago

No problem, happy to contribute.

I'm not tied to the current set of keybindings, so if something works better, that's fine with me. I was not aware of the issue with emacs-nox.