-
Notifications
You must be signed in to change notification settings - Fork 504
[FIX] -out=spupng with EIA608/teletext: offset values in XML may be not correct #893 #1850
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
base: master
Are you sure you want to change the base?
Conversation
|
What video file did you use to test this? |
…oder - Introduced a forward declaration for . - Updated to calculate and set image dimensions before writing XML tags. - Adjusted offset calculations based on screen size for better alignment of subtitles. - Improved handling of the opening XML tag based on subtitle data presence.
|
Testing was done mostly using tests/02-BBC2.No.Such.Thing.As.News.ts, which is a BBC PAL, 720 I manually checked the generated PNG size and the xoffset/yoffset information within the XML file, and I found that it didn't conform to the formula for centering, resulting in this patch. This PR addresses the problem by - correcting the offset calculation order (offsets are now calculated after PNG generation), the removal of NTSC hardcoded numbers, applying the proper centering formula, even alignment. During testing, I also noticed that format detection in Teletext may not be represented in the encoder context (in_fileformat / is_pal) at SPUPNG. I’ve chosen to keep this PR just on the offset logic, rather than making changes to format detection. The NTSC/EIA-608 part of the pipeline remains the same; the current Usa.ts functionality breaks on my Mac environment with respect to font support, not offset logic. |
CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit e3b0def...:
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:
All tests passing on the master branch were passed completely. Check the result page for more info. |
CCExtractor CI platform finished running the test files on windows. Below is a summary of the test results, when compared to test for commit e3b0def...:
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. |
[FIX]
In raising this pull request, I confirm the following (please check boxes):
My familiarity with the project is as follows (check one):
[FIX] -out=spupng with EIA608/teletext: offset values in XML may be not correct #893
The issue occurs because the offsets were hardcoded for a 544×390 subtitle image centered on a 720×480 NTSC screen, while the text-to-PNG renderer generates PNGs with different (dynamic) dimensions (e.g. 600px wide and variable height). This results in visibly misaligned subtitles, especially for Teletext (PAL) sources.
This PR removes the hardcoded NTSC assumptions and calculates SPUPNG offsets dynamically based on the actual rendered PNG geometry and the target screen resolution.
Key changes =>
2 .Introduce an is_pal flag in encoder_ctx to distinguish PAL (576 lines) vs NTSC (480 lines)
4.Add a helper to compute centered xOffset / yOffset dynamically
Enforce even yOffset to prevent interlacing flicker on DVD/SPU
Apply the new logic only to the text-to-SPUPNG pipeline, leaving the bitmap path unchanged
This aligns the text-based SPUPNG output with the already-correct bitmap SPUPNG behavior.
So ,Subtitles are correctly positioned regardless of PNG width/height (PNG size decides the offset — not hardcoded values.).
in the given image i have drawn the flow of this fix .
