From f8543249f0e96a3148f3ea888bfa62882e67f523 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 29 Oct 2020 22:34:44 -0400 Subject: [PATCH 1/5] vp9: Make some member functions internally linked These helper functions don't directly modify any member state and can be hidden from view. --- src/video_core/command_classes/codecs/vp9.cpp | 102 +++++++++--------- src/video_core/command_classes/codecs/vp9.h | 10 -- 2 files changed, 54 insertions(+), 58 deletions(-) diff --git a/src/video_core/command_classes/codecs/vp9.cpp b/src/video_core/command_classes/codecs/vp9.cpp index b3e98aa9f2..aeb9866ded 100644 --- a/src/video_core/command_classes/codecs/vp9.cpp +++ b/src/video_core/command_classes/codecs/vp9.cpp @@ -197,6 +197,60 @@ constexpr std::array map_lut{ 230, 231, 232, 233, 234, 235, 236, 237, 238, 239, 240, 241, 18, 242, 243, 244, 245, 246, 247, 248, 249, 250, 251, 252, 253, 19, }; + +// 6.2.14 Tile size calculation + +s32 CalcMinLog2TileCols(s32 frame_width) { + const s32 sb64_cols = (frame_width + 63) / 64; + s32 min_log2 = 0; + + while ((64 << min_log2) < sb64_cols) { + min_log2++; + } + + return min_log2; +} + +s32 CalcMaxLog2TileCols(s32 frame_width) { + const s32 sb64_cols = (frame_width + 63) / 64; + s32 max_log2 = 1; + + while ((sb64_cols >> max_log2) >= 4) { + max_log2++; + } + + return max_log2 - 1; +} + +// Recenters probability. Based on section 6.3.6 of VP9 Specification +s32 RecenterNonNeg(s32 new_prob, s32 old_prob) { + if (new_prob > old_prob * 2) { + return new_prob; + } + + if (new_prob >= old_prob) { + return (new_prob - old_prob) * 2; + } + + return (old_prob - new_prob) * 2 - 1; +} + +// Adjusts old_prob depending on new_prob. Based on section 6.3.5 of VP9 Specification +s32 RemapProbability(s32 new_prob, s32 old_prob) { + new_prob--; + old_prob--; + + std::size_t index{}; + + if (old_prob * 2 <= 0xff) { + index = static_cast(std::max(0, RecenterNonNeg(new_prob, old_prob) - 1)); + } else { + index = static_cast( + std::max(0, RecenterNonNeg(0xff - 1 - new_prob, 0xff - 1 - old_prob) - 1)); + } + + return map_lut[index]; +} } // Anonymous namespace VP9::VP9(GPU& gpu) : gpu(gpu) {} @@ -236,32 +290,6 @@ void VP9::WriteProbabilityDelta(VpxRangeEncoder& writer, u8 new_prob, u8 old_pro EncodeTermSubExp(writer, delta); } -s32 VP9::RemapProbability(s32 new_prob, s32 old_prob) { - new_prob--; - old_prob--; - - std::size_t index{}; - - if (old_prob * 2 <= 0xff) { - index = static_cast(std::max(0, RecenterNonNeg(new_prob, old_prob) - 1)); - } else { - index = static_cast( - std::max(0, RecenterNonNeg(0xff - 1 - new_prob, 0xff - 1 - old_prob) - 1)); - } - - return map_lut[index]; -} - -s32 VP9::RecenterNonNeg(s32 new_prob, s32 old_prob) { - if (new_prob > old_prob * 2) { - return new_prob; - } else if (new_prob >= old_prob) { - return (new_prob - old_prob) * 2; - } else { - return (old_prob - new_prob) * 2 - 1; - } -} - void VP9::EncodeTermSubExp(VpxRangeEncoder& writer, s32 value) { if (WriteLessThan(writer, value, 16)) { writer.Write(value, 4); @@ -361,28 +389,6 @@ void VP9::WriteMvProbabilityUpdate(VpxRangeEncoder& writer, u8 new_prob, u8 old_ } } -s32 VP9::CalcMinLog2TileCols(s32 frame_width) { - const s32 sb64_cols = (frame_width + 63) / 64; - s32 min_log2 = 0; - - while ((64 << min_log2) < sb64_cols) { - min_log2++; - } - - return min_log2; -} - -s32 VP9::CalcMaxLog2TileCols(s32 frameWidth) { - const s32 sb64_cols = (frameWidth + 63) / 64; - s32 max_log2 = 1; - - while ((sb64_cols >> max_log2) >= 4) { - max_log2++; - } - - return max_log2 - 1; -} - Vp9PictureInfo VP9::GetVp9PictureInfo(const NvdecCommon::NvdecRegisters& state) { PictureInfo picture_info{}; gpu.MemoryManager().ReadBlock(state.picture_info_offset, &picture_info, sizeof(PictureInfo)); diff --git a/src/video_core/command_classes/codecs/vp9.h b/src/video_core/command_classes/codecs/vp9.h index dc52ddbde3..3826f2c955 100644 --- a/src/video_core/command_classes/codecs/vp9.h +++ b/src/video_core/command_classes/codecs/vp9.h @@ -121,12 +121,6 @@ private: /// Generates compressed header probability deltas in the bitstream writer void WriteProbabilityDelta(VpxRangeEncoder& writer, u8 new_prob, u8 old_prob); - /// Adjusts old_prob depending on new_prob. Based on section 6.3.5 of VP9 Specification - s32 RemapProbability(s32 new_prob, s32 old_prob); - - /// Recenters probability. Based on section 6.3.6 of VP9 Specification - s32 RecenterNonNeg(s32 new_prob, s32 old_prob); - /// Inverse of 6.3.4 Decode term subexp void EncodeTermSubExp(VpxRangeEncoder& writer, s32 value); @@ -146,10 +140,6 @@ private: /// Write motion vector probability updates. 6.3.17 in the spec void WriteMvProbabilityUpdate(VpxRangeEncoder& writer, u8 new_prob, u8 old_prob); - /// 6.2.14 Tile size calculation - s32 CalcMinLog2TileCols(s32 frame_width); - s32 CalcMaxLog2TileCols(s32 frame_width); - /// Returns VP9 information from NVDEC provided offset and size Vp9PictureInfo GetVp9PictureInfo(const NvdecCommon::NvdecRegisters& state); From badea3b30134b02c6502c8174719f2c984e37524 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 29 Oct 2020 22:35:52 -0400 Subject: [PATCH 2/5] vp9: Provide a default initializer for "hidden" member The API of VP9 exposes a WasFrameHidden() function which accesses this member. Given the constructor previously didn't initialize this member, it's a potential vector for an uninitialized read. Instead, we can initialize this to a deterministic value to prevent that from occurring. --- src/video_core/command_classes/codecs/vp9.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/video_core/command_classes/codecs/vp9.h b/src/video_core/command_classes/codecs/vp9.h index 3826f2c955..94e8f90902 100644 --- a/src/video_core/command_classes/codecs/vp9.h +++ b/src/video_core/command_classes/codecs/vp9.h @@ -159,7 +159,7 @@ private: std::array loop_filter_ref_deltas{}; std::array loop_filter_mode_deltas{}; - bool hidden; + bool hidden = false; s64 current_frame_number = -2; // since we buffer 2 frames s32 grace_period = 6; // frame offsets need to stabilize std::array frame_ctxs{}; From 0d713cf8eb8c8b8802584c73b83d5ca9d88c70b2 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 29 Oct 2020 22:40:46 -0400 Subject: [PATCH 3/5] vp9: Mark functions with [[nodiscard]] where applicable Prevents values from mistakenly being discarded in cases where it's a bug to do so. --- src/video_core/command_classes/codecs/vp9.cpp | 8 ++++---- src/video_core/command_classes/codecs/vp9.h | 18 +++++++++--------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/video_core/command_classes/codecs/vp9.cpp b/src/video_core/command_classes/codecs/vp9.cpp index aeb9866ded..42520f856f 100644 --- a/src/video_core/command_classes/codecs/vp9.cpp +++ b/src/video_core/command_classes/codecs/vp9.cpp @@ -200,7 +200,7 @@ constexpr std::array map_lut{ // 6.2.14 Tile size calculation -s32 CalcMinLog2TileCols(s32 frame_width) { +[[nodiscard]] s32 CalcMinLog2TileCols(s32 frame_width) { const s32 sb64_cols = (frame_width + 63) / 64; s32 min_log2 = 0; @@ -211,7 +211,7 @@ s32 CalcMinLog2TileCols(s32 frame_width) { return min_log2; } -s32 CalcMaxLog2TileCols(s32 frame_width) { +[[nodiscard]] s32 CalcMaxLog2TileCols(s32 frame_width) { const s32 sb64_cols = (frame_width + 63) / 64; s32 max_log2 = 1; @@ -223,7 +223,7 @@ s32 CalcMaxLog2TileCols(s32 frame_width) { } // Recenters probability. Based on section 6.3.6 of VP9 Specification -s32 RecenterNonNeg(s32 new_prob, s32 old_prob) { +[[nodiscard]] s32 RecenterNonNeg(s32 new_prob, s32 old_prob) { if (new_prob > old_prob * 2) { return new_prob; } @@ -236,7 +236,7 @@ s32 RecenterNonNeg(s32 new_prob, s32 old_prob) { } // Adjusts old_prob depending on new_prob. Based on section 6.3.5 of VP9 Specification -s32 RemapProbability(s32 new_prob, s32 old_prob) { +[[nodiscard]] s32 RemapProbability(s32 new_prob, s32 old_prob) { new_prob--; old_prob--; diff --git a/src/video_core/command_classes/codecs/vp9.h b/src/video_core/command_classes/codecs/vp9.h index 94e8f90902..76b5a8283c 100644 --- a/src/video_core/command_classes/codecs/vp9.h +++ b/src/video_core/command_classes/codecs/vp9.h @@ -37,11 +37,11 @@ public: /// Signal the end of the bitstream void End(); - std::vector& GetBuffer() { + [[nodiscard]] std::vector& GetBuffer() { return base_stream.GetBuffer(); } - const std::vector& GetBuffer() const { + [[nodiscard]] const std::vector& GetBuffer() const { return base_stream.GetBuffer(); } @@ -75,10 +75,10 @@ public: void Flush(); /// Returns byte_array - std::vector& GetByteArray(); + [[nodiscard]] std::vector& GetByteArray(); /// Returns const byte_array - const std::vector& GetByteArray() const; + [[nodiscard]] const std::vector& GetByteArray() const; private: /// Write bit_count bits from value into buffer @@ -104,7 +104,7 @@ public: std::vector& ComposeFrameHeader(NvdecCommon::NvdecRegisters& state); /// Returns true if the most recent frame was a hidden frame. - bool WasFrameHidden() const { + [[nodiscard]] bool WasFrameHidden() const { return hidden; } @@ -141,17 +141,17 @@ private: void WriteMvProbabilityUpdate(VpxRangeEncoder& writer, u8 new_prob, u8 old_prob); /// Returns VP9 information from NVDEC provided offset and size - Vp9PictureInfo GetVp9PictureInfo(const NvdecCommon::NvdecRegisters& state); + [[nodiscard]] Vp9PictureInfo GetVp9PictureInfo(const NvdecCommon::NvdecRegisters& state); /// Read and convert NVDEC provided entropy probs to Vp9EntropyProbs struct void InsertEntropy(u64 offset, Vp9EntropyProbs& dst); /// Returns frame to be decoded after buffering - Vp9FrameContainer GetCurrentFrame(const NvdecCommon::NvdecRegisters& state); + [[nodiscard]] Vp9FrameContainer GetCurrentFrame(const NvdecCommon::NvdecRegisters& state); /// Use NVDEC providied information to compose the headers for the current frame - std::vector ComposeCompressedHeader(); - VpxBitStreamWriter ComposeUncompressedHeader(); + [[nodiscard]] std::vector ComposeCompressedHeader(); + [[nodiscard]] VpxBitStreamWriter ComposeUncompressedHeader(); GPU& gpu; std::vector frame; From 12eeffcb7c7d9d97ee55c96a760dd1f655c1d507 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 29 Oct 2020 22:45:33 -0400 Subject: [PATCH 4/5] vp9: Be explicit with copy and move operators It's deprecated in the language to autogenerate these if the destructor for a type is specified, so we can explicitly specify how we want these to be generated. --- src/video_core/command_classes/codecs/vp9.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/video_core/command_classes/codecs/vp9.h b/src/video_core/command_classes/codecs/vp9.h index 76b5a8283c..05c9682fa5 100644 --- a/src/video_core/command_classes/codecs/vp9.h +++ b/src/video_core/command_classes/codecs/vp9.h @@ -25,6 +25,12 @@ public: VpxRangeEncoder(); ~VpxRangeEncoder(); + VpxRangeEncoder(const VpxRangeEncoder&) = delete; + VpxRangeEncoder& operator=(const VpxRangeEncoder&) = delete; + + VpxRangeEncoder(VpxRangeEncoder&&) = default; + VpxRangeEncoder& operator=(VpxRangeEncoder&&) = default; + /// Writes the rightmost value_size bits from value into the stream void Write(s32 value, s32 value_size); @@ -59,6 +65,12 @@ public: VpxBitStreamWriter(); ~VpxBitStreamWriter(); + VpxBitStreamWriter(const VpxBitStreamWriter&) = delete; + VpxBitStreamWriter& operator=(const VpxBitStreamWriter&) = delete; + + VpxBitStreamWriter(VpxBitStreamWriter&&) = default; + VpxBitStreamWriter& operator=(VpxBitStreamWriter&&) = default; + /// Write an unsigned integer value void WriteU(u32 value, u32 value_size); @@ -99,6 +111,12 @@ public: explicit VP9(GPU& gpu); ~VP9(); + VP9(const VP9&) = delete; + VP9& operator=(const VP9&) = delete; + + VP9(VP9&&) = default; + VP9& operator=(VP9&&) = delete; + /// Composes the VP9 frame from the GPU state information. Based on the official VP9 spec /// documentation std::vector& ComposeFrameHeader(NvdecCommon::NvdecRegisters& state); From 8049b8beb625686a4edd9fd1bdf133496e6f462c Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 29 Oct 2020 22:55:56 -0400 Subject: [PATCH 5/5] common/stream: Be explicit with copy and move operators --- src/common/stream.h | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/common/stream.h b/src/common/stream.h index 2585c16af9..0e40692de1 100644 --- a/src/common/stream.h +++ b/src/common/stream.h @@ -21,6 +21,12 @@ public: explicit Stream(); ~Stream(); + Stream(const Stream&) = delete; + Stream& operator=(const Stream&) = delete; + + Stream(Stream&&) = default; + Stream& operator=(Stream&&) = default; + /// Reposition bitstream "cursor" to the specified offset from origin void Seek(s32 offset, SeekOrigin origin); @@ -30,15 +36,15 @@ public: /// Writes byte at current position void WriteByte(u8 byte); - std::size_t GetPosition() const { + [[nodiscard]] std::size_t GetPosition() const { return position; } - std::vector& GetBuffer() { + [[nodiscard]] std::vector& GetBuffer() { return buffer; } - const std::vector& GetBuffer() const { + [[nodiscard]] const std::vector& GetBuffer() const { return buffer; }