From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <ffmpeg-devel-bounces@ffmpeg.org>
Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100])
	by master.gitmailbox.com (Postfix) with ESMTPS id 452184F3AB
	for <ffmpegdev@gitmailbox.com>; Mon, 16 Jun 2025 14:36:24 +0000 (UTC)
Received: from [127.0.1.1] (localhost [127.0.0.1])
	by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id A487568DBFC;
	Mon, 16 Jun 2025 17:36:19 +0300 (EEST)
Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net
 [217.70.183.196])
 by ffbox0-bg.ffmpeg.org (Postfix) with ESMTPS id 05C7A68D63F
 for <ffmpeg-devel@ffmpeg.org>; Mon, 16 Jun 2025 17:36:12 +0300 (EEST)
Received: by mail.gandi.net (Postfix) with ESMTPSA id 3FCC8444CF;
 Mon, 16 Jun 2025 14:36:12 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc;
 s=gm1; t=1750084572;
 h=from:from:reply-to:subject:subject:date:date:message-id:message-id:
 to:to:cc:mime-version:mime-version:content-type:content-type:
 in-reply-to:in-reply-to:references:references;
 bh=3bMEUmTvA8RjG2ZRcxe1k50V8Vu2p08dESQRQMFHBNc=;
 b=btpyODkSJj3FcHy+2WiPfdHZMtP3wYo1ahbh0idb/ZXBff+7K1Bsa8NFP5bZY9gGWtjFq/
 8kbWHDn+/XrYnxvSc1qWSbvII1GFSq7/1sM1KW2Y867gL3Etf92QqwhP4GUt48mY4s5xT6
 U+nWF5INw7usZPb+tuCIaYJ8XgbVUH8i0CadEKgw11iLCfQ+xJXGAr1fVoEmmqRLppOLaT
 OYEMUxg2ZFrq1Cw+gaAfdz89rN+wur/wgCAZ1tyw4M3NIh4XdquVYZ8xf6QAfVMKiaT5Yh
 0dYu7zUsGeATfyqE4elp6MG9EYInJvcosPkJuszo9YUIVqGwbdJOx2y44At2aw==
Date: Mon, 16 Jun 2025 16:36:11 +0200
From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>,
 Peter Xia <aattyy@gmail.com>
Message-ID: <20250616143611.GH29660@pb2>
References: <20250404011719.21654-1-aattyy@gmail.com>
MIME-Version: 1.0
In-Reply-To: <20250404011719.21654-1-aattyy@gmail.com>
X-GND-State: clean
X-GND-Score: -70
X-GND-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtddugddvieekfecutefuodetggdotefrodftvfcurfhrohhfihhlvgemucfitefpfffkpdcuggftfghnshhusghstghrihgsvgenuceurghilhhouhhtmecufedtudenucesvcftvggtihhpihgvnhhtshculddquddttddmnegfrhhlucfvnfffucdlfedtmdenucfjughrpeffhffvuffkfhggtggujgesghdtreertddtvdenucfhrhhomhepofhitghhrggvlhcupfhivgguvghrmhgrhigvrhcuoehmihgthhgrvghlsehnihgvuggvrhhmrgihvghrrdgttgeqnecuggftrfgrthhtvghrnhepieegkedtjeduffejhfetgeejtdegteetgfegtdfhjefgvefhteegkeejtddvhfevnecukfhppeeguddrieeirdeijedruddufeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepihhnvghtpeeguddrieeirdeijedruddufedphhgvlhhopehlohgtrghlhhhoshhtpdhmrghilhhfrhhomhepmhhitghhrggvlhesnhhivgguvghrmhgrhigvrhdrtggtpdhnsggprhgtphhtthhopedvpdhrtghpthhtohepfhhfmhhpvghgqdguvghvvghlsehffhhmphgvghdrohhrghdprhgtphhtthhopegrrghtthihhiesghhmrghilhdrtghomh
X-GND-Sasl: michael@niedermayer.cc
Subject: Re: [FFmpeg-devel] [PATCH v2] avcodec/libwebpdec: Add libwebp WebP
 decoder.
X-BeenThere: ffmpeg-devel@ffmpeg.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: FFmpeg development discussions and patches <ffmpeg-devel.ffmpeg.org>
List-Unsubscribe: <https://ffmpeg.org/mailman/options/ffmpeg-devel>,
 <mailto:ffmpeg-devel-request@ffmpeg.org?subject=unsubscribe>
