gpac / ComplianceWarden

A pluggable compliance checker (ISOBMFF, HEIF/MIAF/AVIF, AV1 HDR10+)
https://gpac.github.io/ComplianceWarden-wasm/
Other
16 stars 7 forks source link

Feature request: json output for cw.exe #35

Closed podborski closed 1 year ago

podborski commented 2 years ago

First of all, thank you for your amazing work 👍🏻
I would like to integrate cw.exe in an automated workflow for conformance file validation in AOM and for that a json output would be very helpful.

Here is an example how it could look like based on HDR10+ validation:

{
  "cw_version": "v31-master-rev1-gcecd10d",
  "input_file": "./av1-hdr10plus-podborski/conformance/mp4/sparks_libaom_4096x2160_0_888_hdr10plus.mp4",
  "specification": "av1hdr10plus",
  "spec_version": "1.0.0-draft-a84336c",
  "spec_name": "HDR10+ AV1 Metadata Handling Specification",
  "spec_url": "https://aomediacodec.github.io/av1-hdr10plus/",
  "dependencies": ["isobmff"],

  "validation": [
    {
      "spec": "av1hdr10plus",
      "successful_checks": [...],
      "errors": [],
      "warnings": [
        {
          "rule": "2",
          "rule_id": null,
          "details": "Section 2.2.1\nStreams shall use the following values for the AV1 color_config:\n - color_primaries = 9 ([BT-2020])\n - transfer_characteristics = 16 ([SMPTE-ST-2084] / [BT-2100])\n - matrix_coefficients = 9 ([BT-2020])\nAdditionally, the following recommendations apply:\n - VideoFullRangeFlag should be set to 0\n - subsampling_x and subsampling_y should be set to 0\n - mono_chrome should be 0\n - chroma_sample_position should be set to 2",
          "description": "chroma_sample_position should be set as 2, found 0"
        },
        {
          "rule": "2",
          "rule_id": null,
          "details": "Section 2.2.1\nStreams shall use the following values for the AV1 color_config:\n - color_primaries = 9 ([BT-2020])\n - transfer_characteristics = 16 ([SMPTE-ST-2084] / [BT-2100])\n - matrix_coefficients = 9 ([BT-2020])\nAdditionally, the following recommendations apply:\n - VideoFullRangeFlag should be set to 0\n - subsampling_x and subsampling_y should be set to 0\n - mono_chrome should be 0\n - chroma_sample_position should be set to 2",
          "description": "chroma_sample_position should be set as 2, found 0"
        },
        {
          "rule": "2",
          "rule_id": null,
          "details": "Section 2.2.1\nStreams shall use the following values for the AV1 color_config:\n - color_primaries = 9 ([BT-2020])\n - transfer_characteristics = 16 ([SMPTE-ST-2084] / [BT-2100])\n - matrix_coefficients = 9 ([BT-2020])\nAdditionally, the following recommendations apply:\n - VideoFullRangeFlag should be set to 0\n - subsampling_x and subsampling_y should be set to 0\n - mono_chrome should be 0\n - chroma_sample_position should be set to 2",
          "description": "chroma_sample_position should be set as 2, found 0"
        },
        {
          "rule": "2",
          "rule_id": null,
          "details": "Section 2.2.1\nStreams shall use the following values for the AV1 color_config:\n - color_primaries = 9 ([BT-2020])\n - transfer_characteristics = 16 ([SMPTE-ST-2084] / [BT-2100])\n - matrix_coefficients = 9 ([BT-2020])\nAdditionally, the following recommendations apply:\n - VideoFullRangeFlag should be set to 0\n - subsampling_x and subsampling_y should be set to 0\n - mono_chrome should be 0\n - chroma_sample_position should be set to 2",
          "description": "chroma_sample_position should be set as 2, found 0"
        },
        {
          "rule": "2",
          "rule_id": null,
          "details": "Section 2.2.1\nStreams shall use the following values for the AV1 color_config:\n - color_primaries = 9 ([BT-2020])\n - transfer_characteristics = 16 ([SMPTE-ST-2084] / [BT-2100])\n - matrix_coefficients = 9 ([BT-2020])\nAdditionally, the following recommendations apply:\n - VideoFullRangeFlag should be set to 0\n - subsampling_x and subsampling_y should be set to 0\n - mono_chrome should be 0\n - chroma_sample_position should be set to 2",
          "description": "chroma_sample_position should be set as 2, found 0"
        }
      ]
    },
    {
      "spec": "isobmff",
      "successful_checks": [...],
      "errors": [],
      "warnings": []
    }
  ]
}

It would be nice to also include all of the successful checks, all rules that passed without errors or warnings. That would allow us to link files to features from the spec.

In addition to that, we are also working on a way to assign unique assert IDs to some of the AOM specs. For example, if you look into the current av1-isobmff index.html you can find things like:

