Skip to content

Modernize codebase, add scaling, and fix memory leaks - #108

Open
csingleplus wants to merge 87 commits into
ruediger:masterfrom
csingleplus:master
Open

Modernize codebase, add scaling, and fix memory leaks#108
csingleplus wants to merge 87 commits into
ruediger:masterfrom
csingleplus:master

Conversation

@csingleplus

Copy link
Copy Markdown

Hello! I remembered this tool from quite some time ago, and a few months ago I noticed it needed adjustments to build correctly. This PR brings the original public tree closer to the state already maintained in my fork, with the addition of scaling.

I’m opening this both in case it is useful upstream and so users still landing on the original repository have a visible path to a maintained working version.

My fork currently maintains:

  • a legacy branch for older systems
  • a working master branch for current toolchains and the latest updates
  • a provided .deb for users who want the easiest install path

This PR is based on the current development side of that work and includes:

  • cleanup of dead disabled legacy code
  • migration away from older Tesseract usage
  • a fix for a small (67 byte) memory leak
  • subtitle scaling/rendering improvements. more on the way!
  • testing against multiple real-world DVD subtitle samples

Note: Most of these fixes are only implemented on the modern master branch. The legacy branch remains compatibility-oriented and does not include the newer code. Further work will include preprocessing of text, as sometimes, depending on the font in the subtitle, letters don't get recognized properly. While untested on them, the prebuilt modern .deb may work with systems from before ~2020.

Finally, I'd like to thank Ruediger Sonderfeld for creating this tool. It's been quite a good learning experience delving into old multimedia code like this, working through compiler errors and tracing Valgrind reports.

Change return value to 0 for missing subtitle name.
Updated installation instructions for dependencies. I did too much of a cleanup in that area.
Removed mention of Mac usage and maintenance status.
Scale factor also changed to 4x, as I originally intended.
Need to conduct more research to find the best method of text enhancement, if even possible.
Added a note about a rewrite planned for July 2026 with expected improvements.
@milahu

milahu commented Jul 2, 2026

Copy link
Copy Markdown

https://github.com/csingleplus/VobSub2SRT/blob/44b55903b19d72770566d375e7ee6048916410b6/CMakeLists.txt#L67

find_package(Tesseract REQUIRED)

should require tesseract >=4

with tesseract 3, it fails to compile with

