-
Notifications
You must be signed in to change notification settings - Fork 12
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
Improve error handling of srt and add support for 2.7k and 4k on board dvr video as input #43
Conversation
Currently it can parse debug srt, but is bypassing srt parsing completelly.
Also, I ran cargo update to fix build error caused by outdated ahash |
should address #39 |
for #40 , won't help with missing srt, but should handle corrupt srt better |
If anyone wants to test, There are unofficial binaries here: https://github.com/mmosca/walksnail-osd-tool/releases/tag/r0.2 |
Hi, thanks for the contribution! I had this on my to-do list since the new camera was launched but couldn't get around to it. I'm wondering if it would be better to upscale the 720p or 1080p font in the program itself instead of having the user supply them. After all, on the goggles these higher resolutions don't exist so the fonts are only needed for rendering the OSD on the onboard DVR. For the 4k resolution this can be done without scaling artifacts but for the 2.7k resolution this is not the case. Do you know how sneaky generated the font to this resolution? Clippy complained about a few minor things, I pushed a commit to resolve those. |
I considered upscaling, but this was the first time I touched rust. 😅I can say Sneaky_fpv manifested interest in having a go at the 4K and 2.7k fonts, due to the added clarity. The result of the upscaled 1080p file was not bad, but I am sure he can find a use for the extra pixels.
|
he Just scalled the 1080p image. 4k is 200% width and height, 2.7 I think is 133% |
In that case I think it makes more sense to scale the font in the tool, before rendering starts. The |
The scaling of the font for 4k and 2.7k was done on the vertical font file and not the source image the font files are built from. As 99% of the glyphs don't have a vertical cutoff (except for the inav logo) scaling this way was reasonably effective as you have a hard side boundary. So you don't have scaling artefacts bleeding into other adjacent cells, except vertical ones. If you're scaling in the tool itself, provide an option for the user to supply a proper 4k font file, or recognise the difference in the source font file accordingly and scale or not. If you scale in the tool, do so at the cell, not the file as a whole as then should also avoid the vertical artefacts when scaling bleeding over to adjacent cells. edit I've not tested the 2.7k font file so I don't know of any issues in the scaling that may have come. TBH, there's no real reason to record in 2.7k as the EIS on the moonlight is not ideal in current FW. The 2.7k font file could also just be a 200% scale of the 720p font file, but it'd look a little blurry and lose a fair bit of fidelity. |
SNEAKY_FPV_WS_INAV7_Europa(inc 2.7k 4k).zip I've added 2.7k and 4k fonts into this pack. I've updated my scripting to generate these as needed. This time they're generated as a cell scaling and not the whole font image scaling. |
Would it not be better for the tool to detect the size of the font file loaded. Then scale if required, and show a message that the font is being scaled? Then if you use a 4K font, no scaling is done and you have a better image. But if you add the 1080p font, it scales. This could potentially work for all font and image sizes. |
Also add 3 pages for completeness. Considered renaming 4 colors to 4 pages, but didn't think it was needed.
at @shannonbaker 's request, I added side by side font support for inav as well. |
Fix detection to also consider height, to remove ambiguity between font sizes.
Just added support for 540p race dvr as well |
@avsaase The code now should scale whatever font you load (up or down) if it does not match the correct size and I added a font size for race mode, so it should now work with: 540p, 720p, 1080p, 2.7k and 4k video and if the loaded font is not the correct size, it will be scaled automatically. Let me know what you think and if any other changes are needed. |
Crashes the tool if the selected font doesn't have the same number of pages addressed in the .osd file. Scaling seems to work well though. |
Does the crash happens on the release version, or before my scaling changes?I think it didn’t check if you tried to access a character from a non existing page before. How did you cause the crash? Did you try to use a 4 page osd from bf with a single page font?[]s,Marcelo Bezerra ***@***.***>On 22 Feb 2024, at 01:23, shannonbaker ***@***.***> wrote:
Crashes the tool if the selected font doesn't have the same number of pages addressed in the .osd file. Scaling seems to work well though.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>
|
On the prior version that supports BFx4 fonts (PR #35), this crash doesn't occur. This is reproduced by using a .osd file that has OSD glyphs plotted in 4 colours. i.e. from page 2, 3 or 4, and using a font png that is single page. Then move the preview slider across so it attempts to preview a glyph not present in the font file. |
Crash has been fixed. Basics should all be working. @avsaase Is this good enough for merging? I have some additional changes I may look into, but I would rather make them in a separate PR. The follow up PRs would be:
|
I want to take a closer look this weekend but I probably won't ask for major changes. If you want, you can base you new branch on this one. Thanks for the contributions! |
You can manually shift the OSD relative to the video. Otherwise, my OSD does not match the video when recording to a new file (4GB limit) |
If you are on recent walksnail firmware, it is now possible to set it to no longer split the dvr file. You need to have applited the kernel version of firmware 36.42.4 and you can set your recording option to keep. That way you won't loose osd sync again. |
In the settings it is set not to divide, but it still divides. |
You probably did not apply the firmware version with the kernel update that enables xfat support on your goggles/vrx. But getting your dvr recording to not be split, or time shifting the dvr is not really in the scope of this pull request. |
Apologies for taking so long before getting back to this. I pushed a few code quality improvements to your branch. Everything seems to work correctly although I did not do extensive testing. I'll merge the PR and make a new release. Thanks for the contribution @mmosca! |
If using a debug srt, the tool would now allow rendering osd.
While this does not use the debug srt info yet, it allows osd rendering even when there are errors parsing the srt info.
This also add 3 new font sizes for using moonlight onboard dvr as input as well as race dvr. It also increases max bitrate to 160.
Sample video rendered with the tool:
https://youtu.be/LU0JIUhSM6E
Sample fonts from Sneaky_fpv:
WS_IN_Europa_2.7k.zip
WS_IN_Europa_4k.zip
If you select the wrong font file, it will automatically scale to the correct size, but using the correct file will have better quality.