Fix #148 (#149)

* Fix #148

* Safer size check & integer size calculations

* Added a few test images

The blank test image was generated with
    convert -size 800x280 -sampling-factor 2x2 xc:white tests/reftest/images/blank_800x280.jpg

This creates a grayscale image with a sampling factor of 2 and a height that is a multiple of 8 but not 16,
so that it triggers #148.

* Add more test images

The images were generated with imagemagick to match variants of bug #148

    convert -size 16x24 -sampling-factor 2x2 -colorspace gray xc: +noise Random grayscale_16x24_sampling2x2.png
    convert grayscale_16x24_sampling2x2.png -quality 100 -sampling-factor 2x2 grayscale_16x24_sampling2x2.jpg

The idea is that even though they are currently treated the same,
images with a width that is an integer number of blocks are not the same
as images for which the height is an integer number of blocks.

If only the height is not an integer number of blocks, more
optimizations can be applied. This will be the subject of another pull
request.

Author: lovasoa

src/decoder.rs

@@ -776,22 +776,25 @@ fn compute_image(components: &[Component],
 
     if components.len() == 1 {
         let component = &components[0];
-
-        if component.size.width % 8 == 0 && component.size.height % 8 == 0 {
-            return Ok(data[0].clone())
-        }
+        let decoded = &data[0];
 
         let width = component.size.width as usize;
         let height = component.size.height as usize;
+        let size = width * height;
+
+        // if the image size is a multiple of the block size
+        if decoded.len() == size {
+            return Ok(decoded.to_vec())
+        }
 
-        let mut buffer = vec![0u8; width * height];
+        let mut buffer = vec![0u8; size];
         let line_stride = component.block_size.width as usize * component.dct_scale;
 
-        for y in 0 .. height {
+        for y in 0..height {
             let destination_idx = y * width;
             let source_idx = y * line_stride;
             let destination = &mut buffer[destination_idx..][..width];
-            let source = &data[0][source_idx..][..width];
+            let source = &decoded[source_idx..][..width];
             destination.copy_from_slice(source);
         }
 

src/parser.rs

@@ -240,18 +240,24 @@ pub fn parse_sof<R: Read>(reader: &mut R, marker: Marker) -> Result<FrameInfo> {
     })
 }
 
+/// Returns ceil(x/y), requires x>0
+fn ceil_div(x: u32, y: u32) -> u16 {
+    assert!(x>0 && y>0, "invalid dimensions");
+    (1 + ((x - 1) / y)) as u16
+}
+
 fn update_component_sizes(size: Dimensions, components: &mut [Component]) -> Dimensions {
-    let h_max = components.iter().map(|c| c.horizontal_sampling_factor).max().unwrap();
-    let v_max = components.iter().map(|c| c.vertical_sampling_factor).max().unwrap();
+    let h_max = components.iter().map(|c| c.horizontal_sampling_factor).max().unwrap() as u32;
+    let v_max = components.iter().map(|c| c.vertical_sampling_factor).max().unwrap() as u32;
 
     let mcu_size = Dimensions {
-        width: (size.width as f32 / (h_max as f32 * 8.0)).ceil() as u16,
-        height: (size.height as f32 / (v_max as f32 * 8.0)).ceil() as u16,
+        width: ceil_div(size.width as u32, h_max * 8),
+        height: ceil_div(size.height as u32, v_max * 8),
     };
 
     for component in components {
-        component.size.width = (size.width as f32 * component.horizontal_sampling_factor as f32 * component.dct_scale as f32 / (h_max as f32 * 8.0)).ceil() as u16;
-        component.size.height = (size.height as f32 * component.vertical_sampling_factor as f32 * component.dct_scale as f32 / (v_max as f32 * 8.0)).ceil() as u16;
+        component.size.width = ceil_div(size.width as u32 * component.horizontal_sampling_factor as u32 * component.dct_scale as u32, h_max * 8);
+        component.size.height = ceil_div(size.height as u32 * component.vertical_sampling_factor as u32 * component.dct_scale as u32, v_max * 8);
 
         component.block_size.width = mcu_size.width * component.horizontal_sampling_factor as u16;
         component.block_size.height = mcu_size.height * component.vertical_sampling_factor as u16;
@@ -260,6 +266,25 @@ fn update_component_sizes(size: Dimensions, components: &mut [Component]) -> Dim
     mcu_size
 }
 
+#[test]
+fn test_update_component_sizes() {
+    let mut components = [Component {
+        identifier: 1,
+        horizontal_sampling_factor: 2,
+        vertical_sampling_factor: 2,
+        quantization_table_index: 0,
+        dct_scale: 8,
+        size: Dimensions { width: 0, height: 0 },
+        block_size: Dimensions { width: 0, height: 0 },
+    }];
+    let mcu = update_component_sizes(
+        Dimensions { width: 800, height: 280 },
+        &mut components);
+    assert_eq!(mcu, Dimensions { width: 50, height: 18 });
+    assert_eq!(components[0].block_size, Dimensions { width: 100, height: 36 });
+    assert_eq!(components[0].size, Dimensions { width: 800, height: 280 });
+}
+
 // Section B.2.3
 pub fn parse_sos<R: Read>(reader: &mut R, frame: &FrameInfo) -> Result<ScanInfo> {
     let length = read_length(reader, SOS)?;