Fix two bugs in parseAV2SequenceHeader()

[wantehchang]

Remove an early return of AVIF_FALSE in the
!header->reduced_still_picture_header case.

Save the value of the seq_tier syntax element in header->av1C.seqTier0.

[y-guyon]

I vaguely remember this early return was here because the rest of the function was not implemented when reduced_still_picture_header is false. Is this no longer the case? Or maybe the fields have been rearranged in a way that we can skip all the remaining fields because we are only interested in the ones at the beginning of the OBU?

[wantehchang]

There are two problems with this return statement.

1. It skips the rest of the function, leaving several fields in the avifSequenceHeader struct unset
2. It returns AVIF_FALSE, which indicates failure.

Here is the corresponding code in AVM research-v13.0.0:

  if (seq_params->single_picture_header_flag) {
    seq_params->seq_lcr_id = LCR_ID_UNSPECIFIED;
    seq_params->still_picture = 1;
  } else {
    int seq_lcr_id = avm_rb_read_literal(rb, 3);
    if (seq_lcr_id > MAX_NUM_SEQ_LCR_ID) {
      avm_internal_error(&cm->error, AVM_CODEC_UNSUP_BITSTREAM,
                         "Unsupported LCR id in the Sequence Header.\n");
    }
    seq_params->seq_lcr_id = seq_lcr_id;
    seq_params->still_picture = avm_rb_read_bit(rb);
  }

Note that there is no return statement in the else block.

[y-guyon]

There are two problems with this return statement.

1. It skips the rest of the function, leaving several fields in the avifSequenceHeader struct unset
2. It returns AVIF_FALSE, which indicates failure.

With 2., 1. is not a problem.
I think 2. was in fact a lazy "not implemented", due to later code relying on single_picture_header_flag. But these single_picture_header_flag-dependent fields below are all skipped anyway so this PR is fine.

[y-guyon]

I was not sure single_tier_0 is equivalent to seqTier0.

[wantehchang]

single_tier_0 may have been a typo.

Here is the corresponding code in AVM research-v13.0.0:

  if (seq_params->seq_max_level_idx >= SEQ_LEVEL_4_0 &&
      !seq_params->single_picture_header_flag)
    seq_params->seq_tier = avm_rb_read_bit(rb);
  else
    seq_params->seq_tier = 0;

Note: SEQ_LEVEL_4_0 is equal to 8.

The name of the struct member is seq_tier. The syntax element is also called seq_tier in the draft AV2 spec (v13):

  if ( seq_level_idx > 7 && !single_picture_header_flag ) {	
    seq_tier	                                               f(1)
  } else {	
    seq_tier = 0	
  }

[wantehchang]

Thanks for the review!

[wantehchang]

There are two problems with this return statement.

1. It skips the rest of the function, leaving several fields in the avifSequenceHeader struct unset
2. It returns AVIF_FALSE, which indicates failure.

Here is the corresponding code in AVM research-v13.0.0:

  if (seq_params->single_picture_header_flag) {
    seq_params->seq_lcr_id = LCR_ID_UNSPECIFIED;
    seq_params->still_picture = 1;
  } else {
    int seq_lcr_id = avm_rb_read_literal(rb, 3);
    if (seq_lcr_id > MAX_NUM_SEQ_LCR_ID) {
      avm_internal_error(&cm->error, AVM_CODEC_UNSUP_BITSTREAM,
                         "Unsupported LCR id in the Sequence Header.\n");
    }
    seq_params->seq_lcr_id = seq_lcr_id;
    seq_params->still_picture = avm_rb_read_bit(rb);
  }

Note that there is no return statement in the else block.

[wantehchang]

single_tier_0 may have been a typo.

Here is the corresponding code in AVM research-v13.0.0:

  if (seq_params->seq_max_level_idx >= SEQ_LEVEL_4_0 &&
      !seq_params->single_picture_header_flag)
    seq_params->seq_tier = avm_rb_read_bit(rb);
  else
    seq_params->seq_tier = 0;

Note: SEQ_LEVEL_4_0 is equal to 8.

The name of the struct member is seq_tier. The syntax element is also called seq_tier in the draft AV2 spec (v13):

  if ( seq_level_idx > 7 && !single_picture_header_flag ) {	
    seq_tier	                                               f(1)
  } else {	
    seq_tier = 0	
  }