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

[Jeff Downs]

On Sat, 22 Sep 2007, Martin Zlomek wrote:

> >> --- 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.

Actually, I'm totally wrong here; sorry! It is in the spec in the pic id 
calculation and I overlooked it and the relationship to frame number.  
Your other patch thread made me realize whats going on.

Seems like this is the only change (from original PAFF implementation) for 
pic_id-related stuff if we consider Picture::pic_id for short terms not as 
a true picture id, but rather the "no wrap" version (basically frame_num 
for frames, 2*frame_num + parity for fields, aka pic_id & max_pic_num). 
And this is how the original frame implementation treated pic_id 
everywhere but here, which is good to keep the changes limited.

I'm going to throw this concept into the next iteration of the PAFF 
patch. Output of ffmpeg against the compliance file you cited (and others 
in the same group) match. Let me know if it doesn't address everything 
related to pic_id you posted about in this thread.

	-Jeff