/build/source/src/vobsub2srt.c++: In function 'int main(int, char**)':
/build/source/src/vobsub2srt.c++:280:53: error: 'OEM_LSTM_ONLY' is not a member of 'tesseract'; did you mean 'OEM_CUBE_ONLY'?
  280 |   if(tess_base_api.Init(NULL, tess_lang, tesseract::OEM_LSTM_ONLY) == -1) {
      |                                                     ^~~~~~~~~~~~~
      |                                                     OEM_CUBE_ONLY

with libpng 1.6.56, it fails to compile with

-- Found PNG: /nix/store/gsn3vddway3289p6mzy5shd1paly8dp4-libpng-apng-1.6.56/lib/libpng.so (found version "1.6.56")

/build/source/src/vobsub2srt.c++:30:10: fatal error: libpng/png.h: No such file or directory
   30 | #include <libpng/png.h>
      |          ^~~~~~~~~~~~~~

fixed by

-#include <libpng/png.h>
+#include <png.h>

without libtiff, cmake shows the cryptic error

CMake Error at /nix/store/v2i1hgv567g3v91im5x4g5bff52143i0-cmake-4.1.2/share/cmake-4.1/Modules/FindPackageHandleStandardArgs.cmake:227 (message):
  Could NOT find Tesseract (missing: Tesseract_LIBRARIES)
Call Stack (most recent call first):
  /nix/store/v2i1hgv567g3v91im5x4g5bff52143i0-cmake-4.1.2/share/cmake-4.1/Modules/FindPackageHandleStandardArgs.cmake:591 (_FPHSA_FAILURE_MESSAGE)
  CMakeModules/FindTesseract.cmake:57 (find_package_handle_standard_args)
  CMakeLists.txt:67 (find_package)

so maybe add

+find_package(TIFF REQUIRED) # for Tesseract
 find_package(Tesseract REQUIRED)

the language CLI parameter is not working
as soon as i set for example --lang deu i get garbage output in the SRT file
https://github.com/milahu/vobsub2srt-test-mad-max-1979-german

@csingleplus

csingleplus commented Jul 2, 2026

Copy link
Copy Markdown
Author

Seeing that you're on NixOS, I will install a VM with it and have a look at these issues straight away. Thanks for bringing them up!

Edit: I have tried your Mad Max .idx/.sub on my system, and it does recognize near perfect. So yes, I will have a look at this apparent NixOS-specific issue. Seems to do with a specific filesystem store Tesseract data may not be recognized normally.

@milahu

milahu commented Jul 2, 2026

Copy link
Copy Markdown

I will install a VM with it

actually, you can install Nix on any Linux distro, and then run

git clone https://github.com/milahu/vobsub2srt-test-mad-max-1979-german
cd vobsub2srt-test-mad-max-1979-german
nix-shell
command -v vobsub2srt

(but of course a VM is better for sandboxing)

Will get this to work for NixOS. Just a short while.
@csingleplus

Copy link
Copy Markdown
Author

I've uploaded a complete SRT (some mis-identified letters, but mostly accurate) here:
https://github.com/csingleplus/VobSub2SRT/tree/master/test4milahu. Will start work on the fix very soon, and thank you for the info on Nix itself. As you can see I'm not very familiar with it.

@milahu

milahu commented Jul 2, 2026

Copy link
Copy Markdown

umm, you should remove the commit 6491028
since the idx/sub files are protected by copyright
also the sub file has 5MB...

better use the test files from mpv
https://github.com/mpv-player/mpv/tree/master/test/samples

with strace i found this...

$ strace vobsub2srt patrol-mad.max1.1080p 2>&1 | grep -F .traineddata
openat(AT_FDCWD, "/nix/store/0zkrhb2g92d8yqk67vbh50mni1ax7gpj-tesseract-5.5.2/share/tessdata/deu.traineddata", O_RDONLY) = 3
^C

$ strace vobsub2srt --lang deu patrol-mad.max1.1080p 2>&1 | grep -F .traineddata 
openat(AT_FDCWD, "/nix/store/0zkrhb2g92d8yqk67vbh50mni1ax7gpj-tesseract-5.5.2/share/tessdata/div.traineddata", O_RDONLY) = 3
^C

$ strace vobsub2srt --lang de patrol-mad.max1.1080p 2>&1 | grep -F .traineddata 
openat(AT_FDCWD, "/nix/store/0zkrhb2g92d8yqk67vbh50mni1ax7gpj-tesseract-5.5.2/share/tessdata/deu.traineddata", O_RDONLY) = 3
^C

$ strace vobsub2srt --lang ger patrol-mad.max1.1080p 2>&1 | grep -F .traineddata 
openat(AT_FDCWD, "/nix/store/0zkrhb2g92d8yqk67vbh50mni1ax7gpj-tesseract-5.5.2/share/tessdata/eng.traineddata", O_RDONLY) = 3
^C

@csingleplus

Copy link
Copy Markdown
Author

umm, you should remove the commit 6491028 since the idx/sub files are protected by copyright also the sub file has 5MB...

Done. Regarding your strace results, it's as I expected. I will need to add a routine that searches Nix stores for the Tesseract installation. Good find. I should be able to do that from my Linux install, but will test myself on Nix before I commit it.

@milahu

milahu commented Jul 2, 2026

Copy link
Copy Markdown

Done

revert commit != remove commit

git branch bak-$(date -Is)
git reset --hard HEAD~2 # remove the last 2 commits
git push -f

I will need to add a routine that searches Nix stores for the Tesseract installation.

bad idea... the tessdata dir should be

  • the default dir (which is found in my case, only the language selection is broken)
  • the TESSDATA_PREFIX env
  • the tesseract-data CLI parameter

so the only problem i see is the language selection

  • lang=default → deu (good)
  • lang=deu → div (bad)
  • lang=de → deu (good)
  • lang=ger → eng (bad)

Add conditional inclusion for png.h based on system headers.
Updated links to use HTTPS and clarified Tesseract support deprecation. Removed outdated PPA instructions and added NixOS build instructions.
Hey, it's late. So I missed a couple lines.
@csingleplus

Copy link
Copy Markdown
Author

I can confirm my build works with Nix. It just needs language detection fixed as you mentioned, which I'll look at next.

I should have taken care of this earlier. At least it was just a warning.
Updated project status and future plans for codebase improvements.
Removed unused include for the format library. Must have been left over from some testing, but now compiles under GCC 12. VobSub2SRT confirmed working with Manjaro ARM on a PinePhone now, too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants