Conversation
alalek
left a comment
There was a problem hiding this comment.
Awesome!
Could you please squash all commits into one?
modules/imgcodecs/src/loadsave.cpp
Outdated
| Size bufSize = buf.size(); | ||
| if( bufSize.width == 1 && buf.type() == CV_8UC1 ) | ||
| { | ||
| ByteStreamBuffer bsb( reinterpret_cast<char*>(buf.data), bufSize.height ); |
There was a problem hiding this comment.
What is about this? (do not assume exact Mat structure: column vector as 2D Mat)
if (buf.isContinuous())
{
ByteStreamBuffer bsb( buf.ptr<char*>(), buf.total() * bufSize.elemSize() );
...
}
There was a problem hiding this comment.
ahhh ok that makes sense 👍 i still want to check for CV_8UC1 though, right?
f9207d8 to
69daabc
Compare
|
The test that failed here doesn't seem related to this patch
Could you help me find what I might have broke? |
modules/imgcodecs/src/loadsave.cpp
Outdated
| if( buf.isContinuous() ) | ||
| { | ||
| ByteStreamBuffer bsb( reinterpret_cast<char*>(buf.data), buf.total() * buf.elemSize() ); | ||
| std::istream stream( &bsb ); |
There was a problem hiding this comment.
Looks like the current ByteStreamBuffer implementation is buggy.
Please use this instead (with added #include <sstream> and removed ByteStreamBuffer):
std::istringstream stream(std::string(reinterpret_cast<char*>(buf.data), buf.total() * buf.elemSize()), std::ios_base::in);
(it does extra memcpy operation, but it is not critical for I/O tasks)
There was a problem hiding this comment.
i'm not sure this is what caused build to break - it passed with a nearly identical implementation
|
edit: nevermind, didnt see your reply |
|
@alalek Do you feel pretty confident that's what caused it to break? The build did pass on a previous commit, though with the squash the changes have been hidden. IIRC the only things I changed were that I added an additional bounds check for edit: anyway if you are confident then i'll change it, but i don't want to go on a goose chase here :) |
|
Stack trace for It is called from "MotionJpegCapture" in this test. Crash backtrace (with log messages): The similar logs messages are on buildbot (search for "Failure" keyword). |
|
Ah, that helps a lot. I don't see stacktraces in the buildbot output. Maybe I don't have enough permissions? All I see is which didn't really tell me anything :) |
320eb96 to
703295f
Compare
this builds on opencv#8492 and opencv#8511 to add a new method, .orientation(), to ImageDecoder. The JPEG subclass now overrides this to do jpeg-specific exif tag reading we also now export a method, OrientationTransform, which uses this orientation value to do the actual matrix operations
|
|
|
@alalek If you look at ExifReader.cpp, you can see that the only seeking it does (with |
|
@alalek Also, for what it's worth, |
703295f to
4bfedba
Compare
|
@alalek I've pushed a new version which will check the result of |
modules/imgcodecs/src/loadsave.cpp
Outdated
| { | ||
| return -1; | ||
| } | ||
| gbump( static_cast<int>(off) ); |
There was a problem hiding this comment.
"int" type limitation can be dropped (with INT_MAX/INT_MIN checks above):
setg(eback(), gptr() + off, egptr());
There was a problem hiding this comment.
it can't be dropped because the windows build complains about silently converting a long to an int
There was a problem hiding this comment.
I mean this:
- gbump( static_cast<int>(off) );
+ setg(eback(), gptr() + off, egptr());There is no conversion to "int".
f6cd087 to
d2fad10
Compare
d2fad10 to
8b80441
Compare
|
@alalek Thanks for all your help so far 👍 I know it's been kind of a struggle, heh Let me know if there's anything else I can do to help this PR along |
|
👍 |
…mory autorotate in-memory jpegs (opencv#8492) * autorotate in-memory jpegs * correct indentation (4 spaces) * imgcodecs: don't apply EXIF rotation for unloaded images * videoio: don't try to rotate MJPEG stream * imgcodecs: ByteStreamBuffer::seekoff support all seek "dir" * imgcodecs: fix condition: "off == egptr() - eback()" is valid offset
|
This bug is fixed after which version? |
resolves #8172
This pull request changes ExifReader to operate on istreams rather than filenames. It then builds a streambuf on top of Mat so that imdecode may use ApplyExifOrientation like imread does.