[FFmpeg-devel] [PATCH] Implement PAFF in H.264
Jeff Downs
heydowns
Fri Oct 5 20:23:48 CEST 2007
On Thu, 4 Oct 2007, Michael Niedermayer wrote:
[...]
> > 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
Used j because it was already defined and unused in this context. Added
new var named unref_pic.
> [...]
> > + } 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);
OK. Added the assert, changed the log message to a comment, removed the
reference clearing. The block will be a no-op when NDEBUG is defined.
Revised patch attached.
If ok, I'll follow up with revised indent patch to match prior to
application in svn.
-Jeff
-------------- next part --------------
--- ../ffmpeg-mmcorefrename/libavcodec/h264.c 2007-10-04 14:30:07.000000000 -0400
+++ libavcodec/h264.c 2007-10-05 14:16:20.000000000 -0400
@@ -3465,42 +3465,80 @@
av_log(h->s.avctx, AV_LOG_DEBUG, "no mmco here\n");
for(i=0; i<mmco_count; i++){
+ int structure, frame_num, unref_pic;
if(s->avctx->debug&FF_DEBUG_MMCO)
av_log(h->s.avctx, AV_LOG_DEBUG, "mmco:%d %d %d\n", h->mmco[i].opcode, h->mmco[i].short_pic_num, h->mmco[i].long_arg);
switch(mmco[i].opcode){
case MMCO_SHORT2UNUSED:
- pic= remove_short(h, mmco[i].short_pic_num);
- if(pic)
- unreference_pic(h, pic, 0);
- else if(s->avctx->debug&FF_DEBUG_MMCO)
- av_log(h->s.avctx, AV_LOG_DEBUG, "mmco: remove_short() failure\n");
+ if(s->avctx->debug&FF_DEBUG_MMCO)
+ av_log(h->s.avctx, AV_LOG_DEBUG, "mmco: unref short %d count %d\n", h->mmco[i].short_pic_num, h->short_ref_count);
+ frame_num = pic_num_extract(h, mmco[i].short_pic_num, &structure);
+ pic = find_short(h, frame_num, &j);
+ if (pic) {
+ if (unreference_pic(h, pic, structure ^ PICT_FRAME))
+ remove_short_at_index(h, j);
+ } else if(s->avctx->debug&FF_DEBUG_MMCO)
+ av_log(h->s.avctx, AV_LOG_DEBUG, "mmco: unref short failure\n");
break;
case MMCO_SHORT2LONG:
- pic= remove_long(h, mmco[i].long_arg);
- if(pic) unreference_pic(h, pic, 0);
+ if (FIELD_PICTURE && mmco[i].long_arg < h->long_ref_count &&
+ h->long_ref[mmco[i].long_arg]->frame_num ==
+ mmco[i].short_pic_num / 2) {
+ /* do nothing, we've already moved this field pair. */
+ } else {
+ int frame_num = mmco[i].short_pic_num >> FIELD_PICTURE;
+
+ pic= remove_long(h, mmco[i].long_arg);
+ if(pic) unreference_pic(h, pic, 0);
- h->long_ref[ mmco[i].long_arg ]= remove_short(h, mmco[i].short_pic_num);
+ h->long_ref[ mmco[i].long_arg ]= remove_short(h, frame_num);
if (h->long_ref[ mmco[i].long_arg ]){
h->long_ref[ mmco[i].long_arg ]->long_ref=1;
h->long_ref_count++;
}
+ }
break;
case MMCO_LONG2UNUSED:
- pic= remove_long(h, mmco[i].long_arg);
- if(pic)
- unreference_pic(h, pic, 0);
- else if(s->avctx->debug&FF_DEBUG_MMCO)
- av_log(h->s.avctx, AV_LOG_DEBUG, "mmco: remove_long() failure\n");
+ j = pic_num_extract(h, mmco[i].long_arg, &structure);
+ pic = h->long_ref[j];
+ if (pic) {
+ if (unreference_pic(h, pic, structure ^ PICT_FRAME))
+ remove_long_at_index(h, j);
+ } else if(s->avctx->debug&FF_DEBUG_MMCO)
+ av_log(h->s.avctx, AV_LOG_DEBUG, "mmco: unref long failure\n");
break;
case MMCO_LONG:
+ unref_pic = 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 */
+ unref_pic = 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");
+ unref_pic = 0;
+ }
+ }
+
+ if (unref_pic) {
pic= remove_long(h, mmco[i].long_arg);
if(pic) unreference_pic(h, pic, 0);
h->long_ref[ mmco[i].long_arg ]= s->current_picture_ptr;
h->long_ref[ mmco[i].long_arg ]->long_ref=1;
h->long_ref_count++;
+ }
+ s->current_picture_ptr->reference |= s->picture_structure;
current_ref_assigned=1;
break;
case MMCO_SET_MAX_LONG:
@@ -3525,6 +3563,34 @@
}
}
+ if (!current_ref_assigned && FIELD_PICTURE &&
+ !s->first_field && s->current_picture_ptr->reference) {
+
+ /* Second field of complementary field pair; the first field of
+ * which is already referenced. If short referenced, it
+ * should be first entry in short_ref. If not, it must exist
+ * in long_ref; trying to put it on the short list here is an
+ * error in the encoded bit stream (ref: 7.4.3, NOTE 2 and 3).
+ */
+ if (h->short_ref_count && h->short_ref[0] == s->current_picture_ptr) {
+ /* Just mark the second field valid */
+ s->current_picture_ptr->reference = PICT_FRAME;
+ } else if (s->current_picture_ptr->long_ref) {
+ av_log(h->s.avctx, AV_LOG_ERROR, "illegal short term reference "
+ "assignment for second field "
+ "in complementary field pair "
+ "(first field is long term)\n");
+ } else {
+ /*
+ * First field in reference, but not in any sensible place on our
+ * reference lists. This shouldn't happen unless reference
+ * handling somewhere else is wrong.
+ */
+ assert(0);
+ }
+ current_ref_assigned = 1;
+ }
+
if(!current_ref_assigned){
pic= remove_short(h, s->current_picture_ptr->frame_num);
if(pic){
@@ -3538,6 +3604,7 @@
h->short_ref[0]= s->current_picture_ptr;
h->short_ref[0]->long_ref=0;
h->short_ref_count++;
+ s->current_picture_ptr->reference |= s->picture_structure;
}
print_short_term(h);
More information about the ffmpeg-devel
mailing list