[FFmpeg-devel] [PATCH 1/3] avcodec/vp8: Enforce key-frame only for WebP

[FFmpeg-devel] [PATCH 1/3] avcodec/vp8: Enforce key-frame only for WebP

[Andreas Rheinhardt]

VP8-in-WebP only uses key frame encoding (see [1]), yet this
is currently not enforced. This commit does so in order to
make output reproducible with frame-threading as the VP8 decoder's
update_thread_context is not called at all when using decoding
VP8-in-WebP (as this is unnecessary for key frame-only streams).

[1]: https://developers.google.com/speed/webp/docs/riff_container

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/vp8.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
index 83c60adeb0..7972775a1c 100644
--- a/libavcodec/vp8.c
+++ b/libavcodec/vp8.c
@@ -2665,7 +2665,11 @@ int vp78_decode_frame(AVCodecContext *avctx, AVFrame *rframe, int *got_frame,
     if (ret < 0)
         goto err;
 
-    if (s->actually_webp) {
+    if (!is_vp7 && s->actually_webp) {
+        // VP8 in WebP is supposed to be intra-only. Enforce this here
+        // to ensure that output is reproducible with frame-threading.
+        if (!s->keyframe)
+            return AVERROR_INVALIDDATA;
         // avctx->pix_fmt already set in caller.
     } else if (!is_vp7 && s->pix_fmt == AV_PIX_FMT_NONE) {
         s->pix_fmt = get_pixel_format(s);
-- 
2.34.1

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

[FFmpeg-devel] [PATCH 2/3] avcodec/vp8: Remove write-only vp7 struct field

[Andreas Rheinhardt]

This decoder always inlines whether it is VP7 or VP8.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
It even inlines pretty cold code...

 libavcodec/vp8.c | 1 -
 libavcodec/vp8.h | 2 --
 2 files changed, 3 deletions(-)

diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
index 7972775a1c..6b49ec9fe7 100644
--- a/libavcodec/vp8.c
+++ b/libavcodec/vp8.c
@@ -2887,7 +2887,6 @@ int vp78_decode_init(AVCodecContext *avctx, int is_vp7)
     int ret;
 
     s->avctx = avctx;
-    s->vp7   = avctx->codec->id == AV_CODEC_ID_VP7;
     s->pix_fmt = AV_PIX_FMT_NONE;
     avctx->pix_fmt = AV_PIX_FMT_YUV420P;
 
diff --git a/libavcodec/vp8.h b/libavcodec/vp8.h
index eb9fa2f166..798f67b3de 100644
--- a/libavcodec/vp8.h
+++ b/libavcodec/vp8.h
@@ -331,8 +331,6 @@ typedef struct VP8Context {
     int (*decode_mb_row_no_filter)(AVCodecContext *avctx, void *tdata, int jobnr, int threadnr);
     void (*filter_mb_row)(AVCodecContext *avctx, void *tdata, int jobnr, int threadnr);
 
-    int vp7;
-
     /**
      * Interframe DC prediction (VP7)
      * [0] VP8_FRAME_PREVIOUS
-- 
2.34.1

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

[FFmpeg-devel] [PATCH 3/3] avcodec/vp8: Change criterion for calling ff_thread_finish_setup()

[Andreas Rheinhardt]

The current criterion is to check for the existence of
update_thread_context. Change this to check for whether
we are actually decoding VP8 (and not VP7 or VP8-in-WebP).
This is equivalent to the current criterion, but allows
the WebP decoder to evolve and to get its own update_thread_context.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/vp8.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
index 6b49ec9fe7..dd6c1b361b 100644
--- a/libavcodec/vp8.c
+++ b/libavcodec/vp8.c
@@ -2754,7 +2754,7 @@ int vp78_decode_frame(AVCodecContext *avctx, AVFrame *rframe, int *got_frame,
 
     s->next_framep[VP8_FRAME_CURRENT] = curframe;
 
-    if (ffcodec(avctx->codec)->update_thread_context)
+    if (!is_vp7 && !s->actually_webp)
         ff_thread_finish_setup(avctx);
 
     if (avctx->hwaccel) {
-- 
2.34.1

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

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/vp8: Enforce key-frame only for WebP

[Andreas Rheinhardt]

Andreas Rheinhardt:
> VP8-in-WebP only uses key frame encoding (see [1]), yet this
> is currently not enforced. This commit does so in order to
> make output reproducible with frame-threading as the VP8 decoder's
> update_thread_context is not called at all when using decoding
> VP8-in-WebP (as this is unnecessary for key frame-only streams).
> 
> [1]: https://developers.google.com/speed/webp/docs/riff_container
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/vp8.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
> index 83c60adeb0..7972775a1c 100644
> --- a/libavcodec/vp8.c
> +++ b/libavcodec/vp8.c
> @@ -2665,7 +2665,11 @@ int vp78_decode_frame(AVCodecContext *avctx, AVFrame *rframe, int *got_frame,
>      if (ret < 0)
>          goto err;
>  
> -    if (s->actually_webp) {
> +    if (!is_vp7 && s->actually_webp) {
> +        // VP8 in WebP is supposed to be intra-only. Enforce this here
> +        // to ensure that output is reproducible with frame-threading.
> +        if (!s->keyframe)
> +            return AVERROR_INVALIDDATA;
>          // avctx->pix_fmt already set in caller.
>      } else if (!is_vp7 && s->pix_fmt == AV_PIX_FMT_NONE) {
>          s->pix_fmt = get_pixel_format(s);

Will apply this patchset tomorrow unless there are objections.

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

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/vp8: Enforce key-frame only for WebP

[Thilo Borgmann via ffmpeg-devel]

On 03.02.24 14:57, Andreas Rheinhardt wrote:
> Andreas Rheinhardt:
>> VP8-in-WebP only uses key frame encoding (see [1]), yet this
>> is currently not enforced. This commit does so in order to
>> make output reproducible with frame-threading as the VP8 decoder's
>> update_thread_context is not called at all when using decoding
>> VP8-in-WebP (as this is unnecessary for key frame-only streams).
>>
>> [1]: https://developers.google.com/speed/webp/docs/riff_container
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>>   libavcodec/vp8.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
>> index 83c60adeb0..7972775a1c 100644
>> --- a/libavcodec/vp8.c
>> +++ b/libavcodec/vp8.c
>> @@ -2665,7 +2665,11 @@ int vp78_decode_frame(AVCodecContext *avctx, AVFrame *rframe, int *got_frame,
>>       if (ret < 0)
>>           goto err;
>>   
>> -    if (s->actually_webp) {
>> +    if (!is_vp7 && s->actually_webp) {
>> +        // VP8 in WebP is supposed to be intra-only. Enforce this here
>> +        // to ensure that output is reproducible with frame-threading.
>> +        if (!s->keyframe)
>> +            return AVERROR_INVALIDDATA;
>>           // avctx->pix_fmt already set in caller.
>>       } else if (!is_vp7 && s->pix_fmt == AV_PIX_FMT_NONE) {
>>           s->pix_fmt = get_pixel_format(s);
> 
> Will apply this patchset tomorrow unless there are objections.

Works as expected for me for a coupled version of the animated webp 
decoder which can follow-up.

Thanks,
Thilo
_______________________________________________
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".