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

[Michael Niedermayer]

Hi

On Wed, Oct 03, 2007 at 05:03:13PM -0400, Jeff Downs wrote:
[...]
> Separated patches to be applied in order, w/ suggested commit messages:
> 
> 1. paff-mbdecode.patch 
> Partial PAFF implementation at macroblock level.
> PAFF support disabled until implementation complete.
> 
> 2. paff-unreference.patch 
> Modify unreference_pic implementation with PAFF in mind.
> 
> 3. paff-unreference-indent.patch
> Re-indent unreference_pic.
> 
> 4. paff-shortrefmgmt.patch
> Further modularize short reference list management for upcoming PAFF 
> implementation.
> 
> 5. paff-longoprename.patch
> Rename variable to make sense in both field and frame contexts 
> (support of PAFF implementation).
> 
> 
> 
> Then there is the remainder that you didn't specifically ask to be split, 
> but that I've tried to split further with the intention of making it 
> easier to review.
> 
> 6. paff-currpicnum.patch
> Fix h->curr_pic_num for field pictures. Necessary for proper PAFF support.
> 
> 7. paff-keyframe.patch
> Fix Picture.key_frame setting to be compatible with frame and field 
> contexts. Part of PAFF implementation. Contributed in part by Neil Brown.
> 
> 8. paff-longrefmgmt.patch
> Reorganize long reference management to minimize code duplication in 
> upcoming PAFF implementation.
> 
> 9. paff-defreflist.patch
> Support functions and changes to default reference list creation for PAFF.
> 
> 10. paff-defreflist-indent.patch
> Reindent fill_default_ref_list after changes for PAFF
> 
> 11. paff-reordering.patch
> Support function and changes to reference picture reordering for PAFF.

very nicely split :)
i dont remember that anyone splited a patch like that lately ...


> 
> 
> 
> The remainder of the original patch is in paff-noident.patch. 
> Bulk is MMCO for fields and interleaving of fields into one AVPicture.
> 
> Figure we can try to get through the above, then separate even further 
> still if desired.
> 
> 	-Jeff


> Content-Description: Patch 1: mbdecode

looks ok


[...]

> Content-Description: Patch 2: unreference

ok

[...]

> Content-Description: Patch 3: unreference-indent

ok

[...]

> Content-Description: Patch 4: shortrefmgmt

ok

[...]

> Content-Description: Patch 5: longoprename

ok

[...]


> Content-Description: Patch 6: currpicnum

likely ok

[...]


> Content-Description: Patch 7: keyframe

ok

[...]


> Content-Description: Patch 8: longrefmgmt

ok


[...]

> +/**
> + * Split one reference list into field parts, interleaving by parity
> + * as per H.264 spec section 8.2.4.2.5. Output fields have their data pointers
> + * set to look at the actual start of data for that field.
> + *
> + * @param dest output list
> + * @param dest_len maximum number of fields to put in dest
> + * @param src the source reference list containing fields and/or field pairs
> + *            (aka short_ref/long_ref, or
> + *             refFrameListXShortTerm/refFrameListLongTerm in spec-speak)
> + * @param src_len number of Picture's in source (pairs and unmatched fields)
> + * @param parity the parity of the picture being decoded/needing
> + *        these ref pics (PICT_{TOP,BOTTOM}_FIELD)
> + * @return number of fields placed in dest
> + */
> +static int split_field_half_ref_list(Picture *dest, int dest_len,
> +                                     Picture *src,  int src_len,  int parity){
> +    int same   = 1;
> +    int same_i = 0;
> +    int opp_i  = 0;
> +    int out_i;
> +    int i;
> +
> +    for (out_i = 0; out_i < dest_len; out_i += i) {
> +        if (same && same_i < src_len) {
> +            i = split_field_copy(dest + out_i, src + same_i, parity, 1);
> +            same = !i;
> +            same_i++;
> +
> +        } else if (opp_i < src_len) {
> +            i = split_field_copy(dest + out_i, src + opp_i,
> +                                 PICT_FRAME - parity, 0);
> +            same = i;
> +            opp_i++;
> +
> +        } else {
> +            break;
> +        }
> +    }

i think the variables could be named better, especially "i" is not a
good name here


[...]

> +static int pic_num_extract(H264Context *h, int pic_num, int *structure){
> +    MpegEncContext * const s = &h->s;
> +    int ret;
> +
> +    if (FIELD_PICTURE) {
> +        ret = pic_num >> 1;
> +        if (pic_num & 1) {
> +            *structure = s->picture_structure;
> +        } else {
> +            *structure = PICT_FRAME - s->picture_structure;
> +        }
> +    } else {
> +        ret = pic_num;
> +        *structure = PICT_FRAME;
> +    }
> +    return ret;
> +}

*structure = s->picture_structure;
if(FIELD_PICTURE){
    if(!(pic_num & 1))
        *structure ^= 3;
    pic_num >>=1;
}
return pic_num;

remaining patch not reviewed (ill review it tomorrow unless you want to
split it further)

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071004/d070a085/attachment.pgp>