feat(decoder): add FFDC marker support#79
feat(decoder): add FFDC marker support#79patrickhulce merged 3 commits intojpeg-js:masterfrom Chupsy:master
Conversation
patrickhulce
left a comment
There was a problem hiding this comment.
thanks very much for the fix and a test case 🎉 😃
just one question on if numberOfLines is actually worth exposing
lib/decoder.js
Outdated
|
|
||
| case 0xFFDC: // Number of Lines marker | ||
| readUint16() // skip data length | ||
| this.numberOfLines = readUint16() |
There was a problem hiding this comment.
is this number ever different from the height of the image?
There was a problem hiding this comment.
actually it is a pretty worthless information, but I felt bad checking a data and ignoring it, I guess we can ignore it since this marker is deprecated
There was a problem hiding this comment.
Switched to just ignoring the marker, thank you for your review!
patrickhulce
left a comment
There was a problem hiding this comment.
LGTM after revert of the extra test changes :)
| @@ -1,141 +1,143 @@ | |||
| var fs = require('fs'), | |||
There was a problem hiding this comment.
revert all these changes?
There was a problem hiding this comment.
Oh right i'm sorry, format on save option... reverted now
|
many thanks @Chupsy! 🎉 |
Hi,
First of all, thank you for all you hard work on this project, easy to use and very clear to understand.
This PR aims to fix an issue where files using the FFDC marker were refused with the error "Unknown JPEG marker ffdc". Even though it is a rare marker, which simply gives another info of the height of the file, I added the handling and tests for this marker.
Thank you for your time reviewing this PR