Fix: don’t fail if scan contains trailing bytes#66
Fix: don’t fail if scan contains trailing bytes#66patrickhulce merged 2 commits intojpeg-js:masterfrom
Conversation
patrickhulce
left a comment
There was a problem hiding this comment.
thanks very much @rluba!
| // Skip trailing bytes at the end of the scan - until we reach the next marker | ||
| do { | ||
| if (data[offset] === 0xFF) { | ||
| if (data[offset + 1] !== 0x00) { |
There was a problem hiding this comment.
mind walking me through this one? :)
if we're intending on being loose AFAICT we'd be fine just checking for data[offset] === 0xFF?
There was a problem hiding this comment.
According to the spec, any FF followed by 00 is not a marker. Since I don’t have enough examples to know what the heck encoders might use as fill bytes, I want to be sure that I’m not stopping before finding an actual marker.
There was a problem hiding this comment.
But the code could be more concise. It’s a leftover from having more conditions there before arriving at this version.
There was a problem hiding this comment.
it's definitely not a marker, but I was under the impression it was filler data. it is handled appropriately in the rest of jpeg-js fwiw
but we basically cycle through it just like we would here, so it wouldn't really have a practical effect other than consistency with the code below
There was a problem hiding this comment.
maybe we make a note of this conversation in a comment or change it so we don't wonder in the future why FF00 is a valid marker on line 336 but not here? :)
patrickhulce
left a comment
There was a problem hiding this comment.
comment wasn't a big deal, LGTM thanks!
Here's another PR for an issue I found: Some JPEGs contain trailing bytes at the end of a scan. All other jpeg programs and libraries I’ve tested (including the Indepdendend JPEG group’s decoder or stb_image) simply skip these bytes.
This PR changes the decoder to skip trailing bytes at the end of a scan instead of failing with a "maker was not found" error.