[FFmpeg-devel] [PATCH v2 06/11] avcodec: add cbs for h266/vvc

[James Almer]

On 1/11/2021 5:45 PM, Mark Thompson wrote:
>>>> +static int FUNC(vui_parameters)(CodedBitstreamContext *ctx, RWContext
>>> *rw,
>>>> +                                H266RawVUI *current)
>>>> +{
>>>> +    int err;
>>>> +
>>>> +    flag(vui_progressive_source_flag);
>>>> +    flag(vui_interlaced_source_flag);
>>>> +    flag(vui_non_packed_constraint_flag);
>>>> +    flag(vui_non_projected_constraint_flag);
>>>> +    flag(vui_aspect_ratio_info_present_flag);
>>>> +    if (current->vui_aspect_ratio_info_present_flag) {
>>>> +        flag(vui_aspect_ratio_constant_flag);
>>>> +        ub(8, vui_aspect_ratio_idc);
>>>> +        if (current->vui_aspect_ratio_idc == 255) {
>>>> +            ub(16, vui_sar_width);
>>>> +            ub(16, vui_sar_height);
>>>> +        }
>>>> +    } else {
>>>> +        infer(vui_aspect_ratio_constant_flag, 0);
>>>> +        infer(vui_aspect_ratio_idc, 0);
>>>> +    }
>>>> +    flag(vui_overscan_info_present_flag);
>>>> +    if (current->vui_overscan_info_present_flag)
>>>> +        flag(vui_overscan_appropriate_flag);
>>>> +    flag(vui_colour_description_present_flag);
>>>> +    if (current->vui_colour_description_present_flag) {
>>>> +        ub(8, vui_colour_primaries);
>>>> +        ub(8, vui_transfer_characteristics);
>>>> +        ub(8, vui_matrix_coeffs);
>>>> +        flag(vui_full_range_flag);
>>>> +    } else {
>>>> +        infer(vui_colour_primaries, 2);
>>>> +        infer(vui_transfer_characteristics, 2);
>>>> +        infer(vui_matrix_coeffs, 2);
>>>> +        infer(vui_full_range_flag, 0);
>>>> +    }
>>>> +    flag(vui_chroma_loc_info_present_flag);
>>>> +    if (current->vui_chroma_loc_info_present_flag) {
>>>> +        if (current->vui_progressive_source_flag &&
>>>> +            !current->vui_interlaced_source_flag) {
>>>> +            ue(vui_chroma_sample_loc_type_frame, 0, 6);
>>>> +        } else {
>>>> +            ue(vui_chroma_sample_loc_type_top_field, 0, 6);
>>>> +            ue(vui_chroma_sample_loc_type_bottom_field,  0, 6);
>>>> +        }
>>>> +    }
>>>
>>> These are inferred as 6 when not present?
>>>
>> 6 only happened when ChromaFormatIdc = 1, others are not defined.
>> and 6 is the unspecific value in the spec...
>> Do we really need to infer it :)
> 
> To match the colour description cases probably yes?

"When vui_chroma_loc_info_present_flag is equal to 0, 
vui_chroma_sample_loc_type_frame is not present and is inferred to be 
equal to 6, which indicates that the location of the chroma samples is 
unknown or unspecified or specified by other means not specified in this 
Specification. When vui_chroma_sample_loc_type_top_field and 
vui_chroma_sample_loc_type_bottom_field are not present, the values of 
vui_chroma_sample_loc_type_top_field and 
vui_chroma_sample_loc_type_bottom_field are inferred to be equal to 
vui_chroma_sample_loc_type_frame."

So the correct implementation i think would be

     flag(vui_chroma_loc_info_present_flag);
     if (current->vui_chroma_loc_info_present_flag) {
         if (current->vui_progressive_source_flag &&
             !current->vui_interlaced_source_flag) {
             ue(vui_chroma_sample_loc_type_frame, 0, 6);
             infer(vui_chroma_sample_loc_type_top_field,
                   current->vui_chroma_sample_loc_type_frame);
             infer(vui_chroma_sample_loc_type_bottom_field,
                   current->vui_chroma_sample_loc_type_frame);
         } else {
             infer(vui_chroma_sample_loc_type_frame, 6);
             ue(vui_chroma_sample_loc_type_top_field, 0, 6);
             ue(vui_chroma_sample_loc_type_bottom_field,  0, 6);
         }
     } else {
         infer(vui_chroma_sample_loc_type_frame, 6);
         infer(vui_chroma_sample_loc_type_top_field,
               current->vui_chroma_sample_loc_type_frame);
         infer(vui_chroma_sample_loc_type_bottom_field,
               current->vui_chroma_sample_loc_type_frame);
     }

Also, you also need to infer the default value of almost everything when 
sps_vui_parameters_present_flag is 0. See section D.8 and how h265 does 
it with the vui_parameters_default() custom function.