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

[Nuo Mi]

On Tue, Jan 12, 2021 at 9:28 AM James Almer <jamrial at gmail.com> wrote:

> 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);
>      }
>

Thanks for the suggestion. But, according to spec, the infer is only work
for  ChromaFormatIdc == 1.
I have followed spec to fix it.

>
> 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.
>
fixed with   vui_parameters_default

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".