Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix special case of box size == 1 in jp2image.cpp #1537

Open
pydera opened this issue Apr 9, 2021 · 5 comments
Open

Fix special case of box size == 1 in jp2image.cpp #1537

pydera opened this issue Apr 9, 2021 · 5 comments
Assignees
Labels
imageHandler Anything related to specific ImageHandlers
Milestone

Comments

@pydera
Copy link
Collaborator

pydera commented Apr 9, 2021

// FIXME. Special case. the real box size is given in another place.

@clanmills clanmills added this to the v1.00 milestone Apr 9, 2021
@clanmills
Copy link
Collaborator

I've found a collection of JP2 test images here: https://github.com/openpreserve/jpylyzer-test-files_

I'm very pleased to say that both tvisitor and exiv2 read all the files without crashing.

I'm even more pleased to say that tvisitor is about 25 times faster than Exiv2, and reads every file without any error messages except is_codestream.jp2 which is currently unsupported as we have to wait for the Standard to be defined.

The code in src/bmffimage.cpp to respect box.length == 1 is list below. I know this code is used somewhere because I recall debugging it! Although I believe this code can be easily ported into jp2image.cpp, it's very desirable to have a test JP2 image that exercises the code when it has been added to jp2image.cpp.

    long BmffImage::boxHandler(std::ostream& out /* = std::cout*/ , Exiv2::PrintStructureOption option /* = kpsNone */,int depth /* =0 */)
    {
        long result  = (long)io_->size();
        long address = (long)io_->tell();
        // never visit a box twice!
        if ( depth == 0 ) visits_.clear();
        if (visits_.find(address) != visits_.end() || visits_.size() > visits_max_) {
            throw Error(kerCorruptedMetadata);
        }
        visits_.insert(address);

        bool bTrace    = option == kpsBasic || option == kpsRecursive ;

        BmffBoxHeader box = {0, 0};
        if (io_->read((byte*)&box, sizeof(box)) != sizeof(box))
            return result;

        box.length = getLong((byte*)&box.length, endian_);
        box.type = getLong((byte*)&box.type, endian_);
        bool bLF = true;

        if ( bTrace ) {
            bLF = true;
            out << indent(depth) << "Exiv2::BmffImage::boxHandler: " << toAscii(box.type)
                << Internal::stringFormat(" %8ld->%u ", address, box.length);
        }

        if (box.length == 1) {
            DataBuf data(8);
            io_->read(data.pData_, data.size_);
            result = (long) getULongLong(data.pData_, endian_);
            // sanity check
            if (result < 8 || result+address > (long)io_->size()) {
                result = (long)io_->size();
                box.length = result;
            } else {
                box.length = (long) (io_->size() - address);
            }
        }

        // read data in box and restore file position
        ....
    }

@clanmills
Copy link
Collaborator

More test images here: https://github.com/uclouvain/openjpeg-data

@clanmills
Copy link
Collaborator

The following files in my book use box.length == 1

svn://dev.exiv2.org/svn/team/book/files

2021-02-13-1929.heic
Canon.CR3
cr3.cr3
IMG1.HEIC
IMG_3578.HEIC
Photo-21-11-20.heic
Stonehenge.heic
VID_20180630_164713.mp4

@clanmills
Copy link
Collaborator

This issue and #1525 should be consider at the same time. I think the correct way forward is to remove jp2image.cpp from the code base and port jp2image::writeMetadata() into bmffimage.cpp.

We will only add writeMetadata() support for JP2 files. Other bmff files such as HEIC, AVIF, JPX and CR3 are read-only.

@clanmills clanmills added the imageHandler Anything related to specific ImageHandlers label Apr 11, 2021
@piponazo piponazo self-assigned this Mar 19, 2022
@piponazo
Copy link
Collaborator

I have been lately reading the JP2 standard and I would like to improve the support for such standard. I'll dig into this issue in the next days 😉

@kevinbackhouse kevinbackhouse modified the milestones: v0.28.0, Backlog Nov 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
imageHandler Anything related to specific ImageHandlers
Projects
None yet
Development

No branches or pull requests

4 participants