AOMediaCodec / av1-avif

AV1 Image File Format Specification - ISO-BMFF/HEIF derivative
https://aomediacodec.github.io/av1-avif/
BSD 2-Clause "Simplified" License
450 stars 40 forks source link

Fix incorrect 'hdlr' box and truncated multilayer file #175

Closed leo-barnes closed 2 years ago

leo-barnes commented 2 years ago

Also adds some test files for checking correct handling of edge cases regarding properties.

Resolves #174 and #152.

leo-barnes commented 2 years ago

@baumanj Can't add you as a reviewer for some reason.

baumanj commented 2 years ago

Also, @cconcolato, can I get whatever privileges are necessary for me to be a reviewer on this repo?

cconcolato commented 2 years ago

Also, @cconcolato, can I get whatever privileges are necessary for me to be a reviewer on this repo?

done

leo-barnes commented 2 years ago

Checking with Compliance Warden, I see a couple minor issues with all the files that I'm not concerned about, but some bigger ones with

animals_00_multilayer_grid_a1lx.avif animals_00_multilayer_grid_lsel.avif

that I do think should be addressed

I think ComplianceWarden doesn't understand the files correctly, we should probably file an issue to get it updated. The issues it shows me for animals_00_multilayer_grid_lsel.avif is:

AVIF:

[avif][Rule #3] Section 2.1
The AV1 Image Item Data should have its still_picture flag set to 1.

[avif][Rule #4] Section 2.1
The AV1 Image Item Data should have its reduced_still_picture_header flag set to 1.

You can't set still_picture and reduced_still_picture_header flags to 1 for multi-layer content IIRC. I think the spec draft addressed this.

MIAF:

[miaf][Rule #5] Error: construction_method=1 on a coded image item

This is the contents of the iinf and iloc:

  ('iinf' "Item Information Box", size = 224) {
    Version: 0, Flags: 0x000000
    Entry count: 10
      ('infe' "Item Info Entry", (condensed)) {
        Item ID: 1-2
        Item protection index: 0
        Item type: 'grid'
      }
      ('infe' "Item Info Entry", (condensed)) {
        Item ID: 3-10 (Hidden)
        Item protection index: 0
        Item type: 'av01'
      }
  }
...
  ('iloc' "Item Location Box", size = 240) {
    Version: 1, Flags: 0x000000
    Offset size: 4
    Length size: 4
    Base offset size: 0
    Index size: 0
    Item count: 10
      Item ID: 1
        Construction method: 1, Data reference index: 0
        Extent 1/1: Offset 0, Length 8
      Item ID: 2
        Construction method: 1, Data reference index: 0
        Extent 1/1: Offset 8, Length 8
      Item ID: 3
        Construction method: 0, Data reference index: 0
        Extent 1/2: Offset 914, Length 22374
        Extent 2/2: Offset 577666, Length 422411
      Item ID: 4
        Construction method: 0, Data reference index: 0
        Extent 1/2: Offset 23288, Length 151066
        Extent 2/2: Offset 1000077, Length 314112
      Item ID: 5
        Construction method: 0, Data reference index: 0
        Extent 1/2: Offset 174354, Length 183418
        Extent 2/2: Offset 1314189, Length 482385
      Item ID: 6
        Construction method: 0, Data reference index: 0
        Extent 1/2: Offset 357772, Length 219894
        Extent 2/2: Offset 1796574, Length 510504
      Item ID: 7
        Construction method: 0, Data reference index: 0
        Extent 1/2: Offset 914, Length 22374
        Extent 2/2: Offset 577666, Length 422411
      Item ID: 8
        Construction method: 0, Data reference index: 0
        Extent 1/2: Offset 23288, Length 151066
        Extent 2/2: Offset 1000077, Length 314112
      Item ID: 9
        Construction method: 0, Data reference index: 0
        Extent 1/2: Offset 174354, Length 183418
        Extent 2/2: Offset 1314189, Length 482385
      Item ID: 10
        Construction method: 0, Data reference index: 0
        Extent 1/2: Offset 357772, Length 219894
        Extent 2/2: Offset 1796574, Length 510504
  }

Construction method is 1 for items 1 and 2 which are grids and 0 for all coded items. So this also looks correct to me.

HEIF:

[heif][Rule #5] Error: Tile [itemId=1]: the value of reference_count(4) shall be equal to rows(1)*columns(0)=29
[heif][Rule #5] Error: Tile [itemId=2]: the value of reference_count(4) shall be equal to rows(106)*columns(0)=103
[heif][Rule #8] Error: grid (itemID=1) height(3) not covered by tile (ItemId=31088) height(768)*numRows(1)=768
[heif][Rule #8] Error: grid (itemID=1) height(4) not covered by tile (ItemId=31088) height(768)*numRows(1)=768
[heif][Rule #8] Error: grid (itemID=1) height(5) not covered by tile (ItemId=31088) height(768)*numRows(1)=768
[heif][Rule #8] Error: grid (itemID=1) height(6) not covered by tile (ItemId=31088) height(768)*numRows(1)=768
[heif][Rule #19] Error: 'grid' version shall be equal to 0, found 97 (itemId=2)
[heif][Rule #32] Error: 'mif1' brand: this file shall conform to HEIF section 6, check the other errors for details

These errors make no sense. It looks like ComplianceWarden is not interpreting the idat contents correctly.

If I look at the contents of the idat I get:

0000 0101 0800 0600
0000 0101 0400 0300

Which if interpreted as grid boxes is:

aligned(8) class ImageGrid {
    unsigned int(8) version = 0;
    unsigned int(8) flags;
    FieldLength = ((flags & 1) + 1) * 16;
    unsigned int(8) rows_minus_one;
    unsigned int(8) columns_minus_one;
    unsigned int(FieldLength) output_width;
    unsigned int(FieldLength) output_height;
}
  ('idat' "Item Data Box", size = 24) {
    Version: 0, Flags: 0
    Grid size: 2x2
    Output canvas size: 2048x1536

    Version: 0, Flags: 0
    Grid size: 2x2
    Output canvas size: 1024x768
  }

Which also looks correct to me.

leo-barnes commented 2 years ago

Let me open an issue with the compliance warden.

leo-barnes commented 2 years ago

ComplianceWarden has been updated and now only complains about the still_picture flag stuff.

cconcolato commented 2 years ago

You can't set still_picture and reduced_still_picture_header flags to 1 for multi-layer content IIRC. I think the spec draft addressed this.

Yes, the spec says:

If the AV1 Image Item Data consists of a single frame (i.e. when using a single layer),

  • It should have its still_picture flag set to 1.
  • It should have its reduced_still_picture_header flag set to 1.

So when ComplianceWarden gets updated to the latest draft, this should go away.

leo-barnes commented 2 years ago

Merging this now. Will create a new PR to update the lsel to the new wording in the spec.