Skip to content

[FIX] add mac-ocr-hardsubx workflow & ffmpeg variants support #1745

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Nov 3, 2025
Merged

[FIX] add mac-ocr-hardsubx workflow & ffmpeg variants support #1745

merged 12 commits into from
Nov 3, 2025

Conversation

@prateekmedia
Copy link
Member

@prateekmedia prateekmedia commented Sep 6, 2025 •
edited
Loading

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.
  • I have mentioned this change in the changelog.

My familiarity with the project is as follows (check one):

  • I have never used CCExtractor.
  • I have used CCExtractor just a couple of times.
  • I absolutely love CCExtractor, but have not contributed previously.
  • I am an active contributor to CCExtractor.

Fix

  • Update params and there doc

Mac OS:

  • Fix FFMpeg, tesseract compilation
  • Re-add Mac os build hardsubx workflow

FFMpeg used in workflow:

  • MacOS: 8.*
  • Windows: 6.* (pinned VCPKG supports this)
  • Linux: 6.* (Latest ubuntu runner supports this)

@prateekmedia prateekmedia force-pushed the hardsubx_patch branch 4 times, most recently from 4c14b91 to 9a8e379 Compare September 6, 2025 20:34
@prateekmedia
Fix critical hardsubx OCR bugs affecting accuracy and stability
4e96e5f 
- Fix iterator initialization in check_color_alternate_pixels
- Add proper null checks before dereferencing OCR results
- Fix confidence threshold comparison (use abs for negative confidences)
- Fix c_char array initialization in avc/nal.rs for cross-platform compatibility
- Prevent segmentation faults in OCR processing
@prateekmedia
Fix CLI parameters and add platform-specific paths for OCR
469debd 
- Fix CLI argument parsing to use hyphens (--conf-thresh not --conf_thresh)
- Add Homebrew tessdata path (/opt/homebrew/share/) for macOS
- Ensure consistent parameter naming across C and Rust implementations
@prateekmedia
Configure rsmpeg for platform-specific FFmpeg versions
660d524 
- Use rsmpeg 0.18.0 with platform-specific FFmpeg features
- Linux: FFmpeg 6, Windows: FFmpeg 7, macOS: FFmpeg 8
- Single rsmpeg version prevents multiple FFmpeg compilations
- Clean configuration without unnecessary abstraction layers
@prateekmedia
Update build scripts to support FFmpeg version selection
828a9cf 
- Add FFMPEG_VERSION environment variable support in build scripts
- Linux/macOS: Support override via FFMPEG_VERSION env var
- Windows: Support override in rust.bat
- Fix FFmpeg include path detection for Homebrew on macOS
@prateekmedia
Update CI workflows and Windows configuration
b0529d8 
- Update macOS workflow to properly detect FFmpeg from Homebrew
- Update Windows vcpkg baseline for FFmpeg 7.1.1 compatibility
- Remove unnecessary CI-specific rsmpeg configuration
- Workflows now use platform defaults automatically
@prateekmedia
Update COMPILATION.md with FFmpeg version configuration
7d94a15 
- Document platform-specific FFmpeg defaults
- Add instructions for overriding FFmpeg versions
- Include hardsubx build examples for all platforms
- Add FFmpeg dependency information for macOS
@prateekmedia prateekmedia changed the title [FIX] Hardsubx fixes + Mac OS build fixes [FIX] Hardsubx fixes + build fixes Sep 6, 2025
@prateekmedia prateekmedia changed the title [FIX] Hardsubx fixes + build fixes [FIX] Hardsubx + build fixes Sep 6, 2025
@prateekmedia prateekmedia marked this pull request as ready for review September 6, 2025 20:42
@ccextractor-bot
Copy link
Collaborator

ccextractor-bot commented Sep 7, 2025

CCExtractor CI platform finished running the test files on windows. Below is a summary of the test results, when compared to test for commit c2a1f0d...:
Report Name Tests Passed
Broken 13/13
CEA-708 14/14
DVB 7/7
DVD 3/3
DVR-MS 2/2
General 27/27
Hardsubx 0/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 86/86
Teletext 21/21
WTV 13/13
XDS 33/34

Your PR breaks these cases:

  • ccextractor --autoprogram --out=ttxt --xds --latin1 --ucla e274a73653...

NOTE: The following tests have been failing on the master branch as well as the PR:

Congratulations: Merging this PR would fix the following tests:

  • ccextractor --autoprogram --out=srt --latin1 f1422b8bfe..., Last passed: Never
  • ccextractor --datapid 5603 --autoprogram --out=srt --latin1 --teletext 85c7fc1ad7..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 c0d2fba8c0..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 006fdc391a..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 e92a1d4d2a..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 7e4ebf7fd7..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 9256a60e4b..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 27d7a43dd6..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 297a44921a..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 efbe129086..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 eae0077731..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 e2e2b501e0..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 c6407fb294..., Last passed: Never
  • ccextractor --autoprogram --out=ttxt --latin1 --datets dcada745de..., Last passed: Never
  • ccextractor --autoprogram --out=srt --latin1 --tpage 398 5d5838bde9..., Last passed: Never
  • ccextractor --autoprogram --out=srt --latin1 --teletext --tpage 398 3b276ad8bf..., Last passed: Never

It seems that not all tests were passed completely. This is an indication that the output of some files is not as expected (but might be according to you).

Check the result page for more info.

@prateekmedia prateekmedia changed the title [FIX] Hardsubx + build fixes build fixes Nov 2, 2025
@prateekmedia prateekmedia changed the title build fixes [FIX] add mac rust-ocr workflow again and different ffmpeg support Nov 2, 2025
@prateekmedia prateekmedia changed the title [FIX] add mac rust-ocr workflow again and different ffmpeg support [FIX] add mac rust-ocr workflow again & different ffmpeg support Nov 2, 2025
@prateekmedia prateekmedia changed the title [FIX] add mac rust-ocr workflow again & different ffmpeg support [FIX] add mac-ocr-hardsubx workflow & ffmpeg variants support Nov 2, 2025
@ccextractor-bot
Copy link
Collaborator

ccextractor-bot commented Nov 2, 2025

CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit 5338c15...:
Report Name Tests Passed
Broken 13/13
CEA-708 14/14
DVB 7/7
DVD 3/3
DVR-MS 2/2
General 27/27
Hardsubx 0/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 86/86
Teletext 21/21
WTV 13/13
XDS 34/34

Your PR breaks these cases:


It seems that not all tests were passed completely. This is an indication that the output of some files is not as expected (but might be according to you).

Check the result page for more info.

canihavesomecoffee
canihavesomecoffee approved these changes Nov 3, 2025
View reviewed changes
src/lib_ccx/ocr.c
Comment on lines 108 to 109
"/usr/share/tesseract-ocr/4.00/",
"/usr/share/tesseract-ocr/5/",
Copy link
Member

@canihavesomecoffee canihavesomecoffee Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still support these? If not, we can probably remove them to keep the code base clean

Copy link
Member Author

@prateekmedia prateekmedia Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing changed on that part from past, so I do think we support it.

@prateekmedia prateekmedia merged commit 42d7509 into master Nov 3, 2025
15 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@canihavesomecoffee canihavesomecoffee canihavesomecoffee approved these changes

@cfsmp3 cfsmp3 Awaiting requested review from cfsmp3

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@prateekmedia @ccextractor-bot @canihavesomecoffee