subed icon indicating copy to clipboard operation
subed copied to clipboard

adjust time of current subtitle end and next subtitle start?

Open mooseyboots opened this issue 3 years ago • 23 comments

is this actually possible in subed at the moment?

it's great to not allow individual overlaps, but often it means you can't adjust the end of a time stamp without first adjusting the start of the next, so you have to work out of order.

you'd want to do this without touching any subsequent subs also.

mooseyboots avatar Jan 13 '22 16:01 mooseyboots

Would subed-increase-stop-time and subed-decrease-stop-time work for you? subed-set-subtitle-time-stop doesn't adjust overlap, but maybe we can have a version of that command that does.

On Thu., Jan. 13, 2022, 11:17 mooseyboots, @.***> wrote:

is this actually possible in subed at the moment?

it's great to not allow individual overlaps, but often it means you can't adjust the end of a time stamp without first adjusting the start of the next, so you have to work out of order.

you'd want to do this without touching any subsequent subs also.

— Reply to this email directly, view it on GitHub https://github.com/sachac/subed/issues/54, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACD7ERWLGGIJRA5ZVB3DOLUV33IJANCNFSM5L4JD3DQ . 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: @.***>

sachac avatar Jan 13 '22 18:01 sachac

what i had in mind was doing it as the one command, to not have to e.g. first navigate to subtitle B to shrink its start time, and to then move back to subtitle A to expand its end time. but instead, adjust both together while on sub A, and do this while overlapping is still not allowed.

(but also just wanted to confirm this functionality isn't implemented, or hear if anyone else had this use case/desire/frustration)

mooseyboots avatar Jan 13 '22 18:01 mooseyboots

Hmm, yeah, adding that functionality as an option to those functions (or creating similar functions) so that we can adjust the next/previous subtitle's time would be great, and it's definitely not there yet! :) Want to take a crack at it, or want to describe your ideal keybindings and behavior for something like that?

On Thu., Jan. 13, 2022, 13:40 mooseyboots, @.***> wrote:

what i had in mind was doing it as the one command, to not have to e.g. first navigate to subtitle B to shrink its start time, and to then move back to subtitle A to expand its end time. but instead, adjust both together while on sub A, and do this while overlapping is still not allowed.

