Re: [FFmpeg-devel] [PATCH v4 2/3] avcodec/hevc/ps: Add basic HEVC_SCALABILITY_AUXILIARY support

[Zhao Zhili <quinkblack-at-foxmail.com@ffmpeg.org>]

> On Feb 6, 2025, at 21:25, James Almer <jamrial@gmail.com> wrote:
> 
> On 2/6/2025 12:49 AM, Zhao Zhili wrote:
>> From: Zhao Zhili <zhilizhao@tencent.com>
>> Only implementing what's needed for HEVC with alpha.
>> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com>
>> ---
>>  libavcodec/hevc/hevc.h |   5 ++
>>  libavcodec/hevc/ps.c   | 126 +++++++++++++++++++++++++++++------------
>>  libavcodec/hevc/ps.h   |   4 ++
>>  3 files changed, 98 insertions(+), 37 deletions(-)
>> diff --git a/libavcodec/hevc/hevc.h b/libavcodec/hevc/hevc.h
>> index b2229fda40..710786a89d 100644
>> --- a/libavcodec/hevc/hevc.h
>> +++ b/libavcodec/hevc/hevc.h
>> @@ -170,4 +170,9 @@ enum HEVCScalabilityMask {
>>      HEVC_SCALABILITY_MASK_MAX   = 0xFFFF,
>>  };
>>  +enum HEVCAuxId {
>> +    HEVC_AUX_ALPHA = 1,
>> +    HEVC_AUX_DEPTH = 2,
>> +};
>> +
>>  #endif /* AVCODEC_HEVC_HEVC_H */
>> diff --git a/libavcodec/hevc/ps.c b/libavcodec/hevc/ps.c
>> index 861a6bb0a2..4f18cd72e4 100644
>> --- a/libavcodec/hevc/ps.c
>> +++ b/libavcodec/hevc/ps.c
>> @@ -460,14 +460,17 @@ static int decode_vps_ext(GetBitContext *gb, AVCodecContext *avctx, HEVCVPS *vps
>>                            uint64_t layer1_id_included)
>>  {
>>      PTL ptl_dummy;
>> -    uint8_t max_sub_layers[HEVC_MAX_LAYERS];
>> +    uint8_t max_sub_layers[HEVC_MAX_LAYERS] = {1, 1};
>> +    uint8_t dimension_id_len[16] = {0};
>> +    uint8_t dimension_id[16] = {0};
>> +    unsigned n;
>>  -    int splitting_flag, dimension_id_len, view_id_len, num_add_olss, num_scalability_types,
>> +    int splitting_flag, view_id_len, num_add_olss, num_scalability_types,
>>          default_output_layer_idc, direct_dep_type_len, direct_dep_type,
>>          sub_layers_max_present, sub_layer_flag_info_present_flag, nb_ptl;
>>      unsigned non_vui_extension_length;
>>  -    if (vps->vps_max_layers == 1 || vps->vps_num_layer_sets == 1) {
>> +    if (vps->vps_max_layers == 1) {
>>          av_log(avctx, AV_LOG_VERBOSE, "Ignoring VPS extensions with a single layer\n");
>>          return 0;
>>      }
>> @@ -520,7 +523,8 @@ static int decode_vps_ext(GetBitContext *gb, AVCodecContext *avctx, HEVCVPS *vps
>>       */
>>      vps->nb_layers = 2;
>>  -    if (parse_ptl(gb, avctx, 0, &ptl_dummy, vps->vps_max_sub_layers) < 0)
>> +    if (vps->vps_base_layer_internal_flag &&
> 
> What's the point checking for this flag if this code will not be reached if it's false?

Here is only meant to follow the spec with little effort. When someone add support for
vps_base_layer_internal_flag = 0, it’s clear what does the flag can affect.

> 
>> +        parse_ptl(gb, avctx, 0, &ptl_dummy, vps->vps_max_sub_layers) < 0)
>>          return AVERROR_INVALIDDATA;
>>        splitting_flag = get_bits1(gb);
>> @@ -529,20 +533,25 @@ static int decode_vps_ext(GetBitContext *gb, AVCodecContext *avctx, HEVCVPS *vps
>>      if (!num_scalability_types) {
>>          av_log(avctx, AV_LOG_ERROR, "Missing scalability mask\n");
>>          return AVERROR_INVALIDDATA;
>> -    } else if (num_scalability_types > 1) {
>> -        av_log(avctx, AV_LOG_ERROR, "Scalability number %d not supported\n",
>> -               num_scalability_types);
>> -        return AVERROR_PATCHWELCOME;
>>      }
>>  -    if (!(vps->scalability_mask_flag & HEVC_SCALABILITY_MULTIVIEW)) {
>> +    if (!(vps->scalability_mask_flag &
>> +          (HEVC_SCALABILITY_MULTIVIEW | HEVC_SCALABILITY_AUXILIARY))) {
>>          av_log(avctx, AV_LOG_ERROR, "Scalability type %d not supported\n",
>>                 15 - ff_ctz(vps->scalability_mask_flag));
>>          return AVERROR_PATCHWELCOME;
>>      }
>> +    // x265 specify MULTIVIEW when the stream really is alpha video only.
>> +    if (num_scalability_types > 1)
>> +        av_log(avctx, AV_LOG_WARNING, "Multiple scalability types presented\n");
>>  -    if (!splitting_flag)
>> -        dimension_id_len = get_bits(gb, 3) + 1;
>> +    n = 0;
>> +    for (int i = 0; i < num_scalability_types - splitting_flag; i++) {
>> +        dimension_id_len[i] = get_bits(gb, 3) + 1;
>> +        n += dimension_id_len[i];
>> +    }
>> +    if (splitting_flag)
>> +        dimension_id_len[num_scalability_types - 1] = 5 - n;
>>        if (get_bits1(gb)) { /* vps_nuh_layer_id_present_flag */
>>          int layer_id_in_nuh = get_bits(gb, 6);
>> @@ -559,28 +568,57 @@ static int decode_vps_ext(GetBitContext *gb, AVCodecContext *avctx, HEVCVPS *vps
>>      }
>>        if (!splitting_flag) {
>> -        int view_idx = get_bits(gb, dimension_id_len);
>> -        if (view_idx != 1) {
>> -            av_log(avctx, AV_LOG_ERROR, "Unexpected ViewOrderIdx: %d\n", view_idx);
>> +        int index = 0;
>> +
>> +        for (int i = 0; i < num_scalability_types; i++)
>> +            dimension_id[i] = get_bits(gb, dimension_id_len[i]);
>> +
>> +        if (vps->scalability_mask_flag & HEVC_SCALABILITY_MULTIVIEW)
>> +            index++;
>> +
>> +        /* AuxId 1 is alpha, 2 is depth. Only support alpha */
>> +        if (vps->scalability_mask_flag & HEVC_SCALABILITY_AUXILIARY &&
>> +            dimension_id[index] != HEVC_AUX_ALPHA) {
>> +            av_log(avctx, AV_LOG_WARNING,
>> +                   "Unsupported dimension_id %d for HEVC_SCALABILITY_AUXILIARY\n",
>> +                   dimension_id[index]);
>>              return AVERROR_PATCHWELCOME;
>>          }
>>      }
>>        view_id_len = get_bits(gb, 4);
>> -    if (view_id_len)
>> -        for (int i = 0; i < 2 /* NumViews */; i++)
>> +    if (view_id_len) {
>> +        n = (vps->scalability_mask_flag & HEVC_SCALABILITY_MULTIVIEW) ? 2 : 1;
>> +        for (int i = 0; i < n; i++)
>>              vps->view_id[i] = get_bits(gb, view_id_len);
>> +    }
>>  -    if (!get_bits1(gb) /* direct_dependency_flag */) {
>> -        av_log(avctx, AV_LOG_WARNING, "Independent output layers not supported\n");
>> -        return AVERROR_PATCHWELCOME;
>> +    /* direct_dependency_flag */
>> +    vps->num_direct_ref_layers[1] = get_bits1(gb);
>> +    if (!vps->num_direct_ref_layers[1]) {
>> +        vps->num_add_layer_sets = get_ue_golomb(gb);
>> +        if (vps->num_add_layer_sets > 1) {
>> +            av_log(avctx, AV_LOG_WARNING,
>> +                   "Unsupported num_add_layer_sets: %d\n", vps->num_add_layer_sets);
>> +            return AVERROR_PATCHWELCOME;
>> +        }
>> +
>> +        if (vps->num_add_layer_sets) {
>> +            /* highest_layer_idx_plus1 */
>> +            if (!get_bits1(gb))
>> +                return AVERROR_PATCHWELCOME;
>> +        }
>>      }
>> -    vps->num_direct_ref_layers[1] = 1;
>> +    vps->num_output_layer_sets = vps->vps_num_layer_sets + vps->num_add_layer_sets;
>> +    if (vps->num_output_layer_sets != 2)
>> +        return AVERROR_INVALIDDATA;
>>        sub_layers_max_present = get_bits1(gb); // vps_sub_layers_max_minus1_present_flag
>> -    for (int i = 0; i < vps->vps_max_layers; i++)
>> -        max_sub_layers[i] = sub_layers_max_present ? get_bits(gb, 3) + 1 :
>> -                                                     vps->vps_max_sub_layers;
>> +    if (sub_layers_max_present) {
>> +        for (int i = 0; i < vps->vps_max_layers; i++)
>> +            max_sub_layers[i] = sub_layers_max_present ? get_bits(gb, 3) + 1 :
>> +                                                         vps->vps_max_sub_layers;
>> +    }
>>        if (get_bits1(gb) /* max_tid_ref_present_flag */)
>>          skip_bits(gb, 3); // max_tid_il_ref_pics_plus1
>> @@ -613,18 +651,24 @@ static int decode_vps_ext(GetBitContext *gb, AVCodecContext *avctx, HEVCVPS *vps
>>      }
>>        /* Consequence of established layer dependencies */
>> -    if (layer1_id_included != ((1 << vps->layer_id_in_nuh[0]) |
>> +    if (layer1_id_included &&
>> +        layer1_id_included != ((1 << vps->layer_id_in_nuh[0]) |
>>                                 (1 << vps->layer_id_in_nuh[1]))) {
>> -        av_log(avctx, AV_LOG_ERROR, "Dependent layer not included in layer ID?\n");
>> -        return AVERROR_PATCHWELCOME;
>> +            av_log(avctx, AV_LOG_ERROR,
>> +                   "Dependent layer not included in layer ID?\n");
>> +            return AVERROR_PATCHWELCOME;
>>      }
>> +    if (!layer1_id_included)
>> +        vps->ols[1] = 2;
>> +    else
>> +        vps->ols[1] = 3;
>>  -    vps->num_output_layer_sets = 2;
>> -    vps->ols[1] = 3;
>> +    if (vps->vps_num_layer_sets == 1 || default_output_layer_idc == 2)
>> +        skip_bits1(gb);
>>        for (int j = 0; j < av_popcount64(vps->ols[1]); j++) {
>>          int ptl_idx = get_bits(gb, av_ceil_log2(nb_ptl));
>> -        if (ptl_idx < 1 || ptl_idx >= nb_ptl) {
>> +        if (ptl_idx >= nb_ptl) {
>>              av_log(avctx, AV_LOG_ERROR, "Invalid PTL index: %d\n", ptl_idx);
>>              return AVERROR_INVALIDDATA;
>>          }
>> @@ -667,6 +711,8 @@ static int decode_vps_ext(GetBitContext *gb, AVCodecContext *avctx, HEVCVPS *vps
>>        vps->max_one_active_ref_layer = get_bits1(gb);
>>      vps->poc_lsb_aligned          = get_bits1(gb);
>> +    if (!vps->num_direct_ref_layers[1])
>> +        vps->poc_lsb_not_present = get_bits1(gb) << 1;
> 
> Huh, so this was being checked in hls_slice_header() but never set before now?

It’s inferred to be 0 and initialized by memset to 0.

> 
>>        sub_layer_flag_info_present_flag = get_bits1(gb);
>>      for (int j = 0; j < FFMAX(max_sub_layers[0], max_sub_layers[1]); j++) {
>> @@ -688,12 +734,14 @@ static int decode_vps_ext(GetBitContext *gb, AVCodecContext *avctx, HEVCVPS *vps
>>          return AVERROR_INVALIDDATA;
>>      }
>>  -    skip_bits1(gb); /* direct_depenency_all_layers_flag */
>> -    direct_dep_type = get_bits_long(gb, direct_dep_type_len);
>> -    if (direct_dep_type > HEVC_DEP_TYPE_BOTH) {
>> -        av_log(avctx, AV_LOG_WARNING, "Unsupported direct_dep_type: %d\n",
>> -               direct_dep_type);
>> -        return AVERROR_PATCHWELCOME;
>> +    /* direct_dependency_all_layers_flag */
>> +    if (get_bits1(gb) || vps->num_direct_ref_layers[1]) {
>> +        direct_dep_type = get_bits_long(gb, direct_dep_type_len);
>> +        if (direct_dep_type > HEVC_DEP_TYPE_BOTH) {
>> +            av_log(avctx, AV_LOG_WARNING, "Unsupported direct_dep_type: %d\n",
>> +                   direct_dep_type);
>> +            return AVERROR_PATCHWELCOME;
>> +        }
>>      }
>>        non_vui_extension_length = get_ue_golomb(gb);
>> @@ -741,8 +789,12 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx,
>>      }
>>      vps->vps_id = vps_id;
>>  -    if (get_bits(gb, 2) != 3) { // vps_reserved_three_2bits
>> -        av_log(avctx, AV_LOG_ERROR, "vps_reserved_three_2bits is not three\n");
>> +    vps->vps_base_layer_internal_flag = get_bits1(gb);
>> +    vps->vps_base_layer_available_flag = get_bits1(gb);
>> +    if (!vps->vps_base_layer_internal_flag || !vps->vps_base_layer_available_flag) {
>> +        av_log(avctx, AV_LOG_ERROR,
>> +               "vps_base_layer_internal_flag or vps_base_layer_available_flag not set\n");
>> +        ret = AVERROR_PATCHWELCOME;
>>          goto err;
>>      }
>>  diff --git a/libavcodec/hevc/ps.h b/libavcodec/hevc/ps.h
>> index d3465e3d27..f706b889e6 100644
>> --- a/libavcodec/hevc/ps.h
>> +++ b/libavcodec/hevc/ps.h
>> @@ -171,6 +171,9 @@ typedef struct RepFormat {
>>  typedef struct HEVCVPS {
>>      unsigned int vps_id;
>>  +    uint8_t vps_base_layer_internal_flag;
>> +    uint8_t vps_base_layer_available_flag;
> 
> These two can be variables local to ff_hevc_decode_nal_vps() instead of being in this struct.

It’s used by decode_vps_ext, and meant as a start point to add support for these flags been zero
in the future.

> 
>> +
>>      uint8_t vps_temporal_id_nesting_flag;
>>      int vps_max_layers;
>>      int vps_max_sub_layers; ///< vps_max_temporal_layers_minus1 + 1
>> @@ -237,6 +240,7 @@ typedef struct HEVCVPS {
>>        // NumDirectRefLayers[layer_idx]
>>      uint8_t num_direct_ref_layers[HEVC_VPS_MAX_LAYERS];
>> +    uint8_t num_add_layer_sets;
>>        RepFormat rep_format;
>>  
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".