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

add files from m63202: HEIF prdi: Canon #107

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Conversation

podborski
Copy link
Member

No description provided.

@podborski podborski added the conformance-file This initiates file feature extraction on PRs label Oct 19, 2023
@podborski
Copy link
Member Author

@DenizUgur I kind of expected the pipeline to fail since the progressive-diagonal_gpac.ext.json is not yet filled out but it seems to pass.

@podborski
Copy link
Member Author

The contribution includes gpac json files that seem to understand the proposed item. I guess we can:

  • either copy those over and replace the gpac files we generated. And remove ext.json files
  • or copy unknown parts from the modified gpac jsons into our ext.json files

@DenizUgur
Copy link
Collaborator

DenizUgur commented Oct 19, 2023

@podborski prdi has been mentioned in the coverage output. It appears that since it's not in MP4RA it's not expected for it to be present in this repository either. If it were to be present in MP4RA it would've been raised as a warning (to be changed to error later on when we complete everything).

The contribution includes gpac json files that seem to understand the proposed item.

GPAC (latest nightly build) does not understand what prdi box is (hence UnknownBox). Even if it seems okay (no version, flags, etc.) the current process is to rely on GPAC to understand what prdi is. When it is implemented, it is safe to delete the .ext.json file (given _gpac file is also regenerated).

It seems that, the contribution have used an unreleased version of GPAC to generate those dumps. So updating the .ext.json files until upstream GPAC implements these would be the best option

@DenizUgur
Copy link
Collaborator

However, I agree that if there is an unknown box in these files it should raise an error if it's not in standard features. I'll open up an issue and implement this check tomorrow.

@podborski
Copy link
Member Author

It appears that since it's not in MP4RA it's not expected for it to be present in this repository either. If it were to be present in MP4RA it would've been raised as a warning (to be changed to error later on when we complete everything).

@DenizUgur ah I see.

2023-10-18 22:07:18.427 | INFO | construct.coverage:main:109 - Box prdi is not in standard features or MP4RA

Perhaps we should consider changing this to a warning. The problem is that we still want to be able to progress with adding conformance files even before the amendment is published and we register the code points. And it can happen (as just happened in this case) that we forget to add it to standard features as well.

BTW. It's super nice that the conformance framework reports missing candidates for mp4ra.

@cconcolato
Copy link
Contributor

Reviewers needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conformance-file This initiates file feature extraction on PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants