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

libheif: support for shared auxl alpha images #1148

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

bradh
Copy link
Contributor

@bradh bradh commented Mar 19, 2024

Initial work (for discussion / review) on support for shared alpha images.

Relates #1147

I've shown some commented out code that returns an error if the alpha requires scaling (which is valid in HEIF). Until we support that, the options are silently ignoring the alpha plane, or not supporting the image at all.

We do have some scaling code, but my attempt to apply it failed - its designed for an image, not an image plane, and the refactoring got away.

Fixes / suggestions appreciated.

@leo-barnes
Copy link

I see the same issue for cdsc:

        if (ref.header.get_short_type() == fourcc("cdsc")) {
          std::vector<uint32_t> refs = ref.to_item_ID;
          if (refs.size() != 1) {
            return Error(heif_error_Invalid_input,
                         heif_suberror_Unspecified,
                         "Metadata not correctly assigned to image");
          }

Presumably the same holds for thmb references.

@bradh bradh marked this pull request as draft March 20, 2024 10:26
@bradh
Copy link
Contributor Author

bradh commented Mar 20, 2024

I see the same issue for cdsc:

Yes (and a bit lower for the region case too). Not too hard to fix.

Presumably the same holds for thmb references.

This (and depth images) will require a bit more refactoring.

@bradh bradh force-pushed the shared_iref_2024-03-19 branch from 26dbaf3 to 8cfeb40 Compare April 1, 2024 09:52
@bradh
Copy link
Contributor Author

bradh commented Apr 1, 2024

Looks like I need to address the fuzzing issue....

@bradh bradh force-pushed the shared_iref_2024-03-19 branch from 8cfeb40 to 9138f32 Compare April 2, 2024 06:13
@bradh bradh marked this pull request as ready for review April 2, 2024 06:37
@bradh bradh changed the title libheif: WIP on support for shared auxl alpha images libheif: support for shared auxl alpha images Apr 2, 2024
@bradh
Copy link
Contributor Author

bradh commented Apr 2, 2024

This should now fully address #1147

Also cleans up some unused members that made the problem look more complicated.

@leo-barnes
Copy link

Hi @bradh!

Do you have a time-line for when this will land?

@bradh
Copy link
Contributor Author

bradh commented May 14, 2024

Do you have a time-line for when this will land?

I think it is complete but cannot progress it beyond this point.

@farindk
Copy link
Contributor

farindk commented May 15, 2024

Yes, I know I'm the bottleneck. I'm currently completely blocked by other projects. Will have a look as soon as I can spend time on this.

@bradh
Copy link
Contributor Author

bradh commented May 15, 2024

Yes, I know I'm the bottleneck. I'm currently completely blocked by other projects. Will have a look as soon as I can spend time on this.

@farindk I think you should only do libheif when it suits you. Better that it gets done right, and that the long term for both you and the code is healthy and happy. So no rush from me! Sorry if it looked like criticism.

@leo-barnes
Copy link

Got it, no worries. Was just wondering what the plan was and whether there was anything spec or info-wise that was blocking you.

@farindk
Copy link
Contributor

farindk commented May 15, 2024

No worries at all. I didn't think that for a single second.
Just wanted to let you know that I'm fine, healthy and happy. Just working on something different and urgent at the moment.

@farindk farindk merged commit 58ba30d into strukturag:develop-v1.18.0 Jun 11, 2024
2 checks passed
@farindk
Copy link
Contributor

farindk commented Jun 11, 2024

Thanks, this is great.

One thing: I'm not sure if libheif should automatically scale the alpha channel. It is not clear what scaling filter should be used and whether the user wants this. At least, we might want to enable this as a decoding option. But we'll probably have to postpone this since currently, even libheif assumes that the alpha channel has the same resolution.

farindk added a commit that referenced this pull request Jun 11, 2024
@bradh bradh deleted the shared_iref_2024-03-19 branch June 11, 2024 22:28
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.

3 participants