Add option to encode using baseline/constrained baseline. #1932

20kdc · 2023-12-17T23:05:46Z

To be clear, this commit was only tested (and I only have the hardware to test it) with PhoneVR.
EDIT: I did manage to get PhoneVR running to test this commit on master with PhoneVR.

In addition, I have not tested this code on all platforms. I've made a best-effort attempt to get it to work, but I don't know if it will fail on Windows, or on an NVIDIA graphics card, etc. As long as everything compiles and nothing changes if the option is not activated, it should be okay, though.

This commit allows encoding using baseline/constrained baseline. This may improve hardware compatibility, and allows for the "software decoders of last resort" which (according to Moonlight code comments (https://github.com/moonlight-stream/moonlight-android/blob/master/app/src/main/java/com/limelight/binding/video/MediaCodecHelper.java#L90) ) do not like High profile/etc.

I'm unsure if this should be a boolean setting as it is now or be treated like another codec.

zmerp

Instead of having a flag for enabling Baseline profile, I would make an enum for Baseline, Main and High.

zmerp · 2023-12-18T14:43:09Z

alvr/server/cpp/alvr_server/Settings.cpp

+        auto sessionSettings = v.get("session_settings");
+        auto sessionVideo = sessionSettings.get("video");
+
+        m_h264UseBaselineProfile = sessionVideo.get("h264_use_baseline_profile").get<bool>();


If you write this code like this, SteamVR will not restart when this setting has been changed. Please check how the other settings are passed to SteamVR and do something similar.

zmerp · 2023-12-18T14:45:15Z

alvr/session/src/settings.rs

+        help = "Whenever possible, attempts to force the 'baseline profile' or the 'constrained baseline profile' to increase compatibility with varying mobile devices. Only has an effect for h264 and may not have an effect for some configurations."
+    ))]
+    #[schema(flag = "steamvr-restart")]
+    pub h264_use_baseline_profile: bool,


I would put the setting inside EncoderConfig.

zmerp · 2023-12-18T14:46:55Z

Another option is to always force Baseline. I believe this should reduce latency a little bit, although quality may be worse. More people should test this PR

deiteris · 2023-12-22T09:22:24Z

alvr/server/cpp/platform/win32/VideoEncoderAMF.cpp

