[sailfishos][gecko] Disable h264 packetization mode 0. Fixes JB#55671

[d-grigorev]

videoCodec.mStronglyPreferred has higher priority than videoCodec.mEnabled
therefore SDP negotiation fails when the peer offers h264 packetization
mode 0. This commit makes gecko strongly prefer only packetization mode 1,
so mode 0 will never be selected.

[llewelld]

I personally find understanding patch changes tricky (too many red/green varieties), so in case it's helpful to others, this is the diff between the patched file before this change, and the patched file afterwards.

diff --git a/PeerConnectionImpl.cpp.pre b/PeerConnectionImpl.cpp.post
index 3b32261..ad810b8 100644
--- a/PeerConnectionImpl.cpp.pre
+++ b/PeerConnectionImpl.cpp.post
@@ -851,7 +851,11 @@ class ConfigureCodec {
           // Might disable it, but we set up other params anyway
           videoCodec.mEnabled = mH264Enabled;
 
-          if (mHardwareH264Supported) {
+          if (videoCodec.mPacketizationMode == 0 && !mSoftwareH264Enabled) {
+            // We're assuming packetization mode 0 is unsupported by
+            // hardware.
+            videoCodec.mEnabled = false;
+          } else if (mHardwareH264Supported) {
             videoCodec.mStronglyPreferred = true;
           }
         } else if (videoCodec.mName == "red") {

[llewelld]

I wasn't able to test this, but the code looks good to me.

One question arises: can a video codec be selected at all if videoCodec.mEnabled == false;? If not, it might be easier to leave this code (lines 854-862) in their original form (i.e. remove this portion of the patch) to essentially give the same result:

 854           if (videoCodec.mPacketizationMode == 0 && !mSoftwareH264Enabled) {
 855             // We're assuming packetization mode 0 is unsupported by
 856             // hardware.
 857             videoCodec.mEnabled = false;
 858           }
 859 
 860           if (mHardwareH264Supported) {
 861             videoCodec.mStronglyPreferred = true;
 862           }

Looking through the code, I wasn't able to figure out if this was actually the case or not.

[llewelld]

Code changes LGTM, but untested.

[rainemak]

@d-grigorev could you rebase this one, thank you.

[d-grigorev]

One question arises: can a video codec be selected at all if videoCodec.mEnabled == false;?

Actually, this is the problem. If videoCodec.mStronglyPreferred is set, codec is moved on top of the preference list and then dropped because of videoCodec.mEnabled, so the negotiation fails.

I rebased this change.

[rainemak]

One question arises: can a video codec be selected at all if videoCodec.mEnabled == false;?

Actually, this is the problem. If videoCodec.mStronglyPreferred is set, codec is moved on top of the preference list and then dropped because of videoCodec.mEnabled, so the negotiation fails.

How do you want to handle this? Should we land this like it is now?

[d-grigorev]

How do you want to handle this? Should we land this like it is now?

This was the issue only for H264. The code for VP9 doesn't set videoCodec.mStronglyPreferred if VP9 is disabled, so yes, I think we should merge it as it is now.