Re: [FFmpeg-devel] [PATCH v2] avcodec/vvc: Add support for output_corrupt/showall flags

[Zhao Zhili <quinkblack-at-foxmail.com@ffmpeg.org>]

> On Jan 18, 2025, at 23:30, Nuo Mi <nuomi2021@gmail.com> wrote:
> 
> Hi Zhili,
> Thank you for v2,
> +1
> 
> Nit: The if statement is too long. Move it after generate_missing_ref and
> break it up to simplify the logic.

Put the check before generate_missing_ref is meant to avoid allocate frame
when it won’t be used.

Temporary variable can be used to make the if check easier to read, while
temporary variable don’t have the benefit of short-circuit. The actual performance
impact should be negligible, so here is v3:

https://ffmpeg.org/pipermail/ffmpeg-devel/2025-January/338536.html

> like:
> 
>     if (!ref) {
>         ref = generate_missing_ref(s, fc, poc);
>         if (!ref)
>             return AVERROR(ENOMEM);
>     }
> +    if (ref->flags & VVC_FRAME_FLAG_CORRUPT) {
> +        if (!IS_CVSS(s) && (!s->no_output_before_recovery_flag ||
> GDR_IS_RECOVERED(s))) {
> +            if (!(s->avctx->flags & AV_CODEC_FLAG_OUTPUT_CORRUPT) &&
> +                !(s->avctx->flags2 & AV_CODEC_FLAG2_SHOW_ALL)) {
> +                return AVERROR_INVALIDDATA;
> +            }
> +            fc->ref->flags |= VVC_FRAME_FLAG_CORRUPT;
> +        }
> +    }
> 
> On Wed, Jan 15, 2025 at 11:11 PM Zhao Zhili <
> quinkblack-at-foxmail.com@ffmpeg.org> wrote:
> 
>> From: Zhao Zhili <zhilizhao@tencent.com>
>> 
>> ---
>> v2: Fix GDR stream.
>> 
>> libavcodec/vvc/refs.c | 17 ++++++++++++++++-
>> libavcodec/vvc/refs.h |  1 +
>> 2 files changed, 17 insertions(+), 1 deletion(-)
>> 
>> diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
>> index bc3b3d0d13..fb19a2d394 100644
>> --- a/libavcodec/vvc/refs.c
>> +++ b/libavcodec/vvc/refs.c
>> @@ -47,6 +47,8 @@ void ff_vvc_unref_frame(VVCFrameContext *fc, VVCFrame
>> *frame, int flags)
>>         return;
>> 
>>     frame->flags &= ~flags;
>> +    if (!(frame->flags & ~VVC_FRAME_FLAG_CORRUPT))
>> +        frame->flags = 0;
>>     if (!frame->flags) {
>>         av_frame_unref(frame->frame);
>>         av_refstruct_unref(&frame->sps);
>> @@ -248,6 +250,9 @@ int ff_vvc_output_frame(VVCContext *s, VVCFrameContext
>> *fc, AVFrame *out, const
>>         if (nb_output) {
>>             VVCFrame *frame = &fc->DPB[min_idx];
>> 
>> +            if (frame->flags & VVC_FRAME_FLAG_CORRUPT)
>> +                frame->frame->flags |= AV_FRAME_FLAG_CORRUPT;
>> +
>>             ret = av_frame_ref(out, frame->frame);
>>             if (frame->flags & VVC_FRAME_FLAG_BUMPING)
>>                 ff_vvc_unref_frame(fc, frame, VVC_FRAME_FLAG_OUTPUT |
>> VVC_FRAME_FLAG_BUMPING);
>> @@ -357,7 +362,7 @@ static VVCFrame *generate_missing_ref(VVCContext *s,
>> VVCFrameContext *fc, int po
>> 
>>     frame->poc      = poc;
>>     frame->sequence = s->seq_decode;
>> -    frame->flags    = 0;
>> +    frame->flags    = VVC_FRAME_FLAG_CORRUPT;
>> 
>>     ff_vvc_report_frame_finished(frame);
>> 
>> @@ -392,6 +397,16 @@ static int add_candidate_ref(VVCContext *s,
>> VVCFrameContext *fc, RefPicList *lis
>>     if (ref == fc->ref || list->nb_refs >= VVC_MAX_REF_ENTRIES)
>>         return AVERROR_INVALIDDATA;
>> 
>> +    if (!IS_CVSS(s) && (!ref || ref->flags & VVC_FRAME_FLAG_CORRUPT) &&
>> +        (!s->no_output_before_recovery_flag || GDR_IS_RECOVERED(s))) {
>> +        if (!(s->avctx->flags & AV_CODEC_FLAG_OUTPUT_CORRUPT) &&
>> +            !(s->avctx->flags2 & AV_CODEC_FLAG2_SHOW_ALL)) {
>> +            return AVERROR_INVALIDDATA;
>> +        }
>> +
>> +        fc->ref->flags |= VVC_FRAME_FLAG_CORRUPT;
>> +    }
>> +
>>     if (!ref) {
>>         ref = generate_missing_ref(s, fc, poc);
>>         if (!ref)
>> diff --git a/libavcodec/vvc/refs.h b/libavcodec/vvc/refs.h
>> index e2271ab381..a3081a76be 100644
>> --- a/libavcodec/vvc/refs.h
>> +++ b/libavcodec/vvc/refs.h
>> @@ -29,6 +29,7 @@
>> #define VVC_FRAME_FLAG_SHORT_REF (1 << 1)
>> #define VVC_FRAME_FLAG_LONG_REF  (1 << 2)
>> #define VVC_FRAME_FLAG_BUMPING   (1 << 3)
>> +#define VVC_FRAME_FLAG_CORRUPT   (1 << 4)
>> 
>> int ff_vvc_output_frame(VVCContext *s, VVCFrameContext *fc, struct
>> AVFrame *out, int no_output_of_prior_pics_flag, int flush);
>> void ff_vvc_bump_frame(VVCContext *s, VVCFrameContext *fc);
>> --
>> 2.46.0
>> 
>> _______________________________________________
>> 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 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 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".