+			amfEncoder->SetProperty(AMF_VIDEO_ENCODER_PROFILE, AMF_VIDEO_ENCODER_PROFILE_CONSTRAINED_BASELINE);
+		} else {
+			amfEncoder->SetProperty(AMF_VIDEO_ENCODER_PROFILE, AMF_VIDEO_ENCODER_PROFILE_HIGH);
+			amfEncoder->SetProperty(AMF_VIDEO_ENCODER_PROFILE_LEVEL, 42);


You can move profile level outside the condition. This is common for both profiles.

deiteris · 2023-12-22T09:26:28Z

Another option is to always force Baseline. I believe this should reduce latency a little bit, although quality may be worse. More people should test this PR

I'd suggest to keep Main and High profiles since it doesn't make much difference on modern decoders. But need to gather numbers on both encoder/decoder to see the actual impact.

…ng to OpenvrSettings

20kdc · 2023-12-22T17:10:20Z

Hopefully deals with all the reviews, but with that said this PR needs a lot of testing. I avoided trying to implement support for NVENC on Windows owing to the likelihood that a misconfiguration here would lead to breakage (there are many other options and this can affect the exposed profile), and the Linux-SW-x264 driver seems to always use Constrained Baseline regardless of what's passed.

I'm not sure why x264 support on Windows uses ffmpeg, and I'm really not sure why that's the case while it's different for Linux. Not the purpose of this PR, though.

ShootingKing-AM · 2023-12-23T14:25:15Z

Tested on Windows 10 and NVIDIA seems to work fine, but based on ALVR stats page I see neither loss nor gain in perf/quality. Tested all Profiles.

20kdc · 2023-12-23T15:26:20Z

That would be because I avoided implementing support for NVENC on Windows because it seemed risky (the mechanism used for profile selection seems like it could lead to failures). Should I do so anyway?

ShootingKing-AM · 2023-12-24T07:20:49Z

I used h264 encoding option through ALVR settings

zmerp · 2023-12-24T16:46:15Z

alvr/session/src/settings.rs

+pub enum H264Profile {
+    #[schema(strings(display_name = "High"))]
+    High = 0,
+    #[schema(strings(display_name = "Main"))]
+    Main = 1,
+    #[schema(strings(display_name = "Baseline"))]
+    Baseline = 2,
+}


I think it's more natural to have Baseline as 0th index, then Main then High. Also, you don't need to specify the display_name, as in this case the dashboard is able to figure it out by itself.

High was set as zeroth index because I was uncertain of the behaviour of certain defaults. In most safe languages, values which are not strictly defined but are initialized end up zero. Rust is not necessarily the same, but there is also C++ code involved with no default value provided when reading from the JSON. I do not know the behaviour of this code on error, so I decided the zero value should be the default value, so the behaviour is as consistent as reasonably possible.

I'm not too opinionated, that's ok then.

zmerp · 2023-12-24T16:48:09Z

alvr/session/src/settings.rs

@@ -1165,6 +1184,9 @@ pub fn session_settings_default() -> SettingsDefault {
                    variant: RateControlModeDefaultVariant::Cbr,
                },
                filler_data: false,
+                h264_profile: H264ProfileDefault {
+                    variant: H264ProfileDefaultVariant::High,


I believe the default level should be Main. Why was High chosen?

High was chosen for continuity with the existing default settings in some of the encoders.
Reviewing the diff, it is clear that most encoders default to High right now.

@nowrep Any reason for this? Main should have less latency.

Not sure how reliable this is but this says that high main and base have no difference in latency
https://ipvm.com/reports/h264-high-profile-tested
Given these are just cameras idk

While higher profiles do increase complexity it seems it may not correlate to latency

If you want to fix software decoder compatibility then just use ffmpeg in the client instead of MediaCodec.

While this is correct, in practice, AHardwareBuffers are involved, and it'd likely involve some refactoring, and there are APIs that expose these AHardwareBuffers. I don't have the necessary knowledge.

I believe the encoding profile is still good to have.
@20kdc have you checked if the decoder can provide its own supported profiles? We could automate this as a future optimization

As far as I'm aware decoder can't provide supported profiles outside of successful or failed initialization.
Even if it could, decoder bugs make the output potentially untrustworthy.
The encoder might be able to provide supported profiles to some extent, but that's more like an API-level thing, and it requires getting this information about support to the Dashboard, somehow.

As far as I'm aware decoder can't provide supported profiles outside of successful or failed initialization

That's too bad

it requires getting this information about support to the Dashboard

No we don't. we can do the same thing we do for FPS, we just choose the closes one if unsupported by the client.

zmerp · 2024-01-02T02:55:18Z

If there are no objections I would merge this in a day

Add option to encode using baseline/constrained baseline.

c8b4358

20kdc mentioned this pull request Dec 17, 2023

Android: When the hardware/default decoder fails to initialize, fall back to software. #1933

Merged

zmerp requested changes Dec 18, 2023

View reviewed changes

deiteris reviewed Dec 22, 2023

View reviewed changes

Allow selecting h264 profile with a bit more precision, migrate setti…

98855a9

…ng to OpenvrSettings

zmerp requested changes Dec 24, 2023

View reviewed changes

zmerp reviewed Dec 24, 2023

View reviewed changes

zmerp approved these changes Jan 2, 2024

View reviewed changes

zmerp merged commit 12e6ac6 into alvr-org:master Jan 2, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to encode using baseline/constrained baseline. #1932

Add option to encode using baseline/constrained baseline. #1932

20kdc commented Dec 17, 2023 •

edited

Loading

zmerp left a comment

zmerp Dec 18, 2023

zmerp Dec 18, 2023

zmerp commented Dec 18, 2023 •

edited

Loading

deiteris Dec 22, 2023

deiteris commented Dec 22, 2023 •

edited

Loading

20kdc commented Dec 22, 2023 •

edited

Loading

ShootingKing-AM commented Dec 23, 2023

20kdc commented Dec 23, 2023

ShootingKing-AM commented Dec 24, 2023 •

edited

Loading

zmerp Dec 24, 2023

20kdc Dec 24, 2023

zmerp Dec 24, 2023

zmerp Dec 24, 2023 •

edited

Loading

20kdc Dec 24, 2023

zmerp Dec 24, 2023

Vixea Dec 24, 2023

Vixea Dec 24, 2023

nowrep Jan 1, 2024

20kdc Jan 1, 2024

zmerp Jan 2, 2024

20kdc Jan 2, 2024

zmerp Jan 3, 2024

zmerp commented Jan 2, 2024

Add option to encode using baseline/constrained baseline. #1932

Add option to encode using baseline/constrained baseline. #1932

Conversation

20kdc commented Dec 17, 2023 • edited Loading

zmerp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zmerp commented Dec 18, 2023 • edited Loading

Choose a reason for hiding this comment

deiteris commented Dec 22, 2023 • edited Loading

20kdc commented Dec 22, 2023 • edited Loading

ShootingKing-AM commented Dec 23, 2023

20kdc commented Dec 23, 2023

ShootingKing-AM commented Dec 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zmerp Dec 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zmerp commented Jan 2, 2024

20kdc commented Dec 17, 2023 •

edited

Loading

zmerp commented Dec 18, 2023 •

edited

Loading

deiteris commented Dec 22, 2023 •

edited

Loading

20kdc commented Dec 22, 2023 •

edited

Loading

ShootingKing-AM commented Dec 24, 2023 •

edited

Loading

zmerp Dec 24, 2023 •

edited

Loading