<span id="assert-03258f22">It SHALL have the <dfn class="css" data-dfn-for="ISOBMFF Brand" data-dfn-type="value" data-export="" id="valdef-isobmff-brand-av01">av01<a class="self-link" href="#valdef-isobmff-brand-av01"></a></dfn> brand among the compatible brands array of the FileTypeBox</span>

So that a particular SHALL etc. statement is assigned to a unique id. This id could be also a part of the json output rule_id="assert-03258f22".

Do you think it would be possible to add this feature to cw.exe?

rbouqueau commented 2 years ago

The actual report is output here. I think it would be easy to add your JSON output implementation.

Then I think we need to improve the option parsing to propose output formats.

Does that make sense to you?

So that a particular SHALL etc. statement is assigned to a unique id. This id could be also a part of the json output rule_id="assert-03258f22".

Do you think it would be possible to add this feature to cw.exe?

The data structure is very easy to extend (Rule and Spec).

I think having such an id would make sense for all specs. Do you know if there are plans to extend it beyond your workflow?

podborski commented 2 years ago

@rbouqueau yes, a command line option, something like -json to give the json output and if not set the default thing is printed. I'll pipe the json output through to my script to generate conformance report.

I think having such an id would make sense for all specs. Do you know if there are plans to extend it beyond your workflow?

Do you mean if other SDOs are planning to do something similar what we did in AOM? I'm not aware, many are forced to use .docx files which makes it a bit harder. Theoretically possible though, one could hash sentences from a word document and create unique ID's. But its a bit more complicated since someone needs to do it and also update based on amendments, etc.

rbouqueau commented 2 years ago

I've queued it for my next batch of modifications of the tool.

rbouqueau commented 1 year ago

I've started to implement. ATM just add json as a last argument to your command-line. Feedback welcome.

"successful_checks" is currently empty: IIUC this was for content that actually execute the rule. I'm planning to add a return true on checks which exercise a rule. Please don't close this issue or create a new one if necessary.

rbouqueau commented 1 year ago

I've added that: https://github.com/gpac/ComplianceWarden/compare/exercized_rules

Please checkout the exercized_rules branch to see the "successful_checks".

podborski commented 1 year ago

@rbouqueau cool :) One question. the array contained in successful_checks is a list of rule numbers? For example in:

"specification": "av1hdr10plus",
        "successful_checks": [
          0,
          1
        ],

0 and 1 means Rule #0000 and Rule #0001 which can be obtained with cw.exe av1hdr10plus list?

[av1hdr10plus] Rule #0000: Section 2.1
An AV1 stream shall contain at least one OBU

[av1hdr10plus] Rule #0001: Section 2.1
Each HDR10+ OBU includes an ITU-T T.35 identifier with:
 - itu_t_t35_country_code set as 0xB5
 - itu_t_t35_terminal_provider_code set as 0x003C
 - itu_t_t35_terminal_provider_oriented_code set as 0x0001

If yes, would it be possible to print the array of objects, similar to warnings / errors array instead of just rule numbers?

podborski commented 1 year ago

Another question on rule number vs rule-id. Each rule in the spec is now embedded into an assert span with a unique id. For example assert-3d78af2f in AV1-ISOBMFF index.html

    <li data-md><span id="assert-3d78af2f">It SHALL conform to the normative requirements of <a data-link-type="biblio" href="#biblio-isobmff">[ISOBMFF]</a></span> 

We are adding the same feature to the HDR10+ spec as well. The current av1-hdr10plus index.html from the draft PR includes the asserts already. However this is not yet stable and some asserts might change before publication.

Question: would it be possible to add those assert-ids to each rule? This would allow us to generate a conformance coverage report based on cw.exe output.

rbouqueau commented 1 year ago

If yes, would it be possible to print the array of objects, similar to warnings / errors array instead of just rule numbers?

Done.

The current av1-hdr10plus index.html from the draft PR includes the asserts already.

I haven't been able to see them (even when using the html inspector). We can easily add a field for the assert-id when the feature is stable (or I could add a placeholder and you could add your ideas during your test phase?).

podborski commented 1 year ago

Thanks for adding objects to successful checks array :)

On the assert-ids. Yes this feature is currently in the conformance branch and is not yet merged. However, I would like to discuss with you and @cconcolato how to move forward here.

A short explanation:

Those assert-ids are generated using MD5 Hash. For example:

While the first 8 characters of the MD5 hash are taken to the span id with assert- prefix.

$ md5 -s "color_primaries = 9"
MD5 ("color_primaries = 9") = 2d0cc17415f0acda3dd020defaff0d76

Note that the bikeshed specific syntax is not included in hash computation. For example [= and =] are removed, but care needs to be taken when references are included within an assert. E.g. [[!AV1-ISOBMFF]] is translated to [AV1-ISOBMFF] before hash is calculated.


Question 1:

How can ComplianceWarden be aware of these assert-ids? I see that a single rule can include multiple checks which are separated in multiple assertions. This would mean that if a rule passed successfully and there were multiple assertions defined for that rule, we will need an array of assert-ids to be returned. For a warning or an error there will be only one assertion in that array telling what exactly has failed.

