Skip to content

Conversation

@y-guyon
Copy link
Contributor

@y-guyon y-guyon commented Jan 9, 2026

No description provided.

@y-guyon y-guyon requested a review from wantehchang January 9, 2026 14:12
* Update libxml2.cmd/LocalLibXml2.cmake: v2.14.4
* Update aom.cmd/LocalAom.cmake: v3.13.1
* Update LocalAom.cmake: AVM research-v12.0.0
* Update LocalAvm.cmake: research-v13.0.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yannis: Thank you for writing this pull request. I will try to review it today.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yannis: I only did a partial review today. Sorry1 I will try to do a complete review tomorrow.

# CMake Error: Error required internal CMake variable not set, cmake may not be built correctly.
# Missing variable is: CMAKE_ASM_COMPILE_OBJECT
# For some reason this must be done before check_avif_option(AVIF_CODEC_AOM TARGET aom PKG_NAME aom)
enable_language(ASM)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should try to track down why this is necessary.

@jzern James: Do you have any speculations?


add_custom_target(
- dist
+ avm_dist
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jzern @vrabaud

I wonder if there is a way to import another project into a namespace in CMake. Otherwise we will need to audit all the add_custom_target() calls in the libaom and AVM CMake build systems and make sure the custom target names have the "aom_" or "avm_" prefix.


#if defined(AVIF_CODEC_AVM)
// See https://gitlab.com/AOMediaCodec/avm/-/blob/main/av1/decoder/decodeframe.c
// See https://gitlab.com/AOMediaCodec/avm/-/blob/research-v13.0.0/common/av2_config.c?ref_type=tags#L290
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unless we are sure that the code in common/av2_config.c is correct, it is better to follow the read_sequence_header_obu() function in av2/decoder/obu.c as an example.

avifBitsReadVLC(bits); // conf_win_right_offset
avifBitsReadVLC(bits); // conf_win_top_offset
avifBitsReadVLC(bits); // conf_win_bottom_offset
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This if statement (lines 515-520) corresponds to the following code in read_sequence_header_obu():

  av2_read_conformance_window(rb, seq_params);
  av2_validate_seq_conformance_window(seq_params, &cm->error);

Afterwards, read_sequence_header_obu() does the following:

#if CONFIG_CWG_F270_CI_OBU
  av2_read_chroma_format_bitdepth(rb, seq_params, &cm->error);
#else
  av2_read_color_config(rb, seq_params, &cm->error);
#endif  // CONFIG_CWG_F270_CI_OBU

So we should have similar code here. If CONFIG_CWG_F270_CI_OBU is equal to 1, we should only parse the chroma format and the bit depth. We can use a structure similar to the following code in av2/decoder/decodeframe.c:

void av2_read_chroma_format_bitdepth(
    struct avm_read_bit_buffer *rb,
#else
void av2_read_color_config(
    struct avm_read_bit_buffer *rb,
#endif  // CONFIG_CWG_F270_CI_OBU
    SequenceHeader *seq_params, struct avm_internal_error_info *error_info) {
  const uint32_t seq_chroma_format_idc = avm_rb_read_uvlc(rb);
  set_seq_chroma_format(seq_chroma_format_idc, seq_params, error_info);

  read_bitdepth(rb, seq_params, error_info);

  const int is_monochrome = (seq_chroma_format_idc == CHROMA_FORMAT_400);
  seq_params->monochrome = is_monochrome;
#if !CONFIG_CWG_F270_CI_OBU
  int color_description_present_flag = avm_rb_read_bit(rb);
  if (color_description_present_flag) {
    seq_params->color_primaries = avm_rb_read_literal(rb, 8);
    seq_params->transfer_characteristics = avm_rb_read_literal(rb, 8);
    seq_params->matrix_coefficients = avm_rb_read_literal(rb, 8);
  } else {
    seq_params->color_primaries = AVM_CICP_CP_UNSPECIFIED;
    seq_params->transfer_characteristics = AVM_CICP_TC_UNSPECIFIED;
    seq_params->matrix_coefficients = AVM_CICP_MC_UNSPECIFIED;
  }
  if (is_monochrome) {
    // [16,235] (including xvycc) vs [0,255] range
    seq_params->color_range = avm_rb_read_bit(rb);
    seq_params->subsampling_y = seq_params->subsampling_x = 1;
    seq_params->chroma_sample_position = AVM_CSP_UNSPECIFIED;
  } else {
    if (seq_params->color_primaries == AVM_CICP_CP_BT_709 &&
        seq_params->transfer_characteristics == AVM_CICP_TC_SRGB &&
        seq_params->matrix_coefficients == AVM_CICP_MC_IDENTITY) {
      seq_params->subsampling_y = seq_params->subsampling_x = 0;
      seq_params->color_range = 1;  // assume full color-range
      if (!(seq_params->profile == PROFILE_1 ||
            (seq_params->profile == PROFILE_2 &&
             seq_params->bit_depth == AVM_BITS_12))) {
        avm_internal_error(
            error_info, AVM_CODEC_UNSUP_BITSTREAM,
            "sRGB colorspace not compatible with specified profile");
      }
    } else {
      // [16,235] (including xvycc) vs [0,255] range
      seq_params->color_range = avm_rb_read_bit(rb);
      if (seq_params->matrix_coefficients == AVM_CICP_MC_IDENTITY &&
          (seq_params->subsampling_x || seq_params->subsampling_y)) {
        avm_internal_error(
            error_info, AVM_CODEC_UNSUP_BITSTREAM,
            "Identity CICP Matrix incompatible with non 4:4:4 color sampling");
      }
      if (seq_params->subsampling_x && !seq_params->subsampling_y) {
        // YUV 4:2:2
        seq_params->chroma_sample_position = AVM_CSP_UNSPECIFIED;
        const int csp_present_flag = avm_rb_read_bit(rb);
        if (csp_present_flag) {
          seq_params->chroma_sample_position = avm_rb_read_bit(rb);
        }
      } else if (seq_params->subsampling_x && seq_params->subsampling_y) {
        // YUV 4:2:0
        seq_params->chroma_sample_position = AVM_CSP_UNSPECIFIED;
        const int csp_present_flag = avm_rb_read_bit(rb);
        if (csp_present_flag) {
          const int chroma_sample_position = avm_rb_read_literal(rb, 3);
          if (chroma_sample_position > AVM_CSP_BOTTOM) {
            avm_internal_error(error_info, AVM_CODEC_UNSUP_BITSTREAM,
                               "Invalid chroma_sample_position");
          }
          seq_params->chroma_sample_position = chroma_sample_position;
        }
      }
    }
  }
#endif  // !CONFIG_CWG_F270_CI_OBU
}

* Update libxml2.cmd/LocalLibXml2.cmake: v2.14.4
* Update aom.cmd/LocalAom.cmake: v3.13.1
* Update LocalAom.cmake: AVM research-v12.0.0
* Update LocalAvm.cmake: research-v13.0.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yannis: I only did a partial review today. Sorry1 I will try to do a complete review tomorrow.

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