IPP 5.3 JPEGView Fails On Several JPEGs

IPP 5.3 JPEGView Fails On Several JPEGs

The JPEG code in IPP fails on these JPEGS, all valid and loadable by other programs. We have fixed or rewritten the Intel code for the first two, but are stumped by the last one.

ftp://ftp.jriver.com/pub/downloads/music/tmp/ipp_fail_1.jpg
Intel code tries to seek to a negative byte offset -- I can provide the source
code fix for the Intel code if you like

ftp://ftp.jriver.com/pub/downloads/music/tmp/ipp_fail_2.jpg
Intel ippiYCbCrToBGR_JPEG_8u_P3C3R function does not work reliably on boundary
cases and creates an artifact in the lower right box -- we rewrote the color space converter with our own MMX assembly

ftp://ftp.jriver.com/pub/downloads/music/tmp/ipp_fail_3.jpg
This produces a colored box in the lower right corner. I think the DCT value is incorrect, but I can't find the problem. Any help would be greatly appreciated.

All of these fail with JPEGView and with our production code (statically linked, running on Windows).

Thank you for any support.

25 posts / 0 new
Last post
For more complete information about compiler optimizations, see our Optimization Notice.

Hello,

thanks for reporting about that, we will investigate the issue and come back to you soon.

Regards,
Vladimir

Thanks Vladimir.

We would love a solution for the third image, as we switched from IJL to IPP (and purchased IPP) in a production product. Weassumed it would be trouble free since IJL was sooutstanding.

So far it's been a little bumpy.

Thank you.

Yes, there were some mistakes.

theissue with your first file was due to wrong implementation of Seek method in BitStreamInput class. It is better to rewrite the case UIC_SEEK_CUR as:

case UIC_SEEK_CUR:

{

int _offset = m_currPos + offset;

if(_offset > 0 && _offset < m_DataLen)

{

m_currPos = _offset;

}

else

{

m_currPos = m_DataLen;

jerr = m_in->Seek(_offset - m_DataLen,UIC_SEEK_CUR);

if(JPEG_OK != jerr)

return jerr;

jerr = FillBuffer();

if(JPEG_OK != jerr)

return jerr;

}

break;

}

And issue with two other files was in piece of code around line 3150 in jpegdec.cpp file. It is not needed to check return code from FillBitBuffer() call, as it may return error near to the end of file while there might be enough bits prefetched in HuffmanState structure. In case of truncated file you can check return code from ippiDecodeHuffman8x8 function. So, I recommend to rewrite the piece of code

for(k = 0; k < m_ccomp[n].m_vsampling; k++)