List-Archive: <https://ffmpeg.org/pipermail/ffmpeg-devel>
List-Post: <mailto:ffmpeg-devel@ffmpeg.org>
List-Help: <mailto:ffmpeg-devel-request@ffmpeg.org?subject=help>
List-Subscribe: <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>,
 <mailto:ffmpeg-devel-request@ffmpeg.org?subject=subscribe>
Reply-To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Content-Type: multipart/mixed; boundary="===============4176156074075434774=="
Errors-To: ffmpeg-devel-bounces@ffmpeg.org
Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org>
Archived-At: <https://master.gitmailbox.com/ffmpegdev/20250616143611.GH29660@pb2/>
List-Archive: <https://master.gitmailbox.com/ffmpegdev/>
List-Post: <mailto:ffmpegdev@gitmailbox.com>


--===============4176156074075434774==
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="52ugmRoHZZjKz1sc"
Content-Disposition: inline


--52ugmRoHZZjKz1sc
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi Peter

full review below as our native decoder still is not finished

[...]
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index f10519617e..5f8f682111 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c

> @@ -810,8 +810,9 @@ extern FFCodec ff_libvpx_vp9_encoder;
>  extern const FFCodec ff_libvpx_vp9_decoder;
>  extern const FFCodec ff_libvvenc_encoder;
>  /* preferred over libwebp */
> -extern const FFCodec ff_libwebp_anim_encoder;
>  extern const FFCodec ff_libwebp_encoder;
> +extern const FFCodec ff_libwebp_anim_encoder;

please dont reorder things in this patch, its unrelated



> +extern const FFCodec ff_libwebp_decoder;
>  extern const FFCodec ff_libx262_encoder;
>  extern const FFCodec ff_libx264_encoder;
>  extern const FFCodec ff_libx264rgb_encoder;
> diff --git a/libavcodec/libwebpdec.c b/libavcodec/libwebpdec.c
> new file mode 100644
> index 0000000000..e9ca65ef05
> --- /dev/null
> +++ b/libavcodec/libwebpdec.c
> @@ -0,0 +1,192 @@
> +/*
> + * LibWebP decoder
> + * Copyright (c) 2025 Peter Xia
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1=
301 USA
> + */
> +
> +/**
> + * @file
> + * LibWebP decoder
> + */
> +
> +#include "decode.h"
> +#include "codec_internal.h"
> +#include "libavutil/avutil.h"
> +#include "libavutil/opt.h"
> +#include "libavformat/avformat.h"
> +#include "libavformat/avio.h"
> +
> +#include <webp/demux.h>
> +#include <webp/decode.h>
> +
> +typedef struct AnimatedWebPContext
> +{
> +    const AVClass *class;
> +    WebPAnimDecoderOptions dec_options;
> +    WebPAnimDecoder *dec;
> +    AVBufferRef *file_content;
> +    WebPData webp_data; // references |file_content|
> +    uint32_t loop_to_send;
> +    uint32_t loop_sent;
> +
> +    // --- Options ---
> +    int ignore_loop;
> +} AnimatedWebPContext;
> +
> +// Initialize the decoder context
> +static av_cold int decode_libwebp_init(AVCodecContext *avctx)
> +{
> +    AnimatedWebPContext *s =3D avctx->priv_data;
> +
> +    if (!WebPAnimDecoderOptionsInit(&s->dec_options)) {
> +        av_log(avctx, AV_LOG_DEBUG, "Cannot initialize WebPAnimDecoderOp=
tions\n");
> +        return AVERROR(ENOMEM);
> +    }
> +    s->dec_options.color_mode =3D MODE_RGBA;
> +    s->dec_options.use_threads =3D 1;
> +    s->file_content =3D NULL;
> +    s->loop_sent =3D 0;
> +
> +    avctx->pix_fmt =3D AV_PIX_FMT_RGBA;

> +    avctx->pkt_timebase =3D av_make_q(1, 1000);

violates API "* - decoding: set by user." the decoder is not the user


> +    avctx->framerate =3D av_make_q(1, 0);
> +
> +    av_log(avctx, AV_LOG_DEBUG, "Animated WebP decoder initialized.\n");
> +    return 0; // Success
> +}
> +
> +/**
> + * Decode one frame of the animated WebP.
> + * This function will be called multiple times by FFmpeg.
> + * The first call receives the AVPacket with the full WebP file.
> + * Subsequent calls receive empty AVPacket until all frames are decoded.
> + */
> +static int decode_libwebp_frame(AVCodecContext *avctx, AVFrame *p,
> +                                int *got_frame, AVPacket *avpkt)
> +{
> +    WebPAnimInfo anim_info;
> +    uint8_t *frame_rgba;
> +    int timestamp_ms;
> +
> +    AnimatedWebPContext *s =3D avctx->priv_data;
> +    int ret =3D avpkt->size;
> +
> +    // Initialization Phase (First Call)
> +    // |avpkt| contains the entire file.
> +    if (!s->dec) {
> +        if (!avpkt || avpkt->size <=3D 0) {
> +            // Should not happen on the first call, but check anyway.
> +            av_log(avctx, AV_LOG_ERROR, "No input data provided on first=
 call.\n");
> +            return AVERROR(EINVAL);
> +        }
> +
> +        // Store entire WebP file in memory.

> +        s->file_content =3D av_buffer_ref(avpkt->buf);
> +        s->webp_data.bytes =3D s->file_content->data;

Missing failure check for av_buffer_ref()



> +        s->webp_data.size =3D s->file_content->size;
> +
> +        s->dec =3D WebPAnimDecoderNew(&s->webp_data, &s->dec_options);
> +        if (!s->dec) {
> +            av_log(avctx, AV_LOG_ERROR, "Error creating WebPAnimDecoder.=
\n");
> +            av_buffer_unref(&s->file_content);
> +            return AVERROR(ENOMEM);
> +        }
> +

> +        WebPAnimDecoderGetInfo(s->dec, &anim_info);
> +
> +        s->loop_to_send =3D s->ignore_loop ? 1 : anim_info.loop_count;

> +        avctx->width =3D anim_info.canvas_width;
> +        avctx->coded_width =3D anim_info.canvas_width;
> +        avctx->height =3D anim_info.canvas_height;
> +        avctx->coded_height =3D anim_info.canvas_height;

probably this should use ff_set_dimensions()


> +        avctx->framerate =3D av_make_q(1, 0);

Why ?


> +    }
> +
> +    if (!WebPAnimDecoderHasMoreFrames(s->dec)) {
> +        s->loop_sent++;
> +        WebPAnimDecoderReset(s->dec);
> +    }
> +
> +    if (s->loop_sent >=3D s->loop_to_send) {
> +        av_log(avctx, AV_LOG_DEBUG, "End of animated WebP stream.\n");
> +        return AVERROR_EOF;
> +    }
> +
> +
> +    if (!WebPAnimDecoderGetNext(s->dec, &frame_rgba, &timestamp_ms)) {
> +        av_log(avctx, AV_LOG_ERROR, "Error getting next frame from WebPA=
nimDecoder.\n");
> +        return AVERROR(EINVAL);
> +    }
> +
> +    ret =3D ff_get_buffer(avctx, p, 0);

> +    if (ret) {

ret < 0


> +        av_log(avctx, AV_LOG_ERROR, "Failed to allocated buffer.\n");
> +        return AVERROR(ENOMEM);
> +    }
> +

> +    p->width =3D avctx->width;
> +    p->height =3D avctx->height;
> +    p->format =3D AV_PIX_FMT_RGBA;

why is this needed ?


> +    p->pts =3D timestamp_ms;

> +    p->pkt_dts =3D 0;

why ?


> +    p->pict_type =3D AV_PICTURE_TYPE_I;
> +

> +    memcpy(p->data[0], frame_rgba, p->width * p->height * 4);

ignores linesize also cant the lib render directly into the frame


[...]

> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 0d4156e74a..026609f410 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -30,7 +30,7 @@
>  #include "version_major.h"
> =20
>  #define LIBAVCODEC_VERSION_MINOR   0
> -#define LIBAVCODEC_VERSION_MICRO 101
> +#define LIBAVCODEC_VERSION_MICRO 102

minor should be bumped

thx

[...]
--=20
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes

--52ugmRoHZZjKz1sc
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCaFAr1AAKCRBhHseHBAsP
q80MAJwJv4VJFyek48hT1cdpDZ5fU0CzFwCfTY6KAFFpOvdoxsfzpyfRUSUsu60=
=TsRN
-----END PGP SIGNATURE-----

--52ugmRoHZZjKz1sc--

--===============4176156074075434774==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
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".

--===============4176156074075434774==--