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

[Peter Ross]

On Mon, Oct 23, 2023 at 12:14:52PM +0200, Anton Khirnov wrote:
> 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?

the container (rm) does not provide meaningful pts, even though there
b-frames are present in the stream.

should this fixup be done in a bitstream filter?

rv30/rv40 also uses rm container and suffers this problem.

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20231024/982892f6/attachment.sig>