Question 2:

If ComplianceWarden is aware of assertion tags in the spec. We could also have a way to quickly identify which assertions changed or were added and are not yet implemented in cw.exe. I guess this would be useful in the future. What do you think?

rbouqueau commented 1 year ago

Question 1: I think there should be one rule per assert.

Question two: what would prevent to recompute the assert-id from the rule description in cw.exe? IIUC it means that the rule description must match the string on which the MD5 is computed.

podborski commented 1 year ago

Agree. If cw.exe would define each rule by extracting the string between the assert tags, and we can make sure that computing the MD5 hashes from that description strings will result in the same assert-ids from the index.html, we would be able to verify the conformance coverage by computing the id's from rule descriptions strings and comparing them to the entries from index.html

Other way around would also work. As long as we can map cw.exe output to the spec text (by text description itself or MD5 hashes) this will work. So whatever is easiest to implement for you.

podborski commented 1 year ago

Just discussed this with @cconcolato during the AV1-ISOBMFF call. We believe it would actually be better if cw.exe would be aware of the actual assert-ids from the compiled index.html. This would eliminate possible issues with not matching text of rule descriptions.

For convenience I attach the current draft of index.html below.

index.html.zip

rbouqueau commented 1 year ago

I've made a prototype on a new branch. Could you please try and let me know if it fits your need?

When an id is present for a rule, the report looks like this:

          "rule": "3",
          "id": "assert-45af0987",
          "details": ...

Full report:

{
  "cw_version": "v31-exercized-rules_assert-id-rev13-g64f49ec",
  "input_file": "",
  "specification": "av1hdr10plus",
  "spec_name": "HDR10+ AV1 Metadata Handling Specification, 8 December 2021\u000ahttps://aomediacodec.github.io/av1-hdr10plus/",
  "dependencies": [
    "isobmff"
  ],
  "validation": [
    {
      "specification": "av1hdr10plus",
      "successful_checks": [
        {
          "rule": "0",
          "details": "Section 2.1\u000aAn AV1 stream shall contain at least one OBU"
        },
        {
          "rule": "2",
          "details": "Section 2.2.1\u000aStreams shall use the following values for the AV1 color_config:\u000a - color_primaries = 9 ([BT-2020])\u000a - transfer_characteristics = 16 ([SMPTE-ST-2084] / [BT-2100])\u000a - matrix_coefficients = 9 ([BT-2020])\u000aAdditionally, the following recommendations apply:\u000a - VideoFullRangeFlag should be set to 0\u000a - subsampling_x and subsampling_y should be set to 0\u000a - mono_chrome should be 0\u000a - chroma_sample_position should be set to 2"
        }
      ],
      "errors": [
        {
          "rule": "1",
          "details": "Section 2.1\u000aEach HDR10+ OBU includes an ITU-T T.35 identifier with:\u000a - itu_t_t35_country_code set as 0xB5\u000a - itu_t_t35_terminal_provider_code set as 0x003C\u000a - itu_t_t35_terminal_provider_oriented_code set as 0x0001",
          "description": "itu_t_t35_country_code shall be set as 0xB5, found 0xA5"
        },
        {
          "rule": "3",
          "id": "assert-45af0987",
          "details": "Section 2.2.2\u000afor each frame with show_frame=1 or show_existing_frame=1, there shall be one\u000aand only one HDR10+ metadata OBU preceding the frame header for this frame and\u000alocated after the last OBU of the previous frame (if any) or after the\u000aSequence Header (if any) or after the start of the temporal unit (e.g. after the\u000atemporal delimiter, for storage formats where temporal delimiters are preserved).",
          "description": "There shall be one and only one HDR10+ metadata OBU. Found 0 in Temporal Unit #0 (Frame #0)"
        }
      ],
      "warnings": [
      ]
    }
  ]
}
podborski commented 1 year ago

@rbouqueau thanks. One issue I still see is that it writes assert-id fields only for errors. Is it possible also write them for warnings and successful checks?

Also for some reason some error entries also seem to miss the assert-id (as also shown in your example above). Rule 3 has an id while rule 1 does not.

For convenience, here is a list of conformance files and cw.exe outputs, provided by Netflix.

rbouqueau commented 1 year ago

@rbouqueau thanks. One issue I still see is that it writes assert-id fields only for errors. Is it possible also write them for warnings and successful checks?

This came from a typo.

Also for some reason some error entries also seem to miss the assert-id (as also shown in your example above). Rule 3 has an id while rule 1 does not.

We said we would try this for one rule. Please feel free to add ids to the other rules.

For convenience, here is a list of conformance files and cw.exe outputs, provided by Netflix.

Let me know if there is anything that is expected from me at this stage. I'm happy to see that this feature will be helpful!

rbouqueau commented 1 year ago

This is now part of the latest release v32: https://github.com/gpac/ComplianceWarden/releases/tag/v32