Fix jpeg validation to accept a broader range of jpeg files (BugFix)

[tomli380576]

Description

This PR relaxes the validation step for JPEGs to fix #1796 and prints some more debugging messages.

There was another issue where the resolutions subparser was missing the wait_seconds argument. It's fixed now.

Documentation

The check is now done by gst-typefind-1.0 filename.jpeg. If the return value doesn't contain "image/jpeg" then we will mark the file as broken.

Tests

Original unit tests

Real hardware without fswebcam:

• USB cameras that only produce video/x-raw: I couldn't find any :( but they are expected to work since jpegenc will be used to build the final photo.

• USB cameras that produces some formats in image/jpeg: 24.04 devices are mostly OK, but I found one 22.04 device that created an image with trailing 0s after ffd9: https://certification.canonical.com/hardware/202503-36385/ and a 20.04 device https://certification.canonical.com/hardware/202112-29820/. Image looks fine though and the file command says it's jpeg.

  ubuntu@ubuntu:~$ hexdump -C out.jpeg | tail -5
  0003f860  c4 8f 73 12 08 60 9c f1  f5 a5 9a 32 62 46 ec ac  |..s..`.....2bF..|
  0003f870  49 19 f5 a2 e1 71 44 4e  e0 10 33 9e e3 9a bf a4  |I....qDN..3.....|
  0003f880  b2 a5 dd b3 3b 24 60 37  24 b6 07 e3 e9 4f 98 77  |....;$`7$....O.w|
  0003f890  00 ff d9 00 00 00 00 00                           |........|
  0003f898
  

• MIPI cameras that uses v4l2loopback: OK, the photos produced by them all run through jpegenc so they are expected to pass

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.93%. Comparing base (5baa4d0) to head (e3c7798).
Report is 118 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1803      +/-   ##
==========================================
+ Coverage   49.83%   49.93%   +0.09%     
==========================================
  Files         377      378       +1     
  Lines       40719    40766      +47     
  Branches     6851     6859       +8     
==========================================
+ Hits        20294    20356      +62     
+ Misses      19700    19683      -17     
- Partials      725      727       +2     

Flag	Coverage Δ
provider-base	26.24% <100.00%> (∅)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tomli380576]

sry I'm really out of ideas on how to get that last few % of test coverage without an actual image for the validator 😿

[fernando79513]

I was checking at a clean solution for this but I didn't found one either.
I thought about two workarounds:

• Mock the return value of the check_output function so it returns "image/jpeg", and using an empty image and expect the test to return False and a message of "Image dimensions not found in JPEG file"

    @patch("os.path.exists")
    def test_validate_image_no_dimensions(self, mock_exists):
        mock_camera = MagicMock()
        mock_exists.return_value = True
        with patch("builtins.open", mock_open(read_data=b"")):
            with patch("builtins.print") as mocked_print, patch(
                "camera_test.check_output"
            ) as mock_check_output:
                mock_check_output.return_value = "image/jpeg"
                result = CameraTest._validate_image(
                    mock_camera, "/tmp/test.jpg", 480, 320
                )
                # should not even start reading the file if the `file` command
                # check didn't pass
                mocked_print.assert_any_call(
                    "Image dimensions not found in JPEG file"
                )
                self.assertEqual(result, False)

• Actually add a 1x1px image and check the result

    @patch("os.path.exists")
    @patch("camera_test.check_output")
    def test_validate_image_correct_jpeg_format(self, mock_output, mock_exists):
        mock_camera = MagicMock()
        mock_exists.return_value = True

        # Create a temporary file with a valid 1x1 JPEG image
        data = (
            "ffd8ffe000104a46494600010101004800480000fffe001343726561746564207"
            "76974682047494d50ffdb00430001010101010101010101010101010101010101"
            "01010101010101010101010101010101010101010101010101010101010101010"
            "1010101010101010101010101ffdb004301010101010101010101010101010101"
            "01010101010101010101010101010101010101010101010101010101010101010"
            "101010101010101010101010101010101ffc00011080001000103011100021101"
            "031101ffc4001400010000000000000000000000000000000bffc400141001000"
            "00000000000000000000000000000ffc400140101000000000000000000000000"
            "00000000ffc40014110100000000000000000000000000000000ffda000c03010"
            "002110311003f003ff07fffd9"
        )
        from tempfile import NamedTemporaryFile
        with NamedTemporaryFile(delete=False) as f:
            f.write(bytes.fromhex(data))
            f.seek(0)
            mock_open.return_value = f
            mock_output.return_value = "image/jpeg"
            result = CameraTest._validate_image(mock_camera, f.name, 1, 1)
            self.assertEqual(result, True)

Let me know if you think any of the two or both make sense

[tomli380576]

bad news file doesn't seem to exist on the stock ubuntu core image. can we safely ignore this? I think most of the core devices don't use the base provider's camera test I'll go ask people that regularly test ubuntu core images, we might be able to use gst-discoverer-1.0 as fallback

EDIT: we can use gst-typefind-1.0. I installed checkbox on a ubuntu core 16 VM in multipass and the command exists in checkbox.shell

[fernando79513]

Thank you very much for this fix, and also for being so thorough with the testing, I think these last changes could save us from some issues down the line.

LGTM +1!

[tomli380576]

Tysm for the help and review!