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

[Peter Ross <pross@xvid.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1478 bytes --]

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)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".