[Ffmpeg-devel] few remarks for h264 decoder
Fri Dec 30 21:16:16 CET 2005
First of all sorry for not making patches or more detailed descriptions, but I don't
have more time for these problems and I'am not depending on ffmpeg's h264
decoding accuracy. Here are the isseus I think(!) I found:
pred_direct_motion() uses 8bit width fill_rectangle() with ref,ref
which can be negative numbers causing trouble with val*0x0101 in fill_rectangle()
ff_h264_weight_WxH_mmx2() can overflow. "paddw" should be "paddsw"
example log2_denom = 6, offset = -120<<log2_denom, weight = -120
255 * weight + offset = -38280 (overflows mmx 16bit)
Probably ff_h264_biweight_WxH_mmx2 is effected as well, but I'am not
sure "paddsw" doesn't cause trouble because of the three components added together...
In most cases b-frames use implicit weights anyway (which shouldn't overflow)
pred_direct_motion() fills ref_cache[list][scan8] and ref_cache[list][scan8]
before they are used by pred_motion(). They should be PART_NOT_AVAILABLE
until pred_motion() finishes.
Probably a known problem, but b-frame deblocking bs calculation based
on mv_cache condition is far from standard. I guess it's because b-frames
are mostly non reference frames so minor deblocking bug is not a big problem.
There is also a possible problem if the last slice is B_TYPE and it's used in the
upper MB row of the next P_TYPE slice for vertical edge deblocking.
I didn't check it throughly but mmx h264_qpel4_hv_lowpass macro uses
approximation (comment sais ((a-b)/4-b)/4). I know a proper solution
needs 32bit range, but this is why God created "pmaddwd" :) I think the
performance loss needed for accurate calculation would be minimal.
direct_ref_list_init() saves the poc of reference frames in the current Picture.
But what happens if the current frame is decoded with more slices that uses
decode_ref_pic_list_reordering() to reorder the default reference list or maybe
even have different ref_count?
fill_default_ref_list() should be recalled when multiple b-slices have
different h->ref_count. Other solution is to remove the "index < h->ref_count[list]"
condition from fill_default_ref_list's b-frame loop.
There is a problem with pred_direct_motion() when direct_8x8_inference_flag=1
In this case the far corners of the macroblock should be used for each 8x8 block.
What should be:
Actually the current code calculates all the 16 motion vectors and
only sub_mb_type flag tells hl_motion() to use 8x8 blocks. In theory the
non used motion vectors still can have invalid values and cause trouble.
More information about the ffmpeg-devel