Re: [FFmpeg-devel] [PATCH v2] avcodec/libwebpenc_anim: support setting the duration of the last frame

[James Zern via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>]

Hi,

On Fri, Feb 28, 2025 at 1:47 AM wangyaqiang via ffmpeg-devel
<ffmpeg-devel@ffmpeg.org> wrote:
>
> From: Wang Yaqiang <wangyaqiang03@kuaishou.com>
>
> Signed-off-by: Wang Yaqiang <wangyaqiang03@kuaishou.com>
> ---
>  libavcodec/libwebpenc_animencoder.c | 17 +++++++++++++++--
>  libavcodec/libwebpenc_common.c      |  2 ++
>  libavcodec/libwebpenc_common.h      |  1 +
>  3 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/libwebpenc_animencoder.c b/libavcodec/libwebpenc_animencoder.c
> index c5361d7f92..558fcc246d 100644
> --- a/libavcodec/libwebpenc_animencoder.c
> +++ b/libavcodec/libwebpenc_animencoder.c
> @@ -38,6 +38,7 @@ typedef struct LibWebPAnimContext {
>      WebPAnimEncoder *enc;     // the main AnimEncoder object
>      int64_t first_frame_pts;  // pts of the first encoded frame.
>      int64_t end_pts;          // pts + duration of the last frame
> +    int64_t prev_frame_pts;   // pts of the previously encoded frame.
>
>      void           *first_frame_opaque;
>      AVBufferRef    *first_frame_opaque_ref;
> @@ -72,7 +73,19 @@ static int libwebp_anim_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>          if (s->done) {  // Second flush: return empty package to denote finish.
>              *got_packet = 0;
>              return 0;
> -        } else {  // First flush: assemble bitstream and return it.
> +        } else {
> +            if (s->cc.last_delay > 0) {
> +                int timestamp_ms = avctx->time_base.num * (s->cc.last_delay + s->prev_frame_pts) * 1000 / avctx->time_base.den;
> +                ret = WebPAnimEncoderAdd(s->enc, NULL, timestamp_ms, &s->cc.config);
> +                if (!ret) {
> +                    av_log(avctx, AV_LOG_ERROR,
> +                           "Encoding WebP frame failed!\n");
> +                    ret = AVERROR_EXTERNAL;
> +                    goto end;
> +                }
> +                s->end_pts = s->prev_frame_pts + s->cc.last_delay;
> +            }
> +            // First flush: assemble bitstream and return it.
>              WebPData assembled_data = { 0 };
>              ret = WebPAnimEncoderAssemble(s->enc, &assembled_data);
>              if (ret) {
> @@ -137,7 +150,7 @@ static int libwebp_anim_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>
>          if (frame->pts != AV_NOPTS_VALUE)
>              s->end_pts = frame->pts + frame->duration;
> -
> +        s->prev_frame_pts = frame->pts;
>          ret = 0;
>          *got_packet = 0;
>
> diff --git a/libavcodec/libwebpenc_common.c b/libavcodec/libwebpenc_common.c
> index 80040ea9e3..13fb6aa690 100644
> --- a/libavcodec/libwebpenc_common.c
> +++ b/libavcodec/libwebpenc_common.c
> @@ -49,6 +49,8 @@ static const AVOption options[] = {
>      { "cr_threshold","Conditional replenishment threshold",     OFFSET(cr_threshold), AV_OPT_TYPE_INT, { .i64 =  0  },  0, INT_MAX, VE           },
>      { "cr_size"     ,"Conditional replenishment block size",    OFFSET(cr_size)     , AV_OPT_TYPE_INT, { .i64 =  16 },  0, 256,     VE           },
>      { "quality"     ,"Quality",                OFFSET(quality),  AV_OPT_TYPE_FLOAT, { .dbl =  75 }, 0, 100,                         VE           },
> +    { "final_delay", "Force delay (in centiseconds) after the last frame", OFFSET(last_delay),
> +      AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, VE },

This description doesn't sound right as the value looks like it's
being interpreted as if it's in the source timebase. Why not accept a
value in milliseconds and apply that to the final frame? There's also
a risk of overflow as timestamp_ms is an int, though that's ignored in
the existing calculation.
_______________________________________________
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".