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

[Peter Ross]

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.

cheers,

-- 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/20231022/24fd94b7/attachment.sig>