Hi On Thu, Oct 04, 2007 at 03:07:09PM -0400, Jeff Downs wrote:
On Thu, 4 Oct 2007, Jeff Downs wrote:
I'll continue to split as I have time, given how productive this round was.
OK, here is the next round of split patches. I took these diffs against a tree with the defreflist and reordering patches (posted short time ago) applied, but they will likely also apply clean to current svn.
Again, in suggested application order with commit message.
1. paff-mbaddress.patch Modifies macroblock addressing and current macroblock y-position for field decoding. Part of PAFF implementation.
2. paff-poc.patch Set Picture.poc for fields and field pairs. Part of PAFF implementation.
3. paff-emuedge.patch Edge emulation for fields. Part of PAFF implementation
4. paff-mmcodecode.patch Augment mmcodecoding process to work properly with fields. Part of PAFF implementation
5. paff-mmcorefrename.patch Cosmetic renaming variable so that it makes more sense for forthcoming PAFF mmco patches.
6. paff-mmco.patch Augment MMCO execution to work with both fields and frames. Part of PAFF implementation
7. paff-mmco-indent.patch Re-indent after PAFF MMCO implementation patch.
8. paff-picmgmt.patch Manage Picture buffers for fields as well as frames. Pair complementary fields into one MPV Picture. Part of PAFF implementation.
After these, there are two more -- one to indent after #8 and one to actually enable PAFF handling by properly defining FIELD_PICTURE. Since the indent one is quite large, I'll wait until these can be looked at to post the last two.
-Jeff
Content-Description: Patch 1: mbaddress [...] @@ -3928,6 +3928,11 @@ } s->resync_mb_x = s->mb_x = first_mb_in_slice % s->mb_width; s->resync_mb_y = s->mb_y = (first_mb_in_slice / s->mb_width) << h->mb_aff_frame; + if (FIELD_PICTURE) { + s->resync_mb_y = s->mb_y = s->mb_y * 2; + if (s->picture_structure == PICT_BOTTOM_FIELD) + s->resync_mb_y = s->mb_y = s->mb_y + 1; + } assert(s->mb_y < s->mb_height);
i think this breaks the check above this hunk [...]
@@ -6666,7 +6673,7 @@ s->mb_x = 0; ff_draw_horiz_band(s, 16*s->mb_y, 16); ++s->mb_y; - if(FRAME_MBAFF) { + if(FRAME_MBAFF || FIELD_PICTURE) {
this really should be FRAME_MBAFF_OR_FIELD or something like that [...]
Content-Description: Patch 2: poc [...] ok
Content-Description: Patch 3: emuedge [...] ok
Content-Description: Patch 4: MMCO decode [...] ok
Content-Description: Patch 5: MMCOrefrename [...] ok
Content-Description: Patch 6: MMCO [...] case MMCO_LONG: + j = 1; + if (FIELD_PICTURE && !s->first_field) { + if (h->long_ref[mmco[i].long_arg] == s->current_picture_ptr) { + /* Just mark second field as referenced */ + j = 0; + } else if (s->current_picture_ptr->reference) { + /* First field in pair is in short term list or + * at a different long term index. + * This is not allowed; see 7.4.3, notes 2 and 3. + * Report the problem and keep the pair where it is, + * and mark this field valid. + */ + av_log(h->s.avctx, AV_LOG_ERROR, + "illegal long term reference assignment for second " + "field in complementary field pair (first field is " + "short term or has non-matching long index)\n"); + j = 0; + } + } + + if (j) {
please use a better name than j [...]
+ } else { + av_log(h->s.avctx, AV_LOG_ERROR, "problem in internal reference " + "list handling; marking second " + "field in pair finds first field " + "in reference, but not in any " + "ref list\n");
if this cannot happen unless our code is buggy te correct behavior is assert(0); [...]
Content-Description: Patch 7: MMCO indent
ok [...]
Content-Description: Patch 8: picmgmt
will review later also i think it would be better if you would post 1 patch per email instead of many as later can get quite confusing for everyone if some are approved some are not and some arent revied yet ... [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB While the State exists there can be no freedom; when there is freedom there will be no State. -- Vladimir Lenin