{

for(l = 0; l < m_ccomp[n].m_hsampling; l++)

{

jerr = m_BitStreamIn.FillBuffer(SAFE_NBYTES);

if(JPEG_OK != jerr)

return jerr;

currPos = m_BitStreamIn.GetCurrPos();

status = ippiDecodeHuffman8x8_JPEG_1u16s_C1(

src,srcLen,&currPos,pMCUBuf,lastDC,(int*)&m_marker,

dctbl,actbl,m_state);

as

for(k = 0; k < m_ccomp[n].m_vsampling; k++)

{

for(l = 0; l < m_ccomp[n].m_hsampling; l++)

{

m_BitStreamIn.FillBuffer(SAFE_NBYTES);

currPos = m_BitStreamIn.GetCurrPos();

status = ippiDecodeHuffman8x8_JPEG_1u16s_C1(

src,srcLen,&currPos,pMCUBuf,lastDC,(int*)&m_marker,

dctbl,actbl,m_state);

Actually it was written in such a way in previous version of IPP JPEG codec but suddenly it was changed in wrong way.

Regards,
Vladimir

Thank you very much. Both fixes worked well. The first was the change we'd already made.

You might also want to check for < 0 in CMemBuffInput::Seek since it doesn't expect or check against negative offset values.

I would have never found the second bug. Nice work on that one.

One more issue:

ftp://ftp.jriver.com/pub/downloads/music/tmp/ipp_fail_4.jpg

(I posted the wrong link for a second -- make sure you try image 4)

When loadingthe image above (and many other thumbnail size images) with down-sampling (i.e. 1:4), the far right border of pixels will get random (uninitialized memory?) colors.

Use JPGView to load the image above at 1:4. You may need to take a screenshot and zoom in to see that the right border is incorrect.

One workaround would be for us to trim the far right column, but I'd prefer not to have to.

Thanks for any ideas.

Yes, CMemBuffInput should be corrected as well.

Regarding your issue I've found that memory buffer for utput image was not aligned according BMP requirements. Itshould easy to fix, please modify call of image->Alloc() function in file jpeg.cpp from

r = image->Alloc(roi,nchannels,precision);

if(0 != r)

{

return JPEG_ERR_ALLOC;

}

to

r = image->Alloc(roi,nchannels,precision,1);

if(0 != r)

{

return JPEG_ERR_ALLOC;

}

Regards,
Vladimir

When will these corrections make it into the official ipp_samples distribution?

Thanks,
Phil

Hi Vladimir,

I'm afraid the last problem isn't so simple.

If you load this next image at 1:4, you'll see the right-most row is also corrupt even with your alignment change:

ftp://ftp.jriver.com/pub/downloads/music/tmp/ipp_fail_5.jpg

However, it is sporadic, so the state of the system memory or timing may play a role.

In our own code, the Cb and Cr components for the last column are incorrect -- this is before we even think about the output bitmap. That means it's something in the decoder.

The Y component is correct, so it seems likea boundary condition coming from 4:1:1 to 1:4.

Since it's sporadic, I hope you'll be able to reproduce it.

Thanks.

Here's a clue:

If we reset the SS buffer at the top of DecodeScanBaselineIN before reconstructing the MCU rows, the issue goes away:

Here's code added to CJPEGDecoder::DecodeScanBaselineIN:

for(int c = 0; c < m_jpeg_ncomp; c++)
{
CJPEGColorComponent* curr_comp = &m_ccomp[c];
memset(curr_comp->GetSSBufferPtr(idThread), 128, curr_comp->m_ss_bufsize);
}

That seems to indicate that ReconstructMCURowBL8x8To2x2 is either not setting the entire buffer, or that a later stage is reading too much of the buffer.

What is the right way to address this?

Thanks.

It turns out initializing the SS buffer during allocation time masks the problem. It still seems like something in the decoder is reading further ahead than it should:

So, switch:

JERRCODE CJPEGColorComponent::CreateBufferSS(int num_threads)

{

m_ss_bufsize = m_ss_step * m_ss_height;

return m_ss_buf.Allocate(m_ss_bufsize * num_threads);

} // CJPEGColorComponent::CreateBufferSS()

---- to ----

JERRCODE CJPEGColorComponent::CreateBufferSS(int num_threads)

{

m_ss_bufsize = m_ss_step * m_ss_height;

JERRCODE jerr = m_ss_buf.Allocate(m_ss_bufsize * num_threads);

if(jerr == JPEG_OK)

{

memset(m_ss_buf, 128, m_ss_bufsize * num_threads);

}

return jerr;

} // CJPEGColorComponent::CreateBufferSS()

Hi Phil,

Yes I detected that also. We are working on that issue. Actually there should be IPP 5.3 update soon, so I hope we will be in time with fixes, so it can be released earlier. If not, the next IPP version is IPP 6.0 beta which is also should come sometime in 1H'08.

Regards,
Vladimir

We're shipping built on top of the IPP code _now_, so I want to make sure the fix I posted is the correct way to do it.

Thanks for any advice.

Hello,

please find corrected jpegview sample in attachment of this post. These corrections will also go to IPP 5.3 update 2 which should be available soon.

Please note, that now you need to round image size when you calculate size of thumbnail for 1:2, 1:4 or 1:8 decoding mode. The decoder will rely on that. You may apply image bigger than that size but not less.

basically, you will use in jpeg.cpp, ReadImageFromJPEG code like that:

 dd_factor = 1;
 if(JPEG_BASELINE == param->mode || JPEG_PROGRESSIVE == param->mode)
{
switch(param->dct_scale)
{
default:
case JD_1_1:
{
decoder.SetDCTType(param->use_qdct);
}
break;
 case JD_1_2:
roi.width = (roi.width+1) >> 1;
roi.height = (roi.height+1) >> 1;
dd_factor = 2;
break;
 case JD_1_4:
roi.width = (roi.width+3) >> 2;
roi.height = (roi.height+3) >> 2;
dd_factor = 4;
break;
 case JD_1_8:
roi.width = (roi.width+7) >> 3;
roi.height = (roi.height+7) >> 3;
dd_factor = 8;
break;
}
}

Regards,
Vladimir

Attachments: 

AttachmentSize
Download jpegview.zip2.38 MB

Thanks for all of your help.

I've compiled against the latest code and so far things appear to be squared away.

We'll release it for testing soon, and let you know if anything elsepops up.

Thanks again.

Thanks, I hope it will run well. Please feel free to contact if you will have any questions related to that sample.

Regards,
Vladimir

Hi,

There is another jpg, this one is causing an exception in jpegdec.cpp:

ftp://ftp.jriver.com/pub/downloads/music/tmp/ipp_fail_1.jpg

The exception usually occurs in the function call:

ippsZero_16s(pMCUBuf,m_numxMCU * m_nblock * DCTSIZE2);

which is in the function CJPEGDecoder::DecodeScanBaselineIN(void), but only when ippsZero_16p is called on the second block that is using DecodeScanBaselineIN. A second exception occurs on cleanup for the first exception, and it occurs ippFree(m_block_buffer); once in a while only ippFree gets the exception, so there appears to be a memory curruption that somehow involving m_block_buffer.

Interestingly, when ippsZero_16s is replaced with a memset, the exception does not occur at all and the image loads properly, though this might be just a fluke. The jpg listed above is the only one known to cause an exception. Windows picture & pictureviewer say the jpeg is currupted, firefox can load it.

It doesn't matter if the picture loads or fails, only that an exception doesn't occur.

Thanks

Bump. Any help would be appreciated as the latest Intel JPEG code is crashing in a release product for us.

Thanks.

Hello,

I'm sorry for the delay related to vacation of our expert. We are investigating that issue, I'll update you with status of this issue soon. Thanks for your patience.

Regards,
Vladimir

Hello,

we've found the root couse of that issue. In your file you have concatenated two jpeg images (field coding for motion jpeg?) but the first JPEG was not ended with EOI (end of image) marker. By JPEG standard each JPEG image should be bounded by SOI (start of image) and EOI (end of image) markers. So you need to add to your encoding procedure code which generate EOI marker after you finish encoding of your first image.

We will add explicit check for that case into future version of IPP JPEG codec.

Regards,
Vladimir

This file was not created by us. It's a file that a user had that caused our software to crash, because of the IJL problem.

Is there a change we could make to jpegdec.cpp to more gracefully handle this? Failing to load is fine, but hard-crashing the application is not.

Thanks.

Hello,

Please find attached archive with corrected files, you need to substitute corresponding files with these two in IPP 5.3 JPEG codec.

Regards,
Vladimir

Attachments: 

AttachmentSize
Download fix.zip16.49 KB

Thank you very much.

This appears to fix both this issue (crash on invalid JPEG) and the 411 scaling bug here:
/en-us/forums/showthread.php?t=59769

Thanks again!

You are welcome:)

hi dear how are u doing Douglas here 4 u

Hi Douglas,

we are just fine, working hard on the next version of IPP product.

Regards,
Vladimir

Leave a Comment

Please sign in to add a comment. Not a member? Join today