[Ffmpeg-devel] few remarks for h264 decoder

[Loren Merritt]

On Sat, 31 Dec 2005, G?bor Kov?cs wrote:

>>> 3.
>>> pred_direct_motion() fills ref_cache[list][scan8[4]] and 
>>> ref_cache[list][scan8[12]] before they are used by pred_motion(). 
>>> They should be PART_NOT_AVAILABLE until pred_motion() finishes.
>>
>> Nothing uses the mvs in between. Maybe it's faster to avoid 
>> interpolating direct mvs in 8x8 blocks that
>> aren't direct mode, but otherwise I won't change it.
>
> I wasn't clear enough. What happens for non intra macroblocks having 
> partition_count == 4?
> First subtypes are parsed and if there is a direct subblock, 
> pred_direct_motion() is called.
> This call will fill all the ref_cache/mv_cache info for the direct 
> subblocks.
> The problem comes afterward. We continue to process the non direct 
> subblocks. First calling pred_motion() and adding a delta value parsed 
> from the bitstream. Here pred_motion() calls fetch_diagonal_mv() which 
> depends on the topright ref_cache if it's PART_NOT_AVAILABLE or not. 
> This is where preceding pred_direct_motion() could cause trouble (for 
> scan8[4] and scan8[12] positions).

I re-read pred_direct_motion(), and see
   if(is_b8x8 && !IS_DIRECT(h->sub_mb_type[i8]))
     continue;
Are you sure there's a problem?

>>> 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.
>> 
>> Explain. The only known problems are noted as /*FIXME*/.
>
> You mean the /*FIXME*/ about given frame occupy more reference positions?
> Maybe I'am wrong, but I meant another problem.  For b-frames what it does now
> (simplified. a,b are the two neighboring positions):
[...]

True, but that differs from the existing code only if:
A pic is in both L0 and L1 (special case of the /*FIXME*/) or
It's at a slice boundary and a pic is in L0 of one slice and L1 of the 
other slice (special case of your point #6).

So I'll throw it on the pile of "kinda ugly to fix, so I'll document the 
bug and then ignore it until some encoder other than JM makes such vids".

>>> 7.
>> fill_default_ref_list() is recalled at the beginning of every slice, 
>> whether or not anything has changed.
>
> I thought "default_ref_list_done" prevents calling fill_default_ref_list()
> if slice_type doesn't change (in the same frame).

Ok, that looks like a bug... except that I made a vid with various numbers 
of refs in each slice, and it still decodes correctly.

>>> 8.
>> direct_8x8_inference_flag=1 guarantees that the 4 mvs in a 8x8 block will 
>> have the same value, so it doesn't matter which one we use. And they are 
>> all always valid, unless that ref list wasn't used, in which case none are 
>> valid.
>
> Maybe I was not clear again. Example I didn't say it's about 
> direct_spatial_mv_pred.
> I think it possible to have a ref_list[1][0] reference frame with inter 
> MB_TYPE_8x8 macroblock having 4x4 subblocks using different mv values. 
> Although the actual motion vector with direct_spatial_mv_pred is predicted 
> with pred_motion(), the ref_list[1][0] motion vectors still play a role on 
> the final vectors.
> If they considered non moving they will nullify the predicted motion vector
> (when the references fit the conditions). So the problem is which part to use 
> from 4 mvs of ref_list[1][0]? At the moment all four are used independently
> and hl_motion() uses the topleft mv to do the 8x8 motion compensation.

Ok, I misread the spec. I thought direct_8x8_inference_flag did not 
affect decoding in any way, but simply guaranteed that this case doesn't 
occur in the stream.
fixed.

--Loren Merritt