[FFmpeg-devel] [PATCH] avcodec/hevcdec: add support for sps_infer_scaling_list_flag == 1

[Anton Khirnov]

Quoting James Almer (2024-09-23 18:35:35)
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
> Untested, as i can't find a sample that triggers this code.
> 
>  libavcodec/hevc/cabac.c |  3 ++-
>  libavcodec/hevc/ps.c    | 18 +++++++++++++-----
>  libavcodec/hevc/ps.h    |  1 +
>  3 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/hevc/cabac.c b/libavcodec/hevc/cabac.c
> index 892dd1c215..ac2b52986e 100644
> --- a/libavcodec/hevc/cabac.c
> +++ b/libavcodec/hevc/cabac.c
> @@ -1084,8 +1084,9 @@ void ff_hevc_hls_residual_coding(HEVCLocalContext *lc, const HEVCPPS *pps,
>          dc_scale = 16;
>  
>          if (sps->scaling_list_enabled && !(transform_skip_flag && log2_trafo_size > 2)) {
> +            const HEVCLayerContext *const l = &s->layers[sps->sps_scaling_list_ref_layer_id];
>              const ScalingList *sl = pps->scaling_list_data_present_flag ?
> -            &pps->scaling_list : &sps->scaling_list;
> +            &pps->scaling_list : &l->sps->scaling_list;

You need to check that the layer is active and actually has an SPS.

>              int matrix_id = lc->cu.pred_mode != MODE_INTRA;
>  
>              matrix_id = 3 * matrix_id + c_idx;
> diff --git a/libavcodec/hevc/ps.c b/libavcodec/hevc/ps.c
> index f18b88489b..64b3089967 100644
> --- a/libavcodec/hevc/ps.c
> +++ b/libavcodec/hevc/ps.c
> @@ -1373,14 +1373,22 @@ int ff_hevc_parse_sps(HEVCSPS *sps, GetBitContext *gb, unsigned int *sps_id,
>  
>      sps->scaling_list_enabled = get_bits1(gb);
>      if (sps->scaling_list_enabled) {
> +        int sps_infer_scaling_list_flag = 0;
> +
>          set_default_scaling_list_data(&sps->scaling_list);
>  
> -        if (multi_layer_ext && get_bits1(gb)) { // sps_infer_scaling_list_flag
> -            av_log(avctx, AV_LOG_ERROR, "sps_infer_scaling_list_flag=1 not supported\n");
> -            return AVERROR_PATCHWELCOME;
> -        }
> +        if (multi_layer_ext)
> +            sps_infer_scaling_list_flag = get_bits1(gb);
>  
> -        if (get_bits1(gb)) {
> +        sps->sps_scaling_list_ref_layer_id = 0;
> +        if (sps_infer_scaling_list_flag) {
> +            sps->sps_scaling_list_ref_layer_id = get_bits(gb, 6);
> +            if (sps->sps_scaling_list_ref_layer_id >= HEVC_VPS_MAX_LAYERS) {

This value is nuh_layer_id of the reference layer, but you're using it
as an index into the VPS layer list. Mapping nuh_layer_id to VPS layer
indices is done via vps->layer_idx.

> +                av_log(avctx, AV_LOG_ERROR, "Invalid value %d for sps_scaling_list_ref_layer_id",
> +                       sps->sps_scaling_list_ref_layer_id);
> +                return AVERROR_INVALIDDATA;
> +            }
> +        } else if (get_bits1(gb)) {
>              ret = scaling_list_data(gb, avctx, &sps->scaling_list, sps);
>              if (ret < 0)
>                  return ret;
> diff --git a/libavcodec/hevc/ps.h b/libavcodec/hevc/ps.h
> index 6f5b1f8755..aa4326554c 100644
> --- a/libavcodec/hevc/ps.h
> +++ b/libavcodec/hevc/ps.h
> @@ -277,6 +277,7 @@ typedef struct HEVCSPS {
>      VUI vui;
>      PTL ptl;
>  
> +    int sps_scaling_list_ref_layer_id;

Leading sps_ is redundant. Also might be better to store the index
rather than layer id.

Overall I'm not sure it's worth it to support something for which no
samples are known to exist.

-- 
Anton Khirnov