Merge pull request #26835 from sturkmen72:patch-4

sturkmen72 · web-flow · commit d4eed1c5aae6 · 2025-01-25T09:31:00.000+03:00
Corrections on bKGD chunk writing and reading in PNG #26835 ### Pull Request Readiness Checklist See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [x] The PR is proposed to the proper branch - [ ] There is a reference to the original bug report and related work - [ ] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [ ] The feature is well documented and sample code can be built with the project CMake
diff --git a/modules/imgcodecs/src/grfmt_png.cpp b/modules/imgcodecs/src/grfmt_png.cpp
@@ -327,11 +327,10 @@ bool  PngDecoder::readHeader()
         if (id == id_bKGD)
         {
             // The spec is actually more complex: https://www.libpng.org/pub/png/spec/1.2/PNG-Chunks.html#C.bKGD
-            int bgcolor = png_get_uint_32(&chunk.p[8]);
-            m_animation.bgcolor[3] = (bgcolor >> 24) & 0xFF;
-            m_animation.bgcolor[2] = (bgcolor >> 16) & 0xFF;
-            m_animation.bgcolor[1] = (bgcolor >> 8) & 0xFF;
-            m_animation.bgcolor[0] = bgcolor & 0xFF;
+            m_animation.bgcolor[0] = png_get_uint_16(&chunk.p[8]);
+            m_animation.bgcolor[1] = png_get_uint_16(&chunk.p[10]);
+            m_animation.bgcolor[2] = png_get_uint_16(&chunk.p[12]);
+            m_animation.bgcolor[3] = 0;
         }
 
         if (id == id_PLTE || id == id_tRNS)
@@ -721,11 +720,10 @@ uint32_t PngDecoder::read_chunk(Chunk& chunk)
         if (size != 8 + 26 + 4)
             return 0;
     } else if (id == id_bKGD) {
-        // 8=HDR+size, ??=size of bKGD chunk, 4=CRC
+        // 8=HDR+size, (1, 2 or 6)=size of bKGD chunk, 4=CRC
         // The spec is actually more complex:
         // https://www.libpng.org/pub/png/spec/1.2/PNG-Chunks.html#C.bKGD
-        // TODO: we only check that 4 bytes can be read from &chunk.p[8]. Fix.
-        if (size < 8 + 4)
+        if (size != 8 + 1 + 4 && size != 8 + 2 + 4 && size != 8 + 6 + 4)
             return 0;
     } else if (id != id_fdAT && id != id_IDAT && id != id_IEND && id != id_PLTE && id != id_tRNS) {
         if (size > PNG_USER_CHUNK_MALLOC_MAX)
@@ -1533,13 +1531,13 @@ bool PngEncoder::writeanimation(const Animation& animation, const std::vector<in
         if (palsize > 0)
             writeChunk(m_f, "PLTE", (unsigned char*)(&palette), palsize * 3);
 
-        if ((animation.bgcolor != Scalar()) && (animation.frames.size() > 1))
+        if ((animation.bgcolor != Scalar()) && coltype)
         {
-            uint64_t bgvalue = (static_cast<int>(animation.bgcolor[0]) & 0xFF) << 24 |
-                (static_cast<int>(animation.bgcolor[1]) & 0xFF) << 16 |
-                (static_cast<int>(animation.bgcolor[2]) & 0xFF) << 8 |
-                (static_cast<int>(animation.bgcolor[3]) & 0xFF);
-            writeChunk(m_f, "bKGD", (unsigned char*)(&bgvalue), 6); //the bKGD chunk must precede the first IDAT chunk, and must follow the PLTE chunk.
+            unsigned char bgvalue[6] = {};
+            bgvalue[1] = animation.bgcolor[0];
+            bgvalue[3] = animation.bgcolor[1];
+            bgvalue[5] = animation.bgcolor[2];
+            writeChunk(m_f, "bKGD", bgvalue, 6); //the bKGD chunk must precede the first IDAT chunk, and must follow the PLTE chunk.
         }
 
         if (trnssize > 0)
diff --git a/modules/imgcodecs/test/test_animation.cpp b/modules/imgcodecs/test/test_animation.cpp
@@ -425,6 +425,39 @@ TEST(Imgcodecs_APNG, imwriteanimation_rgb)
     EXPECT_EQ(0, remove(output.c_str()));
 }
 
+TEST(Imgcodecs_APNG, imwriteanimation_gray)
+{
+    Animation s_animation, l_animation;
+    EXPECT_TRUE(fillFrames(s_animation, false));
+
+    for (size_t i = 0; i < s_animation.frames.size(); i++)
+    {
+        cvtColor(s_animation.frames[i], s_animation.frames[i], COLOR_BGR2GRAY);
+    }
+
+    s_animation.bgcolor = Scalar(50, 100, 150);
+    string output = cv::tempfile(".png");
+    // Write the animation to a .png file and verify success.
+    EXPECT_TRUE(imwriteanimation(output, s_animation));
+
+    // Read the animation back and compare with the original.
+    EXPECT_TRUE(imreadanimation(output, l_animation));
+
+    EXPECT_EQ(Scalar(), l_animation.bgcolor);
+    size_t expected_frame_count = s_animation.frames.size() - 2;
+
+    // Verify that the number of frames matches the expected count.
+    EXPECT_EQ(expected_frame_count, imcount(output));
+    EXPECT_EQ(expected_frame_count, l_animation.frames.size());
+
+    EXPECT_EQ(0, remove(output.c_str()));
+
+    for (size_t i = 0; i < l_animation.frames.size(); i++)
+    {
+        EXPECT_EQ(0, cvtest::norm(s_animation.frames[i], l_animation.frames[i], NORM_INF));
+    }
+}
+
 TEST(Imgcodecs_APNG, imwritemulti_rgba)
 {
     Animation s_animation;
@@ -492,7 +525,7 @@ TEST(Imgcodecs_APNG, imwriteanimation_bgcolor)
 {
     Animation s_animation, l_animation;
     EXPECT_TRUE(fillFrames(s_animation, true, 2));
-    s_animation.bgcolor = Scalar(50, 100, 150, 128); // different values for test purpose.
+    s_animation.bgcolor = Scalar(50, 100, 150); // will be written in bKGD chunk as RGB.
 
     // Create a temporary output filename for saving the animation.
     string output = cv::tempfile(".png");
