[FFmpeg-devel] HEVC parser - few small bugs and style issues

[Eran Kornblau]

Hi all,

I did some work around parsing HEVC headers, and while looking at ffmpeg's implementation, I noticed
a few bugs and a few style issues. I'm not submitting a patch, since my familiarity with the decoder is
very limited, but hopefully the maintainers will go over the list below and change what is needed.
I marked the two items I find most important with '>>>'.

hevc_ps.c
https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/hevc_ps.c#L117
                             k1 is unused

https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/hevc_ps.c#L142
              rps_ridx = &sps->st_rps[rps - sps->st_rps - 1];
                             this is equivalent to simply rps_ridx = sps - 1;

https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/hevc_ps.c#L156
              use_delta_flag = get_bits1(gb);
>>>                      missing else use_delta_flag = 1 (according to the spec, this is the default value of this field)

https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/hevc_ps.c#L182
              if (rps->num_delta_pocs != 0) {
                             the if is redundant, the for loop won't enter anyway

https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/hevc_ps.c#L198
              if ((rps->num_negative_pics >> 1) != 0) {
                             the if is redundant, the for loop won't enter anyway

https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/hevc_ps.c#L234
              prev -= delta_poc;
                             prev is unsigned and initialized to 0, so it will become negative here
                             type should probably be changed to int

hevc_refs.c
https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/hevc_refs.c#L522
        for (i = 0; i < rps->num_negative_pics; i++)
            ret += !!rps->used[i];
        for (; i < rps->num_delta_pocs; i++)
            ret += !!rps->used[i];
                             can combine to a single for loop (rps->num_delta_pocs = rps->num_negative_pics + nb_positive_pics;)
                             also, 'used' is always coming from get_bits1(gb) so the !! seems unneeded

hevcdec.c
https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/hevcdec.c#L604
              sh->short_term_rps = &s->ps.sps->st_rps[rps_idx];
>>>                      need to return error in case rps_idx >= s->ps.sps->nb_st_rps
                             (don't think if it can overflow the array, but it can reference an uninitialized rps)

Thanks

Eran