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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