(but also just wanted to confirm this functionality isn't implemented)

— Reply to this email directly, view it on GitHub https://github.com/sachac/subed/issues/54#issuecomment-1012406867, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACD7EQAF5UI7XGPETJ2LCLUV4MB7ANCNFSM5L4JD3DQ . 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 avatar Jan 14 '22 01:01 sachac

Hmm... Maybe a customizable option that defaults to 'adjust (adjust next subtitle's start / previous subtitle's end if needed), and has other options like 'reduce (reduce the adjustment to avoid overlap), 'overlap (allow overlap), and 'ask (prompt for the action to take if there's an overlap)? I think it could make sense to make the new behaviour ('adjust) the default. Would that work?

sachac avatar Jan 14 '22 21:01 sachac

I've been working with these. Shouldn't be too hard to make them work for -next rather than -previous:

(defun subed-decrease-start-time-adjust-previous ()
    "Subtract ‘subed-milliseconds-adjust’ milliseconds from start time of the current subtitle and shorten the previous one by the same amount."
    (interactive)
    (save-excursion
      (subed-backward-subtitle-text)
      (subed-decrease-stop-time))
    (subed-decrease-start-time))

  (defun subed-increase-start-time-adjust-previous ()
    "Add ‘subed-milliseconds-adjust’ milliseconds to start time of the current subtitle and lengthen the previous one by the same amount."
    (interactive)
    (subed-increase-start-time)
    (save-excursion
      (subed-backward-subtitle-text)
      (subed-increase-stop-time)))

muello avatar Jan 23 '22 23:01 muello

That's a good idea. Putting it in a separate function instead of relying on customizable variables to overload the existing function can make it easier for people to bind things to different keys and tweak the behaviour. I'll look into it when I get some focused time, maybe next weekend! (ah, parenting during a pandemic...)

sachac avatar Jan 24 '22 06:01 sachac

Ah, wow, parenting!

Another idea I had in this context was that it'd be nice to able to be able to see the CPS overlay not just for the sub at point but also for the ones immediately above and below. A look at the overlay code showed me that I probably can't achieve that with a simple hack, though.

muello avatar Jan 24 '22 11:01 muello

@muello thanks for sharing your hacks, i modified them to work for the next sub instead of previous. it's probably sufficient for my use case. cd also perhaps be added to subed if desired.

mooseyboots avatar Jan 24 '22 14:01 mooseyboots

hm, actually muello's functions need a little tweaking for me, they don't always work correctly with the CPS overlay, or with keeping the position of the player correctly, sometimes it reverts to that of the second subtitle being adjusted. and sometimes the CPS of the adjusted sub appears as the overlay for the current sub.

i think the issue is that when the save excursion part of the function comes second, as with subed-increase-start-time-adjust-previous, subed doesn't fully revert back to the current subtitle. i also tried using subed-save-excursion but its no help in this case either.

it seems like every second time it is called, it leaves mpv at the start of the previous, not the current, subtitle.

mooseyboots avatar Jan 25 '22 12:01 mooseyboots

looks like the trouble i was with the mpv position being moved was because subed-subtitle-time-adjusted-hook contains (subed--replay-adjusted-subtitle), which in turn runs (subed-mpv-jump msecs-start).

let-binding that hook to nil in any excursioning seemed to fix things up. tho perhaps there's a less hacky/messy way to achieve the same thing.

then we also update the CPS after everything's done.

i also test that the subtitles are adjacent, and only adjust the prev/next sub if so.

(defun subed-decrease-start-time-adjust-previous ()
  "Subtract `subed-milliseconds-adjust' milliseconds from start
  time of the current subtitle and shorten the previous one by the
  same amount.

  If subtitles are further apart than `subed-milliseconds-adjust',
  just run `subed-decrease-start-time'."
  (interactive)
  (let (prev-sub-stop)
    (subed-save-excursion
     (let ((subed-subtitle-time-adjusted-hook nil)) ;dont move mpv
       (subed-backward-subtitle-text)
       (setq prev-sub-stop (subed-subtitle-msecs-stop))))
    (if (< (+ prev-sub-stop subed-milliseconds-adjust)
           (subed-subtitle-msecs-start))
        (subed-decrease-start-time)
      (subed-save-excursion
       (let ((subed-subtitle-time-adjusted-hook nil)) ;dont move mpv
         (subed-backward-subtitle-text)
         (subed-decrease-stop-time)))
      (subed-decrease-start-time)
      (subed--update-cps-overlay))))

(defun subed-increase-start-time-adjust-previous ()
  "Add `subed-milliseconds-adjust' milliseconds to start time
  of the current subtitle and lengthen the previous one by the same
  amount.

  If subtitles are further apart than `subed-milliseconds-adjust',
  just run `subed-increase-stop-time'."
  (interactive)
  (let (prev-sub-stop)
    (subed-save-excursion
     (let ((subed-subtitle-time-adjusted-hook nil)) ;dont move mpv
       (subed-backward-subtitle-text)
       (setq prev-sub-stop (subed-subtitle-msecs-stop))))
    (if (< (+ prev-sub-stop subed-milliseconds-adjust)
           (subed-subtitle-msecs-start))
        (subed-increase-start-time)
      (subed-increase-start-time)
      (subed-save-excursion
       (let ((subed-subtitle-time-adjusted-hook nil)) ;dont move mpv
         (subed-backward-subtitle-text)
         (subed-increase-stop-time)))
      (subed--update-cps-overlay))))

(defun subed-decrease-stop-time-adjust-next ()
  "Subtract `subed-milliseconds-adjust' milliseconds from stop
  time of the current subtitle and shorten the next one by the same
  amount.

  If subtitles are further apart than `subed-milliseconds-adjust',
  just run `subed-decrease-start-time'."
  (interactive)
  (let (next-sub-start)
    (subed-save-excursion
     (let ((subed-subtitle-time-adjusted-hook nil)) ;dont move mpv
       (subed-forward-subtitle-text)
       (setq next-sub-start (subed-subtitle-msecs-start))))
    (if (< (subed-subtitle-msecs-stop)
           (- next-sub-start subed-milliseconds-adjust))
        (subed-decrease-stop-time)
      (subed-decrease-stop-time)
      (subed-save-excursion
       (let ((subed-subtitle-time-adjusted-hook nil)) ;dont move mpv
         (subed-forward-subtitle-text)
         (subed-decrease-start-time)))
      ;; (subed--sync-player-to-point)
      (subed--update-cps-overlay))))

(defun subed-increase-stop-time-adjust-next ()
  "Add `subed-milliseconds-adjust' milliseconds to stop time of
  the current subtitle and lengthen the next one by the same
  amount. 

  If subtitles are further apart than `subed-milliseconds-adjust',
  just run `subed-increase-stop-time'."
  (interactive)
  (let (next-sub-start)
    (subed-save-excursion
     (let ((subed-subtitle-time-adjusted-hook nil)) ;dont move mpv
       (subed-forward-subtitle-text)
       (setq next-sub-start (subed-subtitle-msecs-start))))
    (if (< (subed-subtitle-msecs-stop)
           (- next-sub-start subed-milliseconds-adjust))
        (subed-increase-stop-time)
      (subed-save-excursion
       (let ((subed-subtitle-time-adjusted-hook nil)) ;dont move mpv
         (subed-forward-subtitle-text)
         (subed-increase-start-time)))
      (subed-increase-stop-time)
      (subed--update-cps-overlay))))

one little remaining nag is that if the subs are closer together than subed-milliseconds-adjust, the gap will first increase to it before both subs are adjusted.

cd also be refactored to be less of a readability nightmare, but works for now.

i'm also hacking on subed-copy-player-pos-to-stop-time-adjust-next and subed-copy-player-pos-to-start-time-adjust-previous, to achieve similar adjustment functionality.

& yeah, CPS on prev, current, and next sub would be a real luxury for this kind of editing. even as it is, it makes working through things much easier, at least for me.

mooseyboots avatar Jan 26 '22 15:01 mooseyboots

Hmm... In the derived-mode branch, I have a subed-batch-edit macro that tries to make it easier to suppress sync/replay and update overlays afterwards. Would something like that be useful? subed-trim-overlaps tries to use it. Haven't tested it thoroughly yet, improvements welcome.

On Wed, Jan 26, 2022 at 10:49 AM mooseyboots @.***> wrote:

looks like the trouble i was with the mpv position being moved was because subed-subtitle-time-adjusted-hook contains (subed--replay-adjusted-subtitle), which in turn runs (subed-mpv-jump msecs-start).

let-binding that hook to nil in any excursioning seemed to fix things up. then we also update the CPS after everything's done.

i also test that the subtitles are adjacent, and only adjust the prev/next sub if so.

(defun subed-decrease-start-time-adjust-previous () "Subtract `subed-milliseconds-adjust' milliseconds from start time of the current subtitle and shorten the previous one by the same amount.

If subtitles are further apart than subed-milliseconds-adjust', just run subed-decrease-start-time'." (interactive) (let (prev-sub-stop) (subed-save-excursion (let ((subed-subtitle-time-adjusted-hook nil)) ;dont move mpv (subed-backward-subtitle-text) (setq prev-sub-stop (subed-subtitle-msecs-stop)))) (if (< (+ prev-sub-stop subed-milliseconds-adjust) (subed-subtitle-msecs-start)) (subed-decrease-start-time) (subed-save-excursion (let ((subed-subtitle-time-adjusted-hook nil)) ;dont move mpv (subed-backward-subtitle-text) (subed-decrease-stop-time))) (subed-decrease-start-time) (subed--update-cps-overlay))))

(defun subed-increase-start-time-adjust-previous () "Add `subed-milliseconds-adjust' milliseconds to start time of the current subtitle and lengthen the previous one by the same amount.

If subtitles are further apart than subed-milliseconds-adjust', just run subed-increase-stop-time'." (interactive) (let (prev-sub-stop) (subed-save-excursion (let ((subed-subtitle-time-adjusted-hook nil)) ;dont move mpv (subed-backward-subtitle-text) (setq prev-sub-stop (subed-subtitle-msecs-stop)))) (if (< (+ prev-sub-stop subed-milliseconds-adjust) (subed-subtitle-msecs-start)) (subed-increase-start-time) (subed-increase-start-time) (subed-save-excursion (let ((subed-subtitle-time-adjusted-hook nil)) ;dont move mpv (subed-backward-subtitle-text) (subed-increase-stop-time))) (subed--update-cps-overlay))))

(defun subed-decrease-stop-time-adjust-next () "Subtract `subed-milliseconds-adjust' milliseconds from stop time of the current subtitle and shorten the next one by the same amount.

If subtitles are further apart than subed-milliseconds-adjust', just run subed-decrease-start-time'." (interactive) (let (next-sub-start) (subed-save-excursion (let ((subed-subtitle-time-adjusted-hook nil)) ;dont move mpv (subed-forward-subtitle-text) (setq next-sub-start (subed-subtitle-msecs-start)))) (if (< (subed-subtitle-msecs-stop) (- next-sub-start subed-milliseconds-adjust)) (subed-decrease-stop-time) (subed-decrease-stop-time) (subed-save-excursion (let ((subed-subtitle-time-adjusted-hook nil)) ;dont move mpv (subed-forward-subtitle-text) (subed-decrease-start-time))) ;; (subed--sync-player-to-point) (subed--update-cps-overlay))))

(defun subed-increase-stop-time-adjust-next () "Add `subed-milliseconds-adjust' milliseconds to stop time of the current subtitle and lengthen the next one by the same amount.

If subtitles are further apart than subed-milliseconds-adjust', just run subed-increase-stop-time'." (interactive) (let (next-sub-start) (subed-save-excursion (let ((subed-subtitle-time-adjusted-hook nil)) ;dont move mpv (subed-forward-subtitle-text) (setq next-sub-start (subed-subtitle-msecs-start)))) (if (< (subed-subtitle-msecs-stop) (- next-sub-start subed-milliseconds-adjust)) (subed-increase-stop-time) (subed-save-excursion (let ((subed-subtitle-time-adjusted-hook nil)) ;dont move mpv (subed-forward-subtitle-text) (subed-increase-start-time))) (subed-increase-stop-time) (subed--update-cps-overlay))))

one little remaining nag is that if the subs are closer together than subed-milliseconds-adjust, the gap will first increase to it before both subs are adjusted.

i'm also hacking on subed-copy-player-pos-to-stop-time-adjust-next and subed-copy-player-pos-to-start-time-adjust-previous.

— Reply to this email directly, view it on GitHub https://github.com/sachac/subed/issues/54#issuecomment-1022330652, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACD7EVR74LR5FGF5GO4HWLUYAJXJANCNFSM5L4JD3DQ . 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 avatar Jan 28 '22 01:01 sachac

thanks sacha, i didn't know about that macro, i can take a look next time i'm subbing.

sorry for my amateur code, i don't have lots of time to improve as i'm often actually subtitling when i'm tinkering. :)

mooseyboots avatar Jan 28 '22 13:01 mooseyboots

Didn't get to work on it this weekend, sorry. The more I think about it, the more it seems that the basic adjustment functions should call something like a subed-maybe-fix-overlap-with-previous and -next, which should take a strategy like trim, adjust, allow, or prompt. Maybe something like that...

What are people's workflows like? Lately I've been using either the srv2 from YouTube (when I'm editing automatic captions - https://sachachua.com/dotemacs#word-level) or https://github.com/readbeyond/aeneas (when I'm starting from text and getting timings), so I don't usually need to adjust the subtitle times as much. When I used https://github.com/sachac/subed-record to record segments and compile a final audio file, I used https://github.com/sachac/subed-waveform to help tweak the timing. Would stuff like that also be useful for you?

On Fri., Jan. 28, 2022, 08:21 mooseyboots, @.***> wrote:

thanks sacha, i didn't know about that macro, i can take a look next time i'm subbing.

sorry for my amateur code, i don't have lots of time to improve as i'm often actually subtitling when i'm tinkering. :)

— Reply to this email directly, view it on GitHub https://github.com/sachac/subed/issues/54#issuecomment-1024220481, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACD7EUQSHI5IGLO2L7PDPTUYKJ5VANCNFSM5L4JD3DQ . 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 avatar Jan 31 '22 05:01 sachac

for my part, i autogenerate srt files with youtube or an offline script using the vosk api: https://github.com/alphacep/vosk-api. then i translate the transcriptions while watching the video.

i still find that i want to adjust timestamps from the auto-generated ones, and quite a lot, either to make the splitting more related to the language, or to introduce delays to make the subs more readable.

during my last job, i made heavy use of these functions, and once working smoothly they were really helpful. i also shared them w a colleague. maybe because i'm translating i have more need for them, as the translated language needs (or can tolerate) more adjustment relative to the audio track?

i'm also not super savvy about other workflows, i didn't know about the things you mention. thanks for sharing.

re adjustments, i found my preference was to not adjust prev/next sub if further away than subed-subtitle-spacing, and if = or < than it, to just leave the current gap as it was, by let-binding it to 0 like so:

    (if (< (+ prev-sub-stop subed-milliseconds-adjust)
           (subed-subtitle-msecs-start))
        (subed-decrease-start-time)
      (let ((subed-subtitle-spacing 0)) ; maintain current spacing
        (subed-save-excursion
         (let ((subed-subtitle-time-adjusted-hook nil)) ;dont move mpv
           (subed-backward-subtitle-text)
           (subed-decrease-stop-time)))
        (subed-decrease-start-time)
        (subed--update-cps-overlay)))))

that's just my 0.02 cents though!

mooseyboots avatar Jan 31 '22 10:01 mooseyboots

i refactored the functions i pasted above, and got subed-copy-player-pos-to-stop-time-adjust-next and subed-copy-player-pos-to-start-time-adjust-previous going also. let me know if you want to use them, but sounds like you have other fish to fry.

mooseyboots avatar Feb 01 '22 17:02 mooseyboots

That's terrific! Do you want to share them here or in a pull request in case other people find them useful too?

On Tue., Feb. 1, 2022, 12:25 mooseyboots, @.***> wrote:

i refactored the functions i pasted above, and got subed-copy-player-pos-to-stop-time-adjust-next and subed-copy-player-pos-to-start-time-adjust-previous going also. let me know if you want to use them, but sounds like you have other fish to fry.

— Reply to this email directly, view it on GitHub https://github.com/sachac/subed/issues/54#issuecomment-1027098309, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACD7ETEY5VG3T25KIWD5P3UZAJPFANCNFSM5L4JD3DQ . 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 avatar Feb 01 '22 17:02 sachac

Managed to squeeze in some time for drafting a subed-trim-overlap-at-start/stop, will try to add it to the other functions and write tests.

On Tue., Feb. 1, 2022, 12:33 Sacha Chua, @.***> wrote:

That's terrific! Do you want to share them here or in a pull request in case other people find them useful too?

On Tue., Feb. 1, 2022, 12:25 mooseyboots, @.***> wrote:

i refactored the functions i pasted above, and got subed-copy-player-pos-to-stop-time-adjust-next and subed-copy-player-pos-to-start-time-adjust-previous going also. let me know if you want to use them, but sounds like you have other fish to fry.

— Reply to this email directly, view it on GitHub https://github.com/sachac/subed/issues/54#issuecomment-1027098309, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACD7ETEY5VG3T25KIWD5P3UZAJPFANCNFSM5L4JD3DQ . 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 avatar Feb 02 '22 00:02 sachac

Added subed-trim-overlap-at-start and subed-trim-overlap-at-stop and tests at https://github.com/sachac/subed/tree/trim-overlaps-start-stop , need to think about how to use this in the other functions.

sachac avatar Feb 02 '22 03:02 sachac

Okay, https://github.com/sachac/subed/tree/trim-overlaps-start-stop now has subed-increase-start-time-and-adjust-previous , subed-decrease-start-time-and-adjust-previous , subed-increase-stop-time-and-adjust-next , and subed-decrease-stop-time-and-adjust-next , which I think miiight work. It would probably make sense to have something like the Org way of adjusting timestamps, being able to shift-up / shift-down to adjust the timestamp your point is currently on, so that we don't have to keep thinking in terms of which function to call...

sachac avatar Feb 02 '22 06:02 sachac

subed-enforce-time-boundaries of course! i was looking for this without knowing the name. makes the adjusting much simpler because you don't have to do it in a specific order to avoid overlap. also makes my complicated mess of adjustments redundant.

one thing in my cruddy version was that if there a gap btw the subs that was different to, but smaller than, subed-subtitle-spacing it wd be maintained rather than being set to `subed-subtitle-spacing', but that's nbd.

you know the codebase, and elisp, much better, of course. i'm just playing in the sandpit to learn the odd thing and get my work done.

mooseyboots avatar Feb 02 '22 10:02 mooseyboots

for bindings i had just added a ctrl to the existing bindings, so

(define-key subed-mode-map (kbd "C-M-]") 'subed-increase-start-time-adjust-previous)
(define-key subed-mode-map (kbd "C-M-[") 'subed-decrease-start-time-adjust-previous)
(define-key subed-mode-map (kbd "C-M-}") 'subed-increase-stop-time-adjust-next)
(define-key subed-mode-map (kbd "C-M-{") 'subed-decrease-stop-time-adjust-next)

but org-style wd be cool too.

mooseyboots avatar Feb 02 '22 10:02 mooseyboots

https://gist.github.com/mooseyboots/d9a183795e5704d3f517878703407184 was what i got up to.

mooseyboots avatar Feb 02 '22 10:02 mooseyboots

Oh great! Glad you figured out something that works for you. It sounds like we could add something to the README to describe different workflows / stages, to make it easier for people to figure out what knobs to twiddle when...

I'm learning lots about the codebase myself. :) All figuring it out together!

On Wed., Feb. 2, 2022, 05:08 mooseyboots, @.***> wrote:

subed-enforce-time-boundaries of course! i was looking for this without knowing the name. makes the adjusting much simpler because you don't have to do it in a specific order to avoid overlap. also makes my complicated mess of adjustments redundant.

one thing in my cruddy version was that if there a gap btw the subs that was different to, but smaller than, subed-subtitle-spacing it wd be maintained rather than being set to `subed-subtitle-spacing', but that's nbd.

you know the codebase, and elisp, much better, of course. i'm just playing in the sandpit to learn the odd thing and get my work done.

— Reply to this email directly, view it on GitHub https://github.com/sachac/subed/issues/54#issuecomment-1027775199, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACD7EW2CQMBCK5Z5W7Y23LUZD7C3ANCNFSM5L4JD3DQ . 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 avatar Feb 03 '22 05:02 sachac

I think all these commands should check for overlaps inside individual subtitles too, because now you can adjust start point to be further than end point. There should be a variable for the minimum subtitle length.

Edit: Actually, increase and decrease already work that way, and in my case negative-length subtitles came from subed-copy-player-pos-to-start-time.

ceed0 avatar Feb 14 '23 20:02 ceed0

Lovely! I wonder what would be good to do in that case. Maybe we should set the stop time to be at least the start time + default subtitle length if we're copying the player position?

sachac avatar Feb 15 '23 01:02 sachac

Yeah, that would be great, there's no much point to have a subtitle shorter than a second.

Edit: Also, will commands from here be added to the main branch?

ceed0 avatar Feb 15 '23 04:02 ceed0

@ceed0: Oh, there should be a subed-trim-overlaps command in main already. The commands got slightly renamed, so subed-trim-overlap-stop and subed-trim-overlap-next-start instead of subed-trim-overlap-at-stop. Was there a specific thing that I forgot to copy over?

I'm slowly starting to think through moving more of the behaviour into set-subtitle-time-start and set-subtitle-time-stop (branch: https://github.com/sachac/subed/compare/main...set-start-with-boundaries ). It sounds like there are a bunch of different strategies people might want to use, like:

  • A: reporting an error
  • B: silently clipping the time (ex: if you try to adjust the start time to something after the stop time, set it to the stop time)
  • C: silently adjusting the other time (ex: if you try to adjust the start time to something after the stop time, change the stop time to match the new start time)
  • D: accepting the time set without errors or adjustments

The implementation in main is a mix of B and D. I've started adding A to the set-start-with-boundaries branch, and probably the right thing to do is to make subed-enforce-time-boundaries a choice variable that allows you to pick the strategy you want. It might take me a little while (squeezing this into little bits of time here and there while the kiddo doesn't want my attention), so if anyone happens to want to swoop in with a patch, feel free!

sachac avatar Feb 16 '23 16:02 sachac

Was there a specific thing that I forgot to copy over?

I was thinking about subed-increase-start-time-and-adjust-previous, subed-decrease-start-time-and-adjust-previous etc. Without them, if I disable subed-enforce-time-boundaries I either have to use trim-overlap commands all the time, or leave it to on-save hooks, which is worse, because it's less flexible and can result in timings that I didn't intend, when I was moving them interactively.

And offtopic, but it there a way to use subed-mpv-unpause ignoring subed-unpause-after-typing-delay?

ceed0 avatar Feb 16 '23 23:02 ceed0

Ugh, I don't know what past-me was thinking, so it might take me a little while to get back into understanding why I picked the approach that made it into main. Maybe I was trying to keep things simple? It sounds like we kinda want strategy C where we tell subed what to do and it just quietly fixes other things for us (maintaining the spacing between that and the previous subtitle, adjusting the stop time, etc.). Miiiight make it adjust just the next/previous subtitle instead of rippling through the other ones, for now.

For subed-mpv-unpause: could you tell me more about what you want it to do when you interactively unpause it?

sachac avatar Feb 19 '23 01:02 sachac

@ceed0: Would you like to try the https://github.com/sachac/subed/tree/set-start-with-boundaries branch? If subed-enforce-time-boundaries is set to 'adjust (which it should be by default in that branch), things should adjust automatically to avoid overlaps and negative durations.

sachac avatar Feb 19 '23 03:02 sachac