smacke / ffsubsync

Automagically synchronize subtitles with video.
MIT License
6.84k stars 281 forks source link

stops processing multiple files on pysubs2.exceptions.UnknownFPSError #176

Open milahu opened 1 year ago

milahu commented 1 year ago

multiple bugs here

expected:

$ ffs --version
ffsubsync 0.4.22

$ ffs --overwrite-input movie.mp4 -i *.srt *.sub
...
Traceback (most recent call last):
  File "/lib/python3.10/site-packages/pysubs2/microdvd.py", line 38, in from_file
    fps = float(text)
ValueError: could not convert string to float: 'Advertise your product or brand here|contact www.OpenSubtitles.org today\r'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
...
  File "/lib/python3.10/site-packages/pysubs2/microdvd.py", line 42, in from_file
    raise UnknownFPSError("Framerate was not specified and "
pysubs2.exceptions.UnknownFPSError: Framerate was not specified and cannot be read from the MicroDVD file.

problematic subtitle file

$ head -n3 "The.Unbearable.Lightness.Of.Being.1988.en.156226.sub"
{25}{175}Advertise your product or brand here|contact www.OpenSubtitles.org today
{496}{567}[Bell Tolling]
{568}{642}[Pigeons Cooing]
smacke commented 1 year ago

Hmm... for this command:

ffs --overwrite-input *.mp4 -i *.srt *.sub

I don't think ffsubsync is smart enough to figure out the correspondence between references and inputs if multiple references are specified like that. I think the best you can hope for with the current code is syncing multiple inputs against a single reference -- anything fancier requires a wrapper around ffsubsync at the moment.

Regarding the pysubs2 error, can you file an issue against that repo? Contributors there would probably be better equipped get a fix out.

milahu commented 1 year ago

the glob *.mp4 expands to a single file movie.mp4 (i fixed my first comment)

ValueError is just noise, the actual error is UnknownFPSError

let me focus on

expected:

  • show a warning, continue to process other subtitle files

when syncing multiple subs, one broken sub should not stop the whole process

smacke commented 1 year ago

@milahu I tried pushing a fix for this but haven't had a chance to properly test it -- would you be willing to try this out since you have an example where syncing should continue if something fails when trying to sync multiple?

To test, you would need to install from master, which you can do by running the following:

pip install git+https://github.com/smacke/ffsubsync@master
milahu commented 1 year ago

7fd1885b00ff68eceef2f557c334bebdd30f7ae5 (ffsubsync.nix) still stops at pysubs2.exceptions.UnknownFPSError

cat >bad.sub <<EOF
{2}{3}hello world
EOF

cat >good.sub <<EOF
{1}{1}25
{2}{3}hello world
EOF

# generate audio
espeak -w audio.wav "hello world"

# generate empty video, mux with audio
ffmpeg -t 3 -s 640x480 -f rawvideo -pix_fmt rgb24 -r 25 -i /dev/zero -i audio.wav -y video.mp4

md5sum good.sub
# 5964c0de6c837c24876fb4785023df06  good.sub

ffs --overwrite-input video.mp4 -i bad.sub good.sub

md5sum good.sub
# 5964c0de6c837c24876fb4785023df06  good.sub
# -> no change
smacke commented 1 year ago

Sorry for the very late response -- got very busy at work.

For the example you sent, it looks like we're only running it on one case -- what happens if we run on multiple case where, if run individually, the first fails but subsequent ones succeed?

milahu commented 1 year ago

it looks like we're only running it on one case

what? there are two input subs: bad.sub and good.sub

ffs --overwrite-input video.mp4 -i bad.sub good.sub

https://github.com/milahu/ffsubsync-stops-processing-multiple-files

smacke commented 1 year ago

Ah I forgot how my own tool works (i.e. forgot that --overwrite-input lets you pass multiple "to be synced" files). OK thanks for this; will try to take another look soon, hopefully before 3 more months go by :)

smacke commented 1 year ago

Hi @milahu I increased the broadness of the tolerable exceptions we catch in 1c64c03 -- I suspect if you try with this commit hash, hopefully the issue should be fixed.

milahu commented 1 year ago

better : )

now my next case is failing

# pysubs2.exceptions.UnknownFPSError: Framerate was not specified and cannot be read from the MicroDVD file.
sub_files=(bad.sub good.sub)

expected: this should fail *early* because all input files have an unsupported output format


this case fails as expected

# NotImplementedError: unsupported output format: sub
# fixme: the input file should not be processed at all
sub_files=(good.sub)