Skip to content
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

Merged
merged 27 commits into from
Mar 23, 2024

Conversation

mmosca
Copy link
Contributor

@mmosca mmosca commented Feb 11, 2024

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.

@mmosca
Copy link
Contributor Author

mmosca commented Feb 11, 2024

Also, I ran cargo update to fix build error caused by outdated ahash

@mmosca
Copy link
Contributor Author

mmosca commented Feb 11, 2024

should address #39

@mmosca
Copy link
Contributor Author

mmosca commented Feb 11, 2024

for #40 , won't help with missing srt, but should handle corrupt srt better

@mmosca
Copy link
Contributor Author

mmosca commented Feb 11, 2024

If anyone wants to test, There are unofficial binaries here: https://github.com/mmosca/walksnail-osd-tool/releases/tag/r0.2

@mmosca mmosca mentioned this pull request Feb 11, 2024
1 task
@avsaase
Copy link
Owner

avsaase commented Feb 11, 2024

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.

@mmosca
Copy link
Contributor Author

mmosca commented Feb 12, 2024 via email

@mmosca
Copy link
Contributor Author

mmosca commented Feb 12, 2024

Do you know how sneaky generated the font to this resolution

he Just scalled the 1080p image.

4k is 200% width and height, 2.7 I think is 133%

@avsaase
Copy link
Owner

avsaase commented Feb 12, 2024

In that case I think it makes more sense to scale the font in the tool, before rendering starts. The image crate, which this project already depends on, provides a method to resize images with different scaling modes.

@shannonbaker
Copy link

shannonbaker commented Feb 12, 2024

In that case I think it makes more sense to scale the font in the tool, before rendering starts. The image crate, which this project already depends on, provides a method to resize images with different scaling modes.

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.

@shannonbaker
Copy link

shannonbaker commented Feb 12, 2024

SNEAKY_FPV_WS_INAV7_Europa(inc 2.7k 4k).zip
SNEAKY_FPV_WS_BFx4_Moonlight.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.

@MrD-RC
Copy link

MrD-RC commented Feb 17, 2024

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.
@mmosca
Copy link
Contributor Author

mmosca commented Feb 19, 2024

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.
@mmosca
Copy link
Contributor Author

mmosca commented Feb 19, 2024

Just added support for 540p race dvr as well

@mmosca
Copy link
Contributor Author

mmosca commented Feb 20, 2024

@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.

@shannonbaker
Copy link

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.

@mmosca
Copy link
Contributor Author

mmosca commented Feb 22, 2024 via email

@shannonbaker
Copy link

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.

@mmosca
Copy link
Contributor Author

mmosca commented Feb 23, 2024

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:

  • Actual debug srt rendering. There are many fields, so I may try to make some bigger UI changes for the srt fields.
  • Preprocessing or caching scaled glyphs to increase rendering performance when font scaling is needed.

@avsaase
Copy link
Owner

avsaase commented Feb 23, 2024

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!

@kasatka60
Copy link

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)

@mmosca
Copy link
Contributor Author

mmosca commented Mar 17, 2024

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.

@kasatka60
Copy link

In the settings it is set not to divide, but it still divides.

@mmosca
Copy link
Contributor Author

mmosca commented Mar 20, 2024

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.

@avsaase
Copy link
Owner

avsaase commented Mar 23, 2024

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!

@avsaase avsaase merged commit 345deac into avsaase:master Mar 23, 2024
9 checks passed
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.

5 participants