search

sachac / subed

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

Waveform #65

Closed mbork closed 1 year ago

sachac commented 1 year ago

Yay, I've gotten it to work on my computer, and it'd be good to see if my tweaks work on yours as well! What do you think about the changes I made in https://github.com/sachac/subed/tree/waveform ? Summary:

mbork commented 1 year ago

On 2023-05-01, at 00:39, Sacha Chua @.***> wrote:

Yay, I've gotten it to work on my computer, and it'd be good to see if my tweaks work on yours as well! What do you think about the changes I made in https://github.com/sachac/subed/tree/waveform ? Summary:

  • Added options subed-waveform-preview-msecs-before and subed-waveform-preview-msecs-after so we can control that independently of looping
  • Changed it to a minor mode like sachac/subed-waveform so that we can take advantage of putting the enabling/disabling code in one place, having mode maps / messages / documentation / hooks, etc.
  • Copied over some of the features from sachac/subed-waveform (dragging to change times, sampling from the middle, splitting)

Wow, that sounds cool! I'll review these. (May 1 and 3 are holidays here in Poland, so I'm not sure if I manage to do it before Thursday or so, but I will do it as soon as I can.)

Thanks,

-- Marcin Borkowski http://mbork.pl

sachac commented 1 year ago

No rush! :) Take your time.

On Mon, May 1, 2023, 00:54 mbork @.***> wrote:

On 2023-05-01, at 00:39, Sacha Chua @.***> wrote:

Yay, I've gotten it to work on my computer, and it'd be good to see if my tweaks work on yours as well! What do you think about the changes I made in https://github.com/sachac/subed/tree/waveform ? Summary:

  • Added options subed-waveform-preview-msecs-before and subed-waveform-preview-msecs-after so we can control that independently of looping
  • Changed it to a minor mode like sachac/subed-waveform so that we can take advantage of putting the enabling/disabling code in one place, having mode maps / messages / documentation / hooks, etc.
  • Copied over some of the features from sachac/subed-waveform (dragging to change times, sampling from the middle, splitting)

Wow, that sounds cool! I'll review these. (May 1 and 3 are holidays here in Poland, so I'm not sure if I manage to do it before Thursday or so, but I will do it as soon as I can.)

Thanks,

-- Marcin Borkowski http://mbork.pl

— Reply to this email directly, view it on GitHub https://github.com/sachac/subed/pull/65#issuecomment-1529345065, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACD7ERH4XEUBNAYD7WQNFDXD462FANCNFSM6AAAAAAXOVUQBI . You are receiving this because you commented.Message ID: @.***>

mbork commented 1 year ago

On 2023-05-01, at 00:39, Sacha Chua @.***> wrote:

Yay, I've gotten it to work on my computer, and it'd be good to see if my tweaks work on yours as well! What do you think about the changes I made in https://github.com/sachac/subed/tree/waveform ? Summary:

  • Added options subed-waveform-preview-msecs-before and subed-waveform-preview-msecs-after so we can control that independently of looping
  • Changed it to a minor mode like sachac/subed-waveform so that we can take advantage of putting the enabling/disabling code in one place, having mode maps / messages / documentation / hooks, etc.
  • Copied over some of the features from sachac/subed-waveform (dragging to change times, sampling from the middle, splitting)

Hi,

I managed to look at the code. I have a few remarks/questions.

  1. Thanks for fixing a bug I apparently made (the overlay wasn't properly initialized).

  2. I think you have tab-width' set to 2 while I have the default 8. This makes the code weirdly indented on my machine. Not sure what should/can be done about it (apart fromuntabify'ing everything).

  3. I'm a bit on the fence about `subed-waveform-preview-msecs-(before|after)', since using the looping variables ensures that the bar showing the current position (a) is always visible and (b) sweeps the entire waveform. But I'm not against these new variables, it's just that I used the looping ones consciously. I guess I'll just set them to the same value locally.

  4. I /love/ your additional mouse-related commands, though I am not sure why you chose dragging for adjustment (or how exactly it works). I will try to analyze that in a few days.

  5. I don't know why (yet?), but somehow `subed-waveform-minor-mode-map' is not activated.

Thanks for making my code better! I have a few more ideas, and I hope we'll be able to merge them, too, soon!

Best,

-- Marcin Borkowski http://mbork.pl

sachac commented 1 year ago

Looking forward to checking it out this weekend. Thank you so much!

On Thu, May 4, 2023, 14:45 mbork @.***> wrote:

On 2023-05-01, at 00:39, Sacha Chua @.***> wrote:

Yay, I've gotten it to work on my computer, and it'd be good to see if my tweaks work on yours as well! What do you think about the changes I made in https://github.com/sachac/subed/tree/waveform ? Summary:

  • Added options subed-waveform-preview-msecs-before and subed-waveform-preview-msecs-after so we can control that independently of looping
  • Changed it to a minor mode like sachac/subed-waveform so that we can take advantage of putting the enabling/disabling code in one place, having mode maps / messages / documentation / hooks, etc.
  • Copied over some of the features from sachac/subed-waveform (dragging to change times, sampling from the middle, splitting)

Hi,

I managed to look at the code. I have a few remarks/questions.

  1. Thanks for fixing a bug I apparently made (the overlay wasn't properly initialized).

  2. I think you have tab-width' set to 2 while I have the default 8. This makes the code weirdly indented on my machine. Not sure what should/can be done about it (apart fromuntabify'ing everything).

  3. I'm a bit on the fence about `subed-waveform-preview-msecs-(before|after)', since using the looping variables ensures that the bar showing the current position (a) is always visible and (b) sweeps the entire waveform. But I'm not against these new variables, it's just that I used the looping ones consciously. I guess I'll just set them to the same value locally.

  4. I /love/ your additional mouse-related commands, though I am not sure why you chose dragging for adjustment (or how exactly it works). I will try to analyze that in a few days.

  5. I don't know why (yet?), but somehow `subed-waveform-minor-mode-map' is not activated.

Thanks for making my code better! I have a few more ideas, and I hope we'll be able to merge them, too, soon!

Best,

-- Marcin Borkowski http://mbork.pl

— Reply to this email directly, view it on GitHub https://github.com/sachac/subed/pull/65#issuecomment-1535242857, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACD7EVHA45V62RXFG2KR7TXEP2NVANCNFSM6AAAAAAXOVUQBI . You are receiving this because you commented.Message ID: @.***>

sachac commented 1 year ago

mbork @.***> writes:

Hello, Marcin!

  1. I think you have tab-width' set to 2 while I have the default 8. This makes the code weirdly indented on my machine. Not sure what should/can be done about it (apart fromuntabify'ing everything).

Okay, I've untabified everything and added a .dir-locals.el that should set indent-tabs-mode to nil. Good point!

  1. I'm a bit on the fence about `subed-waveform-preview-msecs-(before|after)', since using the looping variables ensures that the bar showing the current position (a) is always visible and (b) sweeps the entire waveform. But I'm not against these new variables, it's just that I used the looping ones consciously. I guess I'll just set them to the same value locally.

Hmmm... The reason I separated them was because of how I was using subed-waveform with subed-record. I split my talk up into sentences, used subed-record to capture timestamps during my recording, and then used subed-waveform to remove the clicks from the keyboard. I used to need my dragging thing in order to expand the subtitle earlier or later with one hand on the keyboard and one hand on the mouse, but I like your idea of showing a few seconds on either side of it. I still want the loop to give me just the actual subtitle, though, so I can easily confirm it by ear without having to check where the cursor is (and trust that it's properly in sync). So maybe it's okay that it's configurable?

  1. I /love/ your additional mouse-related commands, though I am not sure why you chose dragging for adjustment (or how exactly it works). I will try to analyze that in a few days.

I had initially written my subed-waveform without the preview seconds before/after, so if I needed extra time before the subtitle, it was a little awkward to decrease the start time since there wasn't anywhere to click on. In particular, using aeneas for forced alignment of subtitles (given text + audio, figure out timestamps) resulted in timestamps that needed a bit of tweaking. It was pretty handy to be able to use the length of my drag to extend it so and so much: a short drag if it was just a little cut off, a long drag if it needed more time. (I think I might've even used my tablet stylus for some of that.) Even with the preview seconds, I think there might still be situations where dragging to extend the subtitle feels easier to me than clicking multiple times.

  1. I don't know why (yet?), but somehow `subed-waveform-minor-mode-map' is not activated.

That's because I forgot to :keymap it. =) Does it work for you now? Seems to work for me.

Sacha

mbork commented 1 year ago

Hi Sacha,

and sorry for my silence. I think it's ok for me to merge – I've been using this for several months now and it works for me. Thanks!

Would you mind me blogging about it?

Best, mbork

On 2023-05-07, at 02:58, Sacha Chua @.***> wrote:

mbork @.***> writes:

Hello, Marcin!

  1. I think you have tab-width' set to 2 while I have the default 8. This makes the code weirdly indented on my machine. Not sure what should/can be done about it (apart fromuntabify'ing everything).

Okay, I've untabified everything and added a .dir-locals.el that should set indent-tabs-mode to nil. Good point!

  1. I'm a bit on the fence about `subed-waveform-preview-msecs-(before|after)', since using the looping variables ensures that the bar showing the current position (a) is always visible and (b) sweeps the entire waveform. But I'm not against these new variables, it's just that I used the looping ones consciously. I guess I'll just set them to the same value locally.

