Clamp property values when restoring them

app/streaming/video/ffmpeg-renderers/drm.cpp

@@ -182,20 +182,20 @@ DrmRenderer::~DrmRenderer()
 
         // Revert our changes from prepareToRender()
         if (auto prop = m_Connector.property("content type")) {
-            m_PropSetter.set(*prop, prop->initialValue());
+            m_PropSetter.restorePropertyToInitial(*prop);
         }
         if (auto prop = m_Crtc.property("VRR_ENABLED")) {
-            m_PropSetter.set(*prop, prop->initialValue());
+            m_PropSetter.restorePropertyToInitial(*prop);
         }
         if (auto prop = m_Connector.property("max bpc")) {
-            m_PropSetter.set(*prop, prop->initialValue());
+            m_PropSetter.restorePropertyToInitial(*prop);
         }
         if (auto zpos = m_VideoPlane.property("zpos"); zpos && !zpos->isImmutable()) {
-            m_PropSetter.set(*zpos, zpos->initialValue());
+            m_PropSetter.restorePropertyToInitial(*zpos);
         }
         for (int i = 0; i < Overlay::OverlayMax; i++) {
             if (auto zpos = m_OverlayPlanes[i].property("zpos"); zpos && !zpos->isImmutable()) {
-                m_PropSetter.set(*zpos, zpos->initialValue());
+                m_PropSetter.restorePropertyToInitial(*zpos);
             }
         }
         for (auto &plane : m_UnusedActivePlanes) {
@@ -367,8 +367,7 @@ void DrmRenderer::prepareToRender()
             enableVrr = !m_Vsync;
         }
 
-        auto range = prop->range();
+        m_PropSetter.set(*prop, prop->clamp(enableVrr ? 1 : 0));
-        m_PropSetter.set(*prop, std::clamp<uint64_t>(enableVrr ? 1 : 0, range.first, range.second));
     }
 
     if (auto prop = m_Connector.property("max bpc")) {
@@ -382,8 +381,7 @@ void DrmRenderer::prepareToRender()
         }
 
         if (maxBpc > 0) {
-            auto range = prop->range();
+            m_PropSetter.set(*prop, prop->clamp(maxBpc));
-            m_PropSetter.set(*prop, std::clamp<uint64_t>(maxBpc, range.first, range.second));
         }
     }
 
@@ -400,8 +398,7 @@ void DrmRenderer::prepareToRender()
             // means undefined ordering between the planes, but that's fine.
             // The planes should never overlap anyway.
             if (!zpos->isImmutable() && zpos->initialValue() <= m_VideoPlaneZpos) {
-                auto zposRange = zpos->range();
+                uint64_t newZpos = zpos->clamp(m_VideoPlaneZpos + 1);
-                uint64_t newZpos = std::clamp(m_VideoPlaneZpos + 1, zposRange.first, zposRange.second);
 
                 SDL_LogInfo(SDL_LOG_CATEGORY_APPLICATION,
                             "Moving overlay plane %u to zpos: %" PRIu64,
@@ -758,7 +755,7 @@ bool DrmRenderer::initialize(PDECODER_PARAMETERS params)
                     }
                 }
                 else {
-                    auto zposRange = zpos->range();
+                    auto zposRange = *zpos->range();
 
                     uint64_t lowestAcceptableZpos;
 
@@ -782,7 +779,7 @@ bool DrmRenderer::initialize(PDECODER_PARAMETERS params)
                         lowestAcceptableZpos = *zposIt + 1;
                     }
 
-                    m_VideoPlaneZpos = std::clamp(lowestAcceptableZpos, zposRange.first, zposRange.second);
+                    m_VideoPlaneZpos = zpos->clamp(lowestAcceptableZpos);
                 }
             }
             else {
@@ -866,7 +863,7 @@ bool DrmRenderer::initialize(PDECODER_PARAMETERS params)
                         continue;
                     }
                 }
-                else if (zpos->range().second <= m_VideoPlaneZpos) {
+                else if ((*zpos->range()).second <= m_VideoPlaneZpos) {
                     // This plane cannot be raised high enough to be visible
                     drmModeFreePlane(plane);
                     continue;

app/streaming/video/ffmpeg-renderers/drm.h

@@ -86,14 +86,22 @@ class DrmRenderer : public IFFmpegRenderer {
             return m_Prop->prop_id;
         }
 
-        std::pair<uint64_t, uint64_t> range() const {
+        std::optional<std::pair<uint64_t, uint64_t>> range() const {
             if ((m_Prop->flags & (DRM_MODE_PROP_RANGE | DRM_MODE_PROP_SIGNED_RANGE)) &&
                 m_Prop->count_values == 2) {
                 return std::make_pair(m_Prop->values[0], m_Prop->values[1]);
             }
             else {
-                SDL_assert(false);
+                return std::nullopt;
-                return std::make_pair(0, 0);
+            }
+        }
+
+        uint64_t clamp(uint64_t value) const {
+            if (auto range = this->range()) {
+                return std::clamp(value, range->first, range->second);
+            }
+            else {
+                return value;
             }
         }
 
@@ -659,10 +667,17 @@ class DrmRenderer : public IFFmpegRenderer {
             SDL_assert(m_Atomic);
 
             // Set all mutable properties back to their initial values
-            for (auto& prop : object.properties()) {
+            for (auto&[id, prop] : object.properties()) {
-                if (!prop.second.isImmutable()) {
+                restorePropertyToInitial(prop);
-                    set(prop.second, prop.second.initialValue());
+            }
-                }
+        }
+
+        void restorePropertyToInitial(const DrmProperty& prop) {
+            if (!prop.isImmutable()) {
+                // We clamp() here because some DRM drivers actually initialize certain
+                // properties (max bpc) to values outside the legal range. Obviously,
+                // the kernel rejects these values when we later restore them back.
+                set(prop, prop.clamp(prop.initialValue()));
             }
         }