[FFmpeg-devel] [PATCH] avcodec/hevc_ps: Improve PPS SCC extension bit depth check

[James Almer]

On 7/9/2023 9:19 PM, Andreas Rheinhardt wrote:
>  From the spec: "It is a requirement of bitstream conformance that
> the value of luma_bit_depth_entry_minus8 shall be equal to
> the value of bit_depth_luma_minus8"; similarly for chroma.
> 
> Should fix Coverity ticket #1529226.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> ---
>   libavcodec/hevc_ps.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
> index 4c4c1e2c17..1db2d3a242 100644
> --- a/libavcodec/hevc_ps.c
> +++ b/libavcodec/hevc_ps.c
> @@ -1581,11 +1581,13 @@ static int pps_scc_extension(GetBitContext *gb, AVCodecContext *avctx,
>               }
>               pps->monochrome_palette_flag = get_bits1(gb);
>               pps->luma_bit_depth_entry = get_ue_golomb_31(gb) + 8;
> -            if (!pps->monochrome_palette_flag)
> -                pps->chroma_bit_depth_entry = get_ue_golomb_31(gb) + 8;
> -
> -            if (pps->chroma_bit_depth_entry > 16 || pps->chroma_bit_depth_entry > 16)

Is Coverity complaining about this duplicate check? If so, you should 
mention that fixing the Coverity issue is a side effect of the 
compliance check you're applying, rather than the actual objective of 
the change.

You could also replace one chroma_bit_depth_entry check with 
luma_bit_depth_entry in one commit, fixing the Coverity issue, then this 
compliance change in another.

> +            if (pps->luma_bit_depth_entry != sps->bit_depth)
>                   return AVERROR_INVALIDDATA;
> +            if (!pps->monochrome_palette_flag) {
> +                pps->chroma_bit_depth_entry = get_ue_golomb_31(gb) + 8;
> +                if (pps->chroma_bit_depth_entry != sps->bit_depth_chroma)
> +                    return AVERROR_INVALIDDATA;
> +            }
>   
>               num_comps = pps->monochrome_palette_flag ? 1 : 3;
>               for (int comp = 0; comp < num_comps; comp++) {

LGTM either way.