sachac / subed

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

adjust time of current subtitle end and next subtitle start? #54

Closed mooseyboots closed 7 months ago

mooseyboots commented 2 years ago

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.

sachac commented 2 years ago

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: @.***>

mooseyboots commented 2 years ago

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)

sachac commented 2 years ago

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 commented 2 years ago

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?

muello commented 2 years ago

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)))
sachac commented 2 years ago

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...)

muello commented 2 years ago

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.

mooseyboots commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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.

sachac commented 2 years ago

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 runsubed-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 runsubed-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 runsubed-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 runsubed-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: @.***>

mooseyboots commented 2 years ago

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. :)

sachac commented 2 years ago

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: @.***>

mooseyboots commented 2 years ago

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 commented 2 years ago

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.

sachac commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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...

mooseyboots commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

sachac commented 2 years ago

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: @.***>

ceed0 commented 1 year ago

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.

sachac commented 1 year ago

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?

ceed0 commented 1 year ago

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?

sachac commented 1 year ago

@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:

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!

ceed0 commented 1 year ago

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?

sachac commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

If the adjusting works for you and it doesn't break @mooseyboots' workflow (maybe @mooseyboots would set subed-enforce-time-boundaries to 'error, or we can come up with a different configurable behaviour?), I could look into adding a subed-subtitle-minimum-msecs that could be added to things we keep in mind when adjusting and when validating.

ceed0 commented 1 year ago

set-start-with-boundaries

Yes, it works, thank you. Though I think it shouldn't change other subtitles silently, but print a message in that case, something like "Additional subtitles changed: number(s)." so that users would be informed and could undo the change if it's undesirable.

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

I tried to make a function that would seek to the start of a subtitle and start playing the video. Binded to some easily accessible key, like f5, and when syncing and looping are disabled, it should work well with subed-copy-player-pos-to-x-time commands, but the delay really slows it down.

Other strategy would be to use subed-increase/decrease-start/stop-time with syncing and looping, but then there is the problem with adjusting overlapping timings.

Both strategies could work, but feel a little incomplete, though I'm still figuring the workflow out, maybe I'm missing something.

sachac commented 1 year ago

Is that something like subed-mpv-jump-to-current-subtitle? Let me see if I understand things: you want to be able to easily replay the current subtitle and then press a shortcut to copy the player pos to either the stop time or the start time? Are you also typing in text while this is happening?

(Yup, the message is a good idea. I'm still unsnarling the changes I made, since apparently they affect the trimming tests quite a bit and I need to think about what the right behaviour should be...)

ceed0 commented 1 year ago

Is that something like subed-mpv-jump-to-current-subtitle?

Well, it's a part of the function, here it is

(defun +subed-mpv-jump-and-play ()
      (interactive)
      (subed-mpv-jump-to-current-subtitle)
      (subed-mpv-unpause))

This function is a manual approach to both syncing and looping, and works better with subed-copy-player-pos-to-x-time (which you can't really use with syncing/looping).

Ideally the workflow looks like this: I'm transcribing a video, use +subed-mpv-jump-and-play to replay the subtitle, set stop time with subed-copy-player-pos-to-stop-time, move to the next part.

I guess I could "fix" the function by disabling the subed-unpause-after-typing-delay, but I still find it very useful.

Edit: Heh, forgot to say - I want to use subed-mpv-unpause with subed-mpv-jump-to-current-subtitle because I sometimes pause the video, simple as that.

sachac commented 1 year ago

Trying to figure out what would make sense as the default behaviour. Maybe I should make it default to not caring about invalid durations or overlaps (subed-enforce-time-boundaries nil), and then people can enable other strategies if they want, so there's less hard-to-track-down magic for people who might be new to Emacs? Mrph.

ceed0 commented 1 year ago

Maybe I should make it default to not caring about invalid durations or overlaps (subed-enforce-time-boundaries nil)

Wouldn't that just add more fiddling for the end-user? Is there a case when a user would want overlaps and things like that in their subtitles?

Also, those options can be found in customize-group, they aren't that hard to find. I think people who will try to turn emacs into a subtitle editing software are already far too gone have at least some familiarity with emacs configuration.

Which made me remember - sync and loop are harder to track, because hooks from readme won't work. I guess that's because derived modes like subed-srt-mode overwrite those hooks from the parent subed-mode.

sachac commented 1 year ago

Merged the branch into main. I also changed the way subed-mode works so that it doesn't add functions to subed-mode-hook locally, which could be causing other add-hooks from your config to not get applied. It looks like (add-hook 'subed-mode-hook (lambda () (message "subed-mode-hook called"))) shows the message properly now, so the subed-mode-hook is getting called. Updated the README with the new suggested configuration, since I removed the code that sets up the default behavior. Does that work for you?

whatacold commented 7 months ago

Merged the branch into main.

Hi @sachac ,

Seems like this issue has been resolved, and what are the dedicated commands for this operation? I'm sorry that I failed to find it in this thread.

And it would be nice if README would get updated, I looked for this feature last weekend but failed to find it in README and ended up doing it by adjusting the start time of the second subtitles and then adjusting the end time of the first subtitles manually :(

Thanks.

whatacold commented 7 months ago

Ah, by inspecting the git history, it looks like the customizable variable subed-enforce-time-boundaries of the newest subed controls this behavior? If I guess it right, the default value adjust is just what I need, terrific!

Time to upgrade it to v1.2.7 from ... v1.0.24! Thanks for the great work.

sachac commented 7 months ago

Oh, I'm sorry this took you so much time! I'll set aside some time to add more notes on different workflows. I usually use subed-align to get rough timestamps and then subed-waveform-minor-mode to see the waveforms, which makes adjusting timestamps soooo much nicer.wav I can then M-left-click on the waveform to set the start time and copy to previous stop and M-right-click to set the stop time and copy to next start, or just left-click or right-click to set the times without copying. M-[, M-] also increase/reduce start time and M-{, M-} increase/reduce stop time. I just committed 1.2.8, so that'll probably be available in NonGNU ELPA tomorrow.

whatacold commented 7 months ago

Great, would love to learn some workflows!

Thanks.