Disable video modes that are not in the firmware

[haslinghuis]

Disable elements not included in the build instead of hiding them.

See also betaflight/betaflight#13320 (comment)

Recommendation is either use OSD_SD or OSD_HD not both as switching elements is not accounted for in firmware.

[mituritsyn]

OSD_Hd has only one option at the moment, so the whole 'Video Format' section could be hidden.

[nerdCopter]

i rather see the section to know the configurator is setting it correctly. at least in this iteration, maybe 11.0 can hide the section once it is known to 100% work.

[ctzsnooze]

Maybe the status of vcd_video_system should be the factor that primarily assigns the image dimensions of the OSD tab screen?

It is, I think, the fundamental setting of relevance here.

If so, whenever vcd_video_system = HD is selected, the image should be configured to HD size, and the OSD_CANVAS width and height set appropriately, on save. If the Walksnail sends new values, then they should over-ride the defaults, I guess.

Similarly for SD options, eg vcd_video_system = PAL, the screen and canvas size values should suit PAL, and likewise for NTSC.

In situations where the only define in the build is HD_OSD, somehow we should be automatically enabling vcd_video_system = HD and, from that, defaulting the OSD screen to HD dimensions.

In situations where the only define in the build is SD_OSD, the reverse should apply, probably setting vcd_video_system = AUTO, applying SD canvas width and height appropriate for auto.

In situations where both HD_OSD and SD_OSD are in the build defines, then the size of the screen should change appropriately according to the user's selection for vcd_video_system. This means that when the user chooses, say, PAL, and saves, the canvas should change from whatever it was to the settings for PAL. And likewise for HD, etc. I think the default is Auto and that implies SD.

I notice that with the PR at present, if vcd_video_system = AUTO in CLI, in an HD_OSD only build, the video format is shown on the OSD Tab with 'HD' checked, but the canvas is set to SD.

Also, if I click 'auto' in the OSD tab, my change jumps back to 'HD', so Auto cannot be selected. That's OK I think; but Auto might as well not be shown.

Unfortunately, if the current value for vcd_video_systemisAUTO, clicking 'save', with 'HD' selected, does not write 'vcd_video_system = HD. vcd_video_system` still shows as 'Auto' in the CLI.

Also, if I deliberately set vcd_video_system = HD in CLI, replacing vcd_video_system = AUTO, and save, I do not get any changes in the canvas dimensions, and the screen on OSD tab doesn't show the HD size. I have to change the canvas width and height manually.

Hence there is a lot more work to go here.

[mituritsyn]

Hence there is a lot more work to go here.

maybe we need a simple solution for release and subsequent refactoring of the whole subsystem

[ctzsnooze]

Mark @haslinghuis, the OSD tab display must follow the vcd_video_system value. It just ignores it at present.

If I have an FC with pure HD-only firmware, but the canvas dimensions are SD at the start, there is no way in configurator, other than changing the canvas values manually in CLI, to get the canvas values correct.

If canvas values are set for SD, and if I click 'SAVE' even though HD is selected because vcd_video_system = HD, the screen should change size and the OSD_CANVAS values should be set to HD values, if they are not already. If they are already set into an HD range, they do not need to change.

Hence we need a check of the canvas dimensions when entering the OSD tab, and on saving the OSD setup. I don't mind if the check is in firmware or in Configurator, but it can be done entirely in Configurator.

In Configurator, we could say, if canvas width is <35, then the dimensions are SD. And then, if the user wants HD with vcd_video_system = HD and if canvas width < 35, we set canvas width and height to HD default values. If they already configured to HD values, nothing will be changed.

It is possible that the user has customised their HD OSD width and height, but it is not likely they would intend to use a width <35 on an HD system. So if the width and height were previously set to a non-SD range, the above check would not remove their HD screen dimension customisations.

If the vcd_video_system is AUTO, and the build is HD_ONLY, then the above applies also.

If the vcd_video_system is AUTO, NTSC or PAL, and if the canvas width is > 35, we should reset the canvas to appropriate values according the the video system user selection (see following comment).

With this PR, in an HD-only build, if the prior canvas dimensions are 30x16, the canvas is locked at 30x16, despite vcd_video_system = HD. The only fix is to update Canvas values manually in CLI by entering width and height numbers, and most people won't know how to do that.

Users will not swap rapidly from one mode to the other, so for now don't worry about re-positioning elements.

Just get the canvas width and height to honour the user selected video format.

[ctzsnooze]

PS about PAL and NTSC...

PAL(720 x 576) is 20% 'taller' than NTSC (720 x 480) in 'pixels'. If all pixels were 'square', the NTSC width:height ratio would be 1.5:1, and for PAL 1.25:1.

Interestingly on the old analog TV's, both NTSC and PAL pixels were non-square. NTSC pixels are taller than wide, ratio 1:0.906, whereas PAL pixels are wider than they are tall, 1:1.06. If we consider the non-square pixels, the old analog TVs ended up with almost exactly the same screen ratios, 1:1.36 for NTSC and 1:1.38 for PAL.

But if we stick to 'pixels' and draw them 'square', the ratios are quite different, and NTSC images end up being more of a wide-screen, with fewer vertical pixels.

in display.h I see:

#define VIDEO_COLUMNS_SD          30
#define VIDEO_LINES_NTSC          13
#define VIDEO_LINES_PAL           16

These ratios don't quite match the pixel ratios of either NTSC or PAL. That explains why I often had to make manual adjustments.

So I guess we need a way in Configurator to detect whether the user wants to use HD or SD.

If they select HD, and the values for canvas are in the SD range, ie width <35, we should use apply HD values, and vice versa.

Otherwise, unless the user changes their video system selection, we should NOT modify the user's canvas settings, which may have been carefully customised.

[limonspb]

I think hiding unused OSD modes should be separated and go in 10.11 or 11.0.
If there are issues with the current osd tab, of course we should solve them for 10.10, but separately from improvements.

[HThuren]

You can make a Build with both OSD_SD and OSD_HD, something to take into consideration or maybe change.

[SteveCEvans]

We don’t just need to consider vcd_video_system, but also osd_displayport_device. HD is only supported for MSP.

Alas WS and DJI don’t send the MSP_SET_OSD_CANVAS command to invoke https://github.com/betaflight/betaflight/blob/42267349db0c92a0e1063bd1d07633a3322dfb07/src/main/msp/msp.c#L4318

When switching to HD the canvas size gets initialised at
https://github.com/betaflight/betaflight/blob/42267349db0c92a0e1063bd1d07633a3322dfb07/src/main/msp/msp.c#L4183

Rather that hiding the video resolution selection on the configurator, wouldn’t it be better to just grey out the options which aren’t available?

That said, do we have a big enough issue here for this to be classified as a bug and thus worthy of inclusion in 4.5/10.10?
The configurator shouldn’t mess with the canvas size, but should pick up what the firmware tells it.

[haslinghuis]

Hiding video option is not the issue - as it reflects what is configurable in the build. Would be the same as gray out options for me.

MSPCodes.MSP_SET_OSD_CANVAS does not fire as MSP_SET_OSD_CONFIG already sets video_system correctly. So this call does nothing at the moment - so this has been removed for now.

[nerdCopter]

bug found. with local builds without buildkey.

local build without build-key - make FOXEERF405 EXTRA_FLAGS="-D'RELEASE_NAME=4.5.0-zulu' -DCLOUD_BUILD -DUSE_CAMERA_CONTROL -DUSE_DSHOT -DUSE_EMFAT_AUTORUN -DUSE_EMFAT_ICON -DUSE_EMFAT_TOOLS -DUSE_GPS -DUSE_GPS_PLUS_CODES -DUSE_LED_STRIP -DUSE_LED_STRIP_64 -DUSE_MAG -DUSE_OSD -DUSE_OSD_SD -DUSE_PINIO -DUSE_SERIALRX -DUSE_SERIALRX_CRSF -DUSE_TELEMETRY -DUSE_TELEMETRY_CRSF -DUSE_VTX"

cannot select PAL/NTSC

will test, other customs and edit comment.

edit: with only -DUSE_OSD_HD properly sets hd & locks all.

when both SD and HD defined, set HD and lock all.

[nerdCopter]

🚀 2c8f4b72 seemingly fixed for local builds without build-key.
SD-only hex: selecting HD, will auto pop-back to another SD.
HD-only hex: selecting an SD will pop-back to HD.
both in hex: works as expected.

did not re-test cloud-built hex.

[ctzsnooze]

@haslinghuis

With a local build using these settings:

make FOXEERF722V3 EXTRA_FLAGS="-D'RELEASE_NAME=4.5.0-zulu' -DCLOUD_BUILD -DUSE_DHOT -DUSE_GPS -DUSE_GPS_PLUS_CODES -DUSE_TELEMETRY -DUSE_TELEMETRY_CRSF -DUSE_OSD -DUSE_OSD_SD -DUSE_MAG -DUSE_LED_STRIP"

I see AUTO, PAL, NTSC and HD options in the OSD tab. Selecting HD drops back to the previous selection. This behaviour is like for Master.

original test results not correct...

With a cloud build, it looks, on first glance, exactly the same. But there is a tiny difference. The button next to the text 'HD' is grey, not black, and cannot be selected. It is a bit subtle, so I missed it:

Now that I cannot select HD, this is a bit better than before. It helps the user understand that they don't have the option when they are shown disabled.

It would definitely be easier to interpret if the text with the button was also made grey :-) sorry if that is a lot of work.

PS I'm using the 7401d32 build downloaded from here dated 26 January.

[haslinghuis]

Instead of hiding - we disable radio buttons which are not included in the build - so if user only use OSD_SD or OSD_HD it won't affect centering of elements.

[ctzsnooze]

Would it be possible to 'grey' the text associated with the option as well as the little circle for the option? eg in the above image, not just make the little circle grey, but also make the text 'HD' grey?

That would be really good I think. A bit confusing at the moment.

Definitely works very nicely for cloud builds, I tested both SD only and HD only. For both SD and HD, behaviour is OK also, just the screen moves around, that's OK.

If the text could be grey, that would be awesome!

[haslinghuis]

@ctzsnooze changed to

[sonarqubecloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[github-actions]

Do you want to test this code? Here you have an automated build:
Betaflight-Configurator-Android
Betaflight-Configurator-Linux
Betaflight-Configurator-macOS
Betaflight-Configurator-Windows
WARNING: It may be unstable and result in corrupted configurations or data loss. Use only for testing!

[HThuren]

@haslinghuis @nerdCopter @ctzsnooze I had trouble to flash with the BF PR, I may have better luck next time

[limonspb]

Recommendation is either use OSD_SD or OSD_HD not both as switching elements is not accounted for in firmware.

i think now it is being accommodated in the firmware after betaflight/betaflight#13320