search

sachac / subed

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

Trim overlaps #50

Closed mooseyboots closed 2 years ago

mooseyboots commented 2 years ago

i had a crack at implementing trimming overlapping subtitles and thought i would share what i came up with.

as per rndusr's recommendations, it includes

personally i only have experience with .srt subtitle files, so that is all i have used this with. i also don't know the codebase super well. but thought i would share my efforts nonetheless.

sachac commented 2 years ago

That's a neat idea! Any chance you can write tests for it? You can look in the tests subdirectory to see some examples, and type "make test" in the main directory to run the tests. Let me know if you get stuck or need help figuring things out! :)

Sacha

On Tue., Dec. 21, 2021, 13:05 mooseyboots, @.***> wrote:

i had a crack at implementing trimming overlapping subtitles and thought i would share what i came up with.

as per rndusr's recommendations, it includes

  • customize option to ask to trim on load
  • customize option to always trim on save
  • custom options and prefix args to trim by:
    • any number of milliseconds
    • the value of subed-subtitle-spacing, or
    • 1 millisecond
  • customize option to either trim end of current or start of next sub.

personally i only have experience with .srt subtitle files, so that is all i have used this with. i also don't know the codebase super well. but thought i would share my efforts nonetheless.

You can view, comment on, or merge this pull request online at:

https://github.com/sachac/subed/pull/50 Commit Summary

File Changes

(3 files https://github.com/sachac/subed/pull/50/files)

Patch Links:

— Reply to this email directly, view it on GitHub https://github.com/sachac/subed/pull/50, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACD7ES7FGL4YEB4OJ5TV23USC6XPANCNFSM5KQYJ2RQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

mooseyboots commented 2 years ago

hey s, sure that's a good idea. i'm a silly noob with testing but am slowly getting the hang of it on another project. i'll just take a while to nut it out.

reference: #6

sachac commented 2 years ago

Sure! I'll add it to my todo list too in case I get to it first. It's a good learning opportunity. :) Thanks again for sharing this!

On Sat., Dec. 25, 2021, 06:52 mooseyboots, @.***> wrote:

hey s, sure that's a good idea. i'm a silly noob with testing but am slowly getting the hang of it on another project. i'll just take a while to nut it out.

— Reply to this email directly, view it on GitHub https://github.com/sachac/subed/pull/50#issuecomment-1001008154, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACD7ETUWRTL7YDKOERNTVTUSWWA7ANCNFSM5KQYJ2RQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

sachac commented 2 years ago

Worked on some tests tonight and rewrote it to use the functions for adjusting time stop/start. Haven't pushed my changes yet, as I want to take another look when I'm less sleepy. :) I'm a little unsure about having all the options for controlling it. Are you okay if I simplify it to just use either msecs as a numeric argument or subed-subtitle-spacing, instead of having another way to control whether subed-subtitle-spacing is used? People can set subed-subtitle-spacing to 1 if they want stop times and start times to be at least 1 msec apart, or have a function that calls it with an arg. (I'll probably change my config to that, actually.) Would that still work for your use case?

mooseyboots commented 2 years ago

hey, that's great to hear! it would have taken me days and days of work. i'm also happy if you modify what i proposed, i'm sure you'll improve it. in my use case, i just tend to use 1ms. i really just need to blast away all overlaps before i start editing, so the gap isn't that crucial for me. i kind of wrote what i wanted but then expanded it to fit rndusr's suggestions. it was also a way for me to start learning what (interactive) is capable of and how to tame it. it's great for me that you took a look at it, as i learn stuff when more experienced people edit my hacks.

sachac commented 2 years ago

Okay, I think I've mostly gotten things sorted out with tests and stuff at https://github.com/sachac/subed/commit/a97a9e565ea3ceaacc69db6aef4abe0602fac965 . I promise I didn't rename and fiddle with things just to be ornery. =) I wanted functions to be mostly discoverable with M-x, so I gave them (mostly) the subed-trim-overlap prefix. Customization options tend to be grouped alphabetically too, so I renamed them to be close together. I wanted to reuse the subed-adjust-subtitle-time-stop and -start functions because they seemed to have a fair bit of thought put into the logic. And then I ended up also tweaking how the sanitizing and validation functions were called so that we could have generic validation functions without having to add them to each of the specific validation functions...

Would you consider trying out this version and seeing if it works for you? It seems to pass the tests and it works interactively for me.

(Sorry for the somewhat heavy touch editing the code; I'm learning my way around maintaining subed too!)

mooseyboots commented 2 years ago

hey, s, no probs, happy to defer to your hands.