[FFmpeg-devel] [PATCH] Implement PAFF in H.264

[Martin Zlomek]

On Fri, 21 Sep 2007 18:36:32 +0200, Jeff Downs <heydowns at borg.com> wrote:

> On Thu, 20 Sep 2007, Martin Zlomek wrote:
>
>> The attached patch fixes picNumX in "Adaptive memory control
>> decoded reference picture marking process".
>>
>
>> --- h264.c.orig	2007-09-20 23:30:38.000000000 +0200
>> +++ h264.c	2007-09-20 23:31:42.000000000 +0200
>> @@ -3644,7 +3644,7 @@
>>
>>                  h->mmco[i].opcode= opcode;
>>                  if(opcode==MMCO_SHORT2UNUSED ||  
>> opcode==MMCO_SHORT2LONG){
>> -                    h->mmco[i].short_pic_num= h->curr_pic_num -  
>> get_ue_golomb(gb) - 1;
>> +                    h->mmco[i].short_pic_num= (h->curr_pic_num -  
>> get_ue_golomb(gb) - 1) & (h->max_pic_num - 1);
>>  /*                    if(h->mmco[i].short_frame_num >=  
>> h->short_ref_count || h->short_ref[ h->mmco[i].short_frame_num ] ==  
>> NULL){
>>                          av_log(s->avctx, AV_LOG_ERROR, "illegal short  
>> ref in memory management control operation %d\n", mmco);
>>                          return -1;
>
> Why?  I'm having trouble mapping this to anything in the spec. Compliant
> streams should contain data that produces a valid short_pic_num here.
> Non-compliant streams might produce a garbage pic num that is beyond
> maximum pic num, but that'll just cause the MMCO execution to skip this
> operation because it can't find a picture with that picture number.

You are right that this is not in the spec.
But using (short_)pic_num in execute_ref_pic_marking(), you are calculating
frame_num in remove_field_short() in MMCO_SHORT2UNUSED case and comparing
to frame_num in MMCO_SHORT2LONG case. pic_num can be negative, frame_num
should not - pic_num / 2 does not match frame_num in such cases...

PS: Be careful to that "x >> 1" and "x / 2" does not equal for negative
     odd values of x.


-- 
Martin Zlomek
martin.zlomek at email.cz