From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH v9 2/6] avcodec/webp: separate VP8 decoding
Date: Sat, 3 Feb 2024 14:53:53 +0100
Message-ID: <AS8P250MB07444171BA3DF98A1EE083FB8F412@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <18fe4a45-06f6-48b9-9c2e-34ce2a48a108@mail.de>
Thilo Borgmann via ffmpeg-devel:
> Am 28.01.24 um 11:29 schrieb Anton Khirnov:
>> Quoting Thilo Borgmann via ffmpeg-devel (2024-01-25 16:39:19)
>>> Am 25.01.24 um 11:04 schrieb Anton Khirnov:
>>>> Quoting Thilo Borgmann via ffmpeg-devel (2023-12-31 13:30:14)
>>>>> ---
>>>>> libavcodec/webp.c | 50
>>>>> +++++++++++++++++++++++++++++++++++++++++------
>>>>> 1 file changed, 44 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/libavcodec/webp.c b/libavcodec/webp.c
>>>>> index 4fd107aa0c..58a20b73da 100644
>>>>> --- a/libavcodec/webp.c
>>>>> +++ b/libavcodec/webp.c
>>>>> @@ -194,6 +194,7 @@ typedef struct WebPContext {
>>>>> AVFrame *alpha_frame; /* AVFrame for alpha
>>>>> data decompressed from VP8L */
>>>>> AVPacket *pkt; /* AVPacket to be passed
>>>>> to the underlying VP8 decoder */
>>>>> AVCodecContext *avctx; /* parent AVCodecContext */
>>>>> + AVCodecContext *avctx_vp8; /* wrapper context for VP8
>>>>> decoder */
>>>>
>>>> Nested codec contexts are in general highly undesirable and should be
>>>> avoided whenever possible.
>>>
>>> AFAICT we do it that way in the other codecs as well (cri, ftr, imm5,
>>> tdsc, tiff). So what do you suggest to do to avoid having it nested?
>>
>> Integrating the two decoders directly, as is done now.
>>
>> With nesting it is very tricky to handle all the corner cases properly,
>> especially passing through all the options to the innner decoder, like
>> direct rendering, other user callbacks, etc. It should only be done as a
>> last resort and there should be a strong argument to do it this way.
>
> IIUC that was what the patch still did some some versions ago.
> It brought us the data races in animated files, decoupling the decoder
> solving the issue.
>
If one keeps the codecs integrated, then one needs at the very least
change the check for whether to call ff_thread_finish_setup() as I did:
https://ffmpeg.org/pipermail/ffmpeg-devel/2024-January/320490.html
This will not be enough: E.g. changing the dimensions in VP8 code and
then reverting that change in WebP (as has been done in the earlier
version of your patch which made me propose that these decoders should
be separated) will have to be avoided.
- Andreas
_______________________________________________
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".
next prev parent reply other threads:[~2024-02-03 13:52 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-31 12:30 [FFmpeg-devel] [PATCH v9 0/6] webp: add support for animated WebP decoding Thilo Borgmann via ffmpeg-devel
2023-12-31 12:30 ` [FFmpeg-devel] [PATCH v9 1/6] avcodec/webp: remove unused definitions Thilo Borgmann via ffmpeg-devel
2023-12-31 12:30 ` [FFmpeg-devel] [PATCH v9 2/6] avcodec/webp: separate VP8 decoding Thilo Borgmann via ffmpeg-devel
2024-01-25 10:04 ` Anton Khirnov
2024-01-25 15:39 ` Thilo Borgmann via ffmpeg-devel
2024-01-28 10:29 ` Anton Khirnov
2024-01-30 18:39 ` Thilo Borgmann via ffmpeg-devel
2024-02-03 13:53 ` Andreas Rheinhardt [this message]
2024-02-04 11:09 ` Thilo Borgmann via ffmpeg-devel
2023-12-31 12:30 ` [FFmpeg-devel] [PATCH v9 3/6] libavcodec/webp: add support for animated WebP Thilo Borgmann via ffmpeg-devel
2023-12-31 12:56 ` Tomas Härdin
2023-12-31 14:54 ` Thilo Borgmann via ffmpeg-devel
2023-12-31 15:17 ` Tomas Härdin
2023-12-31 12:30 ` [FFmpeg-devel] [PATCH v9 4/6] avcodec/webp: make init_canvas_frame static Thilo Borgmann via ffmpeg-devel
2023-12-31 12:30 ` [FFmpeg-devel] [PATCH v9 5/6] libavformat/webp: add WebP demuxer Thilo Borgmann via ffmpeg-devel
2023-12-31 12:59 ` Tomas Härdin
2023-12-31 14:55 ` Thilo Borgmann via ffmpeg-devel
2023-12-31 12:30 ` [FFmpeg-devel] [PATCH v9 6/6] fate: add test for animated WebP Thilo Borgmann via ffmpeg-devel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=AS8P250MB07444171BA3DF98A1EE083FB8F412@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM \
--to=andreas.rheinhardt@outlook.com \
--cc=ffmpeg-devel@ffmpeg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git