-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[bug] Content of the waveform for the last subtitle is not properly centered #68
Comments
Thanks for a thorough investigation! I wonder what ffmpeg does to make the
image and how to adjust the math if it's not quite what we expect... It
might take me a while to get to this (limited computer time), so if you or
anyone happen to be able to fix it, that would be amazing.
…On Thu, Jun 20, 2024, 00:48 Rodrigo Morales ***@***.***> wrote:
How I reproduced the bug
I created a sample audio file /tmp/subed/a.mp3 using the command shown
below. The audio plays beeps at times 00:01, 00:03, 00:05 and 00:07 and
00:09. The screenshot shown below shows the waveform in Audacity version
2.4.2.
ffmpeg \
-v error \
-y \
-f lavfi -i "sine=frequency=1000:duration=10" \
-af "volume=0:enable='between(t,0,1)',\volume=0:enable='between(t,1.2,3)',\volume=0:enable='between(t,3.2,5)',\volume=0:enable='between(t,5.2,7)',\volume=0:enable='between(t,7.2,9)',\volume=0:enable='between(t,9.2,10)'" \
/tmp/subed/a.mp3
I inserted the content of the code block below into the file
/tmp/subed/a.srt.
1
00:00:00,000 --> 00:00:02,000
This is the subtitle no. 1
2
00:00:02,000 --> 00:00:04,000
This is the subtitle no. 2
3
00:00:04,000 --> 00:00:06,000
This is the subtitle no. 3
4
00:00:06,000 --> 00:00:08,000
This is the subtitle no. 4
5
00:00:08,000 --> 00:00:10,000
This is the subtitle no. 5
I inserted the content of the code block below to ~/.config/emacs/init.el.
(use-package subed
:hook
((subed-mode . subed-enable-sync-player-to-point)
(subed-mode . subed-enable-loop-over-current-subtitle)))
I launched Emacs by executing the command shown below.
emacs --no-splash /tmp/subed/a.srt
I pressed M-x subed-waveform-minor-mode RET.
I moved the point to the last subtitle (i.e. subtitle no. 5)
What happened?
The waveform image which was inserted into the buffer showed the waveform
for the beep further to the right than its actual position. See screenshot
below.
What should have happened?
The waveform for the beep should be shown at the middle of the waveform
image because the beep is approximately played at 00:09 and the subtitle
spans from 00:08 to 00:10.
—
Reply to this email directly, view it on GitHub
<#68>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACD7EQZO5ELGMTUIRDSCH3ZIJNKBAVCNFSM6AAAAABJTFJF4GVHI2DSMVQWIX3LMV43ASLTON2WKOZSGM3DGNJQGA2DGNQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
I investigated a little bit more on this issue. Here's a note to myself or others that might be interested to fix this in the future. The problem is that the parameter I found out this by adding some
The following code block contains the contents of the buffer
Note that in the information shown above |
Thinking out loud: we could use ffprobe to get the actual duration of the file, and then we can adjust the stop-ms to consider that duration, possibly adjusting the width of the image as well. We'll probably want to cache that duration so that we're not calculating it all the time, so that would be a good buffer-local variable. |
Since I have the appropriate ffprobe command at hand, here it is:
ffprobe -v error -show_entries format=duration -of default=noprint_wrappers=1:nokey=1 video.mkv
That should print the duration in seconds as a float.
|
* subed/subed-waveform.el (subed-waveform-ffprobe-executable): New. (subed-waveform-file-duration-ms-cache): New. (subed-waveform-file-duration-ms): New function for calculating and caching the duration. (subed-waveform-clear-file-duration-ms-cache): New. (subed-mpv): Add advice around subed-mpv-play-from-file for now; ideally change this to a hook later on. (subed-waveform--image-parameters): Move to a separate function for easier testing. (subed-waveform--make-overlay): Do the calculations in subed-waveform--image-parameters. (subed-waveform--update-bars): Use the actual stop time if needed. Thanks to rodrigomorales1 for the detailed bug report at #68 !
@rodrigomorales1 Would it be easy for you to test the https://github.com/sachac/subed/tree/waveform-long-cue branch, or would you like me to merge it into the main branch? |
@sachac I'll give that branch a try, write some tests and make a pull request on that branch. |
subed-waveform should now handle the case where the stop time + subed-waveform-preview-msecs-after might extend past the end of the file. * subed/subed-waveform.el (subed-waveform-ffprobe-executable): New. (subed-waveform-file-duration-ms-cache): New. (subed-waveform-ffprobe-duration-ms): New, calculates duration. (subed-waveform-file-duration-ms): New function for caching the duration. (subed-waveform-clear-file-duration-ms-cache): New. (subed-mpv): Add advice around subed-mpv-play-from-file for now; ideally change this to a hook later on. (subed-waveform--image-parameters): Move to a separate function for easier testing. (subed-waveform--make-overlay): Do the calculations in subed-waveform--image-parameters. (subed-waveform--update-bars): Use the actual stop time if needed. * tests/test-subed-waveform.el: New. * Set lexical-binding: t in tests/* files Thanks to rodrigomorales1 and rndusr for bug reports and pull requests! Related: - #68 - #75 - #74
How I reproduced the bug
I created a sample audio file
/tmp/subed/a.mp3
using the command shown below. The audio plays beeps at times 00:01, 00:03, 00:05 and 00:07 and 00:09. The screenshot shown below shows the waveform in Audacity version 2.4.2.I inserted the content of the code block below into the file
/tmp/subed/a.srt
.I cloned the
subed
git repository to make sure that I was using the latest changes in subed.I built GNU Emacs 29.3 from source. For convenience to reproduce the bug, I wrote the following command to start Emacs, visit the file
/tmp/subed/a.srt
and enablesubed-waveform-minor-mode
.When I executed the command, an Emacs window and a
mpv
window were opened and the waveform for the first subtitle (the subtitle the point was on) was shown. See screenshot below.I moved the point to the last subtitle (i.e.
This is subtitle no. 5
).What happened?
The waveform image which was inserted into the buffer showed the waveform for the beep further to the right than its actual position. See screenshot below.
The screenshot shown below shows the buffer after calling
subed-waveform-toggle-show-all
. Note that the waveform for the beep is centered for all subtitles except for the last one.What should have happened?
The waveform for the beep in the last subtitle should be shown at the middle of the image since the beep is approximately played at 00:09 and the subtitle spans from 00:08 to 00:10.
System information
The text was updated successfully, but these errors were encountered: