HEVC slice parsing probably wrong
while looking for an issue in my own code I stumbled upon a potential problem in mkvmerge. From my point of view the calculation of length of the slice_segment_address field in hevc_es_parser.cpp - es_parser_c::parse_slice is not correct.
According to the H265 standard calculation should look like the following:
PicWidthInCtbsY = Ceil( pic_width_in_luma_samples ÷ CtbSizeY ) // ÷: floating point division PicHeightInCtbsY = Ceil( pic_height_in_luma_samples ÷ CtbSizeY ) PicSizeInCtbsY = PicWidthInCtbsY * PicHeightInCtbsY slice_segment_address length in bits = Ceil( Log2( PicSizeInCtbsY ) )
Currently the code in mkvmerge doesn't do floating point arithmetic although a double cast is used:
auto pic_width_in_ctbs_y = ceil(static_cast<double>(sps.width / ctb_size_y)); // (1) auto pic_height_in_ctbs_y = ceil(static_cast<double>(sps.height / ctb_size_y)); // (2)
But as the variables (sps.width, sps.height and ctb_size_y) are integer values the division will be an integer division - decimal places are cut off and ceiling will not take place.
The calculation of the number of bits is also wrong:
auto v = mtx::math::int_log2(pic_size_in_ctbs_y); // (3)
The function int_log2 does not include the "Ceil" call from the standard. A call to std::log2(pic_size_in_ctbs_y) will most likely return a value with decimal places. The function int_log2 simply discards these places. But due to ceil we need the next integer value that is not less than the result of std::log2. Therefore most of the time we will get a value one less than the value we need.
I would recommend the following changes:
For (1) and (2):
auto pic_width_in_ctbs_y = ceil(static_cast<double>(sps.width) / static_cast<double>(ctb_size_y)); auto pic_height_in_ctbs_y = ceil(static_cast<double>(sps.height) / static_cast<double>(ctb_size_y));
auto v = mtx::math::int_log2((pic_size_in_ctbs_y - 1) * 2); // -1 and *2 do the ceiling.
ffmpeg works more or less the same way:
libavcodec/hevcdec.c - hls_slice_header
slice_address_length = av_ceil_log2(s->ps.sps->ctb_width * s->ps.sps->ctb_height); av_ceil_log2(x): av_log2((x - 1) << 1);
libavcodec/hevc_ps.c - ff_hevc_parse_sps (nearly at the end of the function)
sps->ctb_width = (sps->width + (1 << sps->log2_ctb_size) - 1) >> sps->log2_ctb_size; sps->ctb_height = (sps->height + (1 << sps->log2_ctb_size) - 1) >> sps->log2_ctb_size; This code divides width and height by ctb_size with an included ceil: (1 << sps->log2_ctb_size) - 1 = divider - 1
I also created a patch for the three changed lines. The patch is based on version 41.0.0.
Kind regards, Torsten Hauska