Hmmm... The reason I separated them was because of how I was using subed-waveform with subed-record. I split my talk up into sentences, used subed-record to capture timestamps during my recording, and then used subed-waveform to remove the clicks from the keyboard. I used to need my dragging thing in order to expand the subtitle earlier or later with one hand on the keyboard and one hand on the mouse, but I like your idea of showing a few seconds on either side of it. I still want the loop to give me just the actual subtitle, though, so I can easily confirm it by ear without having to check where the cursor is (and trust that it's properly in sync). So maybe it's okay that it's configurable?

  1. I /love/ your additional mouse-related commands, though I am not sure why you chose dragging for adjustment (or how exactly it works). I will try to analyze that in a few days.

I had initially written my subed-waveform without the preview seconds before/after, so if I needed extra time before the subtitle, it was a little awkward to decrease the start time since there wasn't anywhere to click on. In particular, using aeneas for forced alignment of subtitles (given text + audio, figure out timestamps) resulted in timestamps that needed a bit of tweaking. It was pretty handy to be able to use the length of my drag to extend it so and so much: a short drag if it was just a little cut off, a long drag if it needed more time. (I think I might've even used my tablet stylus for some of that.) Even with the preview seconds, I think there might still be situations where dragging to extend the subtitle feels easier to me than clicking multiple times.

  1. I don't know why (yet?), but somehow `subed-waveform-minor-mode-map' is not activated.

That's because I forgot to :keymap it. =) Does it work for you now? Seems to work for me.

Sacha

-- Marcin Borkowski http://mbork.pl

sachac commented 1 year ago

Yay! Always glad to get confirmation! Merged.

I'd be delighted if you shared your experiences and tips in a blog post. :) The more, the merrier!

Sacha

On Sat, Jun 17, 2023, 15:27 mbork @.***> wrote:

Hi Sacha,

and sorry for my silence. I think it's ok for me to merge – I've been using this for several months now and it works for me. Thanks!

Would you mind me blogging about it?

Best, mbork

On 2023-05-07, at 02:58, Sacha Chua @.***> wrote:

mbork @.***> writes:

Hello, Marcin!

  1. I think you have tab-width' set to 2 while I have the default 8. This makes the code weirdly indented on my machine. Not sure what should/can be done about it (apart fromuntabify'ing everything).

Okay, I've untabified everything and added a .dir-locals.el that should set indent-tabs-mode to nil. Good point!

  1. I'm a bit on the fence about `subed-waveform-preview-msecs-(before|after)', since using the looping variables ensures that the bar showing the current position (a) is always visible and (b) sweeps the entire waveform. But I'm not against these new variables, it's just that I used the looping ones consciously. I guess I'll just set them to the same value locally.

Hmmm... The reason I separated them was because of how I was using subed-waveform with subed-record. I split my talk up into sentences, used subed-record to capture timestamps during my recording, and then used subed-waveform to remove the clicks from the keyboard. I used to need my dragging thing in order to expand the subtitle earlier or later with one hand on the keyboard and one hand on the mouse, but I like your idea of showing a few seconds on either side of it. I still want the loop to give me just the actual subtitle, though, so I can easily confirm it by ear without having to check where the cursor is (and trust that it's properly in sync). So maybe it's okay that it's configurable?

  1. I /love/ your additional mouse-related commands, though I am not sure why you chose dragging for adjustment (or how exactly it works). I will try to analyze that in a few days.

I had initially written my subed-waveform without the preview seconds before/after, so if I needed extra time before the subtitle, it was a little awkward to decrease the start time since there wasn't anywhere to click on. In particular, using aeneas for forced alignment of subtitles (given text + audio, figure out timestamps) resulted in timestamps that needed a bit of tweaking. It was pretty handy to be able to use the length of my drag to extend it so and so much: a short drag if it was just a little cut off, a long drag if it needed more time. (I think I might've even used my tablet stylus for some of that.) Even with the preview seconds, I think there might still be situations where dragging to extend the subtitle feels easier to me than clicking multiple times.

  1. I don't know why (yet?), but somehow `subed-waveform-minor-mode-map' is not activated.

That's because I forgot to :keymap it. =) Does it work for you now? Seems to work for me.

Sacha

-- Marcin Borkowski http://mbork.pl

— Reply to this email directly, view it on GitHub https://github.com/sachac/subed/pull/65#issuecomment-1595840613, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACD7EREEARK3LDBNEZDEXDXLYALFANCNFSM6AAAAAAXOVUQBI . You are receiving this because you commented.Message ID: @.***>