On Fri, 25 Jul 2008, Michael Niedermayer wrote:
On Thu, Sep 20, 2007 at 03:04:50PM -0400, Jeff Downs wrote:
On Wed, 19 Sep 2007, Michael Niedermayer wrote: [...] case MMCO_LONG: - pic= remove_long(h, mmco[i].long_arg); - if(pic) unreference_pic(h, pic); + 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->valid_structure) { + /* First field in pair in short term list. + * This is not allowed; see 7.4.3, notes 2 and 3. + */ + 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)\n"); + j = 0; [...] - if(!current_is_long){ - pic= remove_short(h, s->current_picture_ptr->frame_num); + if (!current_ref_assigned && FIELD_PICTURE && + !s->first_field && s->current_picture_ptr->valid_structure) { + + /* 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 on neither list, we have a logic problem elsewhere + */
the notes 2 & 3 in 7.4.3 do not say anything that would support that view.
Yes. Apologies - the citation as you noticed should be 7.4.3.3. I've changed this.
the notes 2 & 3 in 7.4.3.3 could be bent toward this given the assumtation that the stream does not contain non paired reference fields. What made you think such fields do not occur? No i dont have a stream that i know contains such things. iam just curious if theres something in the spec that supports this view.
The first hunk quoted up there is wrong. I misread spec to be that you cannot have second field in a complementary pair be marked long unless the first had already been marked long. That doesn't hold here. The only restriction is that if the first was made long, then the second must be too. It looks like that comment is the only thing surviving there, and can probably be removed if you agree. Second hunk... Both ways to assign long term to a field (IDR long term ref flag, MMCO current is long, and MMCO short to long) specify that if applied to the first, you have to apply the same to the second field in a pair. Unpaired fields aren't considered by that comment/code (first_field == 1 for unpaired fields); the restriction doesn't apply. All that said, maybe its better to just remove this check and put the field on the short list anyway. I was unsure of how to handle this case since it shouldn't happen. In light of the comments on the first hunk, I do not know if the code properly handles field pairs on mixed lists or not. I am still catching up on the recent work that's been done. -Jeff