Skip to content

fix(heif): Fix incorrect tracking of current subimage#5166

Open
lgritz wants to merge 1 commit intoAcademySoftwareFoundation:mainfrom
lgritz:lg-heifoverflow
Open

fix(heif): Fix incorrect tracking of current subimage#5166
lgritz wants to merge 1 commit intoAcademySoftwareFoundation:mainfrom
lgritz:lg-heifoverflow

Conversation

@lgritz
Copy link
Copy Markdown
Collaborator

@lgritz lgritz commented Apr 25, 2026

Oh dear, somehow our heif reader never correctly overloaded the current_subimage method, instead inheriting the one from the base class that always returns 0. That's very bad for a file type that supports multiple subimages.

Along the way, simplified how we deal with the fact that the "primary" subimage doesn't need to be first in the file, but we always want to present it as if it were first. There was a point where I thought that was related to the bug so I was rewriting it; it turned out to not be the problem, but I'd still like to keep the cleanup I had done.

Oh dear, somehow our heif reader never correctly overloaded the
current_subimage method, instead inheriting the one from the base
class that always returns 0. That's very bad for a file type that
supports multipe subimages.

Along the way, simplified how we deal with the fact that the "primary"
subimage doesn't need to be first in the file, but we always want to
present it as if it were first. There was a point where I thought that
was related to the bug so I was rewriting it; it turned out to not be
the problem, but I'd still like to keep the cleanup I had done.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
m_item_ids.erase(m_item_ids.begin() + i);
break;
}
m_item_ids.resize(0);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Perhaps using
m_item_ids = {} would be a bit clearer to the intent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants