[FFmpeg-devel] [PATCH 2/3] avcodec/rv60: RealVideo 6.0 decoder

[Anton Khirnov]

Quoting Peter Ross (2023-10-22 01:31:32)
> On Wed, Oct 18, 2023 at 04:42:01PM +0200, Anton Khirnov wrote:
> > Quoting Peter Ross (2023-10-18 10:03:54)
> [..]
> > I think you can simplify this into:
> >     if (s->last_frame[NEXT_PIC]->data[0]) {
> >         av_frame_move_ref(frame, s->last_frame[NEXT_PIC]);
> >         *got_frame = 1;
> >     }
> [..]
> > You just unreffed the frame above, what is the point of using
> > reget_buffer()?
> [..]
> > The generic code should be doing this already.
> [..]
> > You could change this into av_frame_move_ref() and drop the unref below.
> 
> many thanks anton for these suggestions. i agree with all of them.
> 
> > > +    if (s->pict_type != AV_PICTURE_TYPE_B) {
> > > +        s->ref_pts[0] = s->ref_pts[1];
> > > +        s->ref_pts[1] = avpkt->pts;
> > > +
> > > +        s->ref_ts[0] = s->ref_ts[1];
> > > +        s->ref_ts[1] = s->ts;
> > > +
> > > +        if (s->ref_pts[1] > s->ref_pts[0] && s->ref_ts[1] > s->ref_ts[0])
> > > +            s->ts_scale = (s->ref_pts[1] - s->ref_pts[0]) / (s->ref_ts[1] - s->ref_ts[0]);
> > > +    } else {
> > > +        frame->pts = s->ref_pts[0] + (s->ts - s->ref_ts[0]) * s->ts_scale;
> > 
> > This looks immensely evil. Isn't ff_get_buffer() already setting the
> > timestamps correctly?
> 
> no ff_reget_buffer() does not set pts correctly.
> the ref_ts ('s->ts') value is sent with every rv60 in the header, and is
> used to calculate the pts value. unsure how to make this less evil.

Typically the timestamps come from the container, so the decoder merely
copies them from packet to frame. Does the container not provide valid
timestamps here?

-- 
Anton Khirnov