Skip to content

Commit

Permalink
Merge pull request #96 from noritada/fix/nbit-zero-handling-in-decoders
Browse files Browse the repository at this point in the history
Fix decoders handling cases where nbit is 0

This PR makes decoders correctly handle cases where nbit is 0.

Following two possible issues will be corrected by this PR:

- Simple packing and JPEG 2000 code stream format decoders have returned $R$ for all points when nbit was 0,
  although they should return $R * 10^{-D}$.
  I noticed this problem when I was reading the source code of wgrib2.
  Although I have went through every one of data files I had on hand,
  I have not encountered any data files where the problem actually occurs,
  since every message has had $D$ value of 0 when nbit is 0.

- PNG format decoder may possibly have crashed when nbit was 0.
  I have not encountered any data files where the problem actually occurs,
  since I only have PNG-format encoded data files whose nbit is 16.
  • Loading branch information
noritada committed Aug 11, 2024
2 parents 6feb81b + 5176bc2 commit 1a61e1a
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 9 deletions.
7 changes: 2 additions & 5 deletions src/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@ use crate::{
complex::ComplexPackingDecodeError,
png::PngDecodeError,
run_length::RunLengthEncodingDecodeError,
simple::{
SimplePackingDecodeError, SimplePackingDecodeIterator,
SimplePackingDecodeIteratorWrapper,
},
simple::{SimplePackingDecodeError, SimplePackingDecodeIteratorWrapper},
},
error::*,
reader::Grib2Read,
Expand Down Expand Up @@ -186,7 +183,7 @@ enum Grib2SubmessageDecoderIteratorWrapper<T0, T2, T3, T40, T41> {
Template40(PhantomData<T40>),
#[cfg(not(target_arch = "wasm32"))]
Template40(SimplePackingDecodeIteratorWrapper<T40>),
Template41(SimplePackingDecodeIterator<T41>),
Template41(SimplePackingDecodeIteratorWrapper<T41>),
Template200(std::vec::IntoIter<f32>),
}

Expand Down
2 changes: 1 addition & 1 deletion src/decoder/jpeg2000.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub(crate) fn decode(
Please report your data and help us develop the library."
);
let decoder = SimplePackingDecodeIteratorWrapper::FixedValue(FixedValueIterator::new(
simple_param.ref_val,
simple_param.zero_bit_reference_value(),
target.num_points_encoded,
));
return Ok(decoder);
Expand Down
4 changes: 4 additions & 0 deletions src/decoder/param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ impl SimplePackingParam {
nbit,
})
}

pub(crate) fn zero_bit_reference_value(&self) -> f32 {
self.ref_val * 10_f32.powi(-i32::from(self.dig))
}
}

pub(crate) struct ComplexPackingParam {
Expand Down
19 changes: 17 additions & 2 deletions src/decoder/png.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::{
decoder::{
param::SimplePackingParam, simple::SimplePackingDecodeIterator, stream::NBitwiseIterator,
param::SimplePackingParam,
simple::{SimplePackingDecodeIterator, SimplePackingDecodeIteratorWrapper},
stream::{FixedValueIterator, NBitwiseIterator},
},
DecodeError, Grib2SubmessageDecoder, GribError,
};
Expand All @@ -13,7 +15,7 @@ pub enum PngDecodeError {

pub(crate) fn decode(
target: &Grib2SubmessageDecoder,
) -> Result<SimplePackingDecodeIterator<impl Iterator<Item = u32> + '_>, GribError> {
) -> Result<SimplePackingDecodeIteratorWrapper<impl Iterator<Item = u32> + '_>, GribError> {
let sect5_data = &target.sect5_payload;
let param = SimplePackingParam::from_buf(&sect5_data[6..16])?;

Expand All @@ -23,6 +25,18 @@ pub(crate) fn decode(
)))
})?;

if param.nbit == 0 {
eprintln!(
"WARNING: nbit = 0 for PNG decoder is not tested.
Please report your data and help us develop the library."
);
let decoder = SimplePackingDecodeIteratorWrapper::FixedValue(FixedValueIterator::new(
param.zero_bit_reference_value(),
target.num_points_encoded,
));
return Ok(decoder);
};

if param.nbit != 16 {
eprintln!(
"WARNING: nbit != 16 for PNG decoder is not tested.
Expand All @@ -32,6 +46,7 @@ pub(crate) fn decode(

let iter = NBitwiseIterator::new(buf, usize::from(param.nbit));
let iter = SimplePackingDecodeIterator::new(iter, &param);
let iter = SimplePackingDecodeIteratorWrapper::SimplePacking(iter);
Ok(iter)
}

Expand Down
2 changes: 1 addition & 1 deletion src/decoder/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ pub(crate) fn decode(

let decoder = if param.nbit == 0 {
SimplePackingDecodeIteratorWrapper::FixedValue(FixedValueIterator::new(
param.ref_val,
param.zero_bit_reference_value(),
target.num_points_encoded,
))
} else {
Expand Down

0 comments on commit 1a61e1a

Please sign in to comment.