diff mbox series

[FFmpeg-devel,v2] avcodec/vvc: Fix output and unref a frame which isn't decoding yet

Message ID tencent_285A389D167CCD23202BCD934F895504A707@qq.com
State New
Headers show
Series [FFmpeg-devel,v2] avcodec/vvc: Fix output and unref a frame which isn't decoding yet | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 fail Make fate failed
andriy/make_x86 success Make finished
andriy/make_fate_x86 fail Make fate failed

Commit Message

Zhao Zhili Sept. 13, 2024, 6:34 a.m. UTC 
From: Zhao Zhili <zhilizhao@tencent.com>

ff_vvc_output_frame is called before actually decoding. It's possible
for ff_vvc_output_frame to select current frame to output. If current
frame is nonref frame, it will be released by ff_vvc_unref_frame.

Fix this by always marking the current frame with
VVC_FRAME_FLAG_SHORT_REF, as is done by the HEVC decoder.
---
fate sample:
https://drive.google.com/file/d/1U5WGWeSsMFiEkhsl_vL4NiMma-LLh02t/view?usp=drive_link
md5sum:
8054b4b8e62c0171476b40206d044590  Hierarchical.bit

 libavcodec/vvc/refs.c                       |  5 +--
 tests/fate/vvc.mak                          |  1 +
 tests/ref/fate/vvc-conformance-Hierarchical | 35 +++++++++++++++++++++
 3 files changed, 37 insertions(+), 4 deletions(-)
 create mode 100644 tests/ref/fate/vvc-conformance-Hierarchical

Comments

Nuo Mi Sept. 15, 2024, 12:46 a.m. UTC | #1 
Hi Zhili
Thank you for the patch.


On Fri, Sep 13, 2024 at 2:35 PM Zhao Zhili <quinkblack@foxmail.com> wrote:

> From: Zhao Zhili <zhilizhao@tencent.com>
>
> ff_vvc_output_frame is called before actually decoding. It's possible
> for ff_vvc_output_frame to select current frame to output. If current
> frame is nonref frame, it will be released by ff_vvc_unref_frame.
>
> Fix this by always marking the current frame with
> VVC_FRAME_FLAG_SHORT_REF, as is done by the HEVC decoder.
> ---
> fate sample:
>
> https://drive.google.com/file/d/1U5WGWeSsMFiEkhsl_vL4NiMma-LLh02t/view?usp=drive_link
> md5sum
> <https://drive.google.com/file/d/1U5WGWeSsMFiEkhsl_vL4NiMma-LLh02t/view?usp=drive_linkmd5sum>
> :
> 8054b4b8e62c0171476b40206d044590  Hierarchical.bit
>
>  libavcodec/vvc/refs.c                       |  5 +--
>  tests/fate/vvc.mak                          |  1 +
>  tests/ref/fate/vvc-conformance-Hierarchical | 35 +++++++++++++++++++++
>  3 files changed, 37 insertions(+), 4 deletions(-)
>  create mode 100644 tests/ref/fate/vvc-conformance-Hierarchical
>
> diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
> index bebcef7fd6..d5aa2ff28d 100644
> --- a/libavcodec/vvc/refs.c
> +++ b/libavcodec/vvc/refs.c
> @@ -193,10 +193,7 @@ int ff_vvc_set_new_ref(VVCContext *s, VVCFrameContext
> *fc, AVFrame **frame)
>      if (s->no_output_before_recovery_flag && (IS_RASL(s) ||
> !GDR_IS_RECOVERED(s)))
>          ref->flags = VVC_FRAME_FLAG_SHORT_REF;
>      else if (ph->r->ph_pic_output_flag)
> -        ref->flags = VVC_FRAME_FLAG_OUTPUT;
> -
> -    if (!ph->r->ph_non_ref_pic_flag)
> -        ref->flags |= VVC_FRAME_FLAG_SHORT_REF;
>
Could you clarify why we need to remove the above line?
Not every frame needs to be an output frame; some are used solely for
reference

Could you help split this into two separate patches?
1.  Fix the issue.
2. Add the FATE test.

Thank you

> +        ref->flags = VVC_FRAME_FLAG_OUTPUT | VVC_FRAME_FLAG_SHORT_REF;
>
>      ref->poc      = poc;
>      ref->sequence = s->seq_decode;
> diff --git a/tests/fate/vvc.mak b/tests/fate/vvc.mak
> index 5335460263..7fd0a47214 100644
> --- a/tests/fate/vvc.mak
> +++ b/tests/fate/vvc.mak
> @@ -1,5 +1,6 @@
>  VVC_SAMPLES_8BIT =        \
>      CodingToolsSets_A_2   \
> +    Hierarchical          \
>
>  VVC_SAMPLES_10BIT =       \
>      APSALF_A_2            \
> diff --git a/tests/ref/fate/vvc-conformance-Hierarchical
> b/tests/ref/fate/vvc-conformance-Hierarchical
> new file mode 100644
> index 0000000000..0797305b9a
> --- /dev/null
> +++ b/tests/ref/fate/vvc-conformance-Hierarchical
> @@ -0,0 +1,35 @@
> +#tb 0: 1/25
> +#media_type 0: video
> +#codec_id 0: rawvideo
> +#dimensions 0: 480x320
> +#sar 0: 0/1
> +0,          0,          0,        1,   230400, 0x3293f7f1
> +0,          1,          1,        1,   230400, 0xe2570fa4
> +0,          2,          2,        1,   230400, 0xecd608fb
> +0,          3,          3,        1,   230400, 0xea46f9f4
> +0,          4,          4,        1,   230400, 0xb715d24a
> +0,          5,          5,        1,   230400, 0x69faaf46
> +0,          6,          6,        1,   230400, 0xf9a362db
> +0,          7,          7,        1,   230400, 0x2dcd19ca
> +0,          8,          8,        1,   230400, 0xf8fda185
> +0,          9,          9,        1,   230400, 0x48a35bfd
> +0,         10,         10,        1,   230400, 0x27efe832
> +0,         11,         11,        1,   230400, 0x74279617
> +0,         12,         12,        1,   230400, 0x91935248
> +0,         13,         13,        1,   230400, 0x29b621e6
> +0,         14,         14,        1,   230400, 0x89b1ec0b
> +0,         15,         15,        1,   230400, 0x898fdba1
> +0,         16,         16,        1,   230400, 0xc6d18e6f
> +0,         17,         17,        1,   230400, 0xedff651b
> +0,         18,         18,        1,   230400, 0x677e2260
> +0,         19,         19,        1,   230400, 0x930918ef
> +0,         20,         20,        1,   230400, 0x70da2c30
> +0,         21,         21,        1,   230400, 0x699a3b9d
> +0,         22,         22,        1,   230400, 0xff3b1b3a
> +0,         23,         23,        1,   230400, 0xca11d9a5
> +0,         24,         24,        1,   230400, 0x904394e0
> +0,         25,         25,        1,   230400, 0x392e5445
> +0,         26,         26,        1,   230400, 0x6191f4d8
> +0,         27,         27,        1,   230400, 0xa7d7be12
> +0,         28,         28,        1,   230400, 0xbb29752c
> +0,         29,         29,        1,   230400, 0x14ff297e
> --
> 2.42.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".
>
Zhao Zhili Sept. 15, 2024, 4:56 a.m. UTC | #2 
> On Sep 15, 2024, at 08:46, Nuo Mi <nuomi2021@gmail.com> wrote:
> 
> Hi Zhili
> Thank you for the patch.
> 
> 
> On Fri, Sep 13, 2024 at 2:35 PM Zhao Zhili <quinkblack@foxmail.com <mailto:quinkblack@foxmail.com>> wrote:
> 
>> From: Zhao Zhili <zhilizhao@tencent.com>
>> 
>> ff_vvc_output_frame is called before actually decoding. It's possible
>> for ff_vvc_output_frame to select current frame to output. If current
>> frame is nonref frame, it will be released by ff_vvc_unref_frame.
>> 
>> Fix this by always marking the current frame with
>> VVC_FRAME_FLAG_SHORT_REF, as is done by the HEVC decoder.
>> ---
>> fate sample:
>> 
>> https://drive.google.com/file/d/1U5WGWeSsMFiEkhsl_vL4NiMma-LLh02t/view?usp=drive_link
>> md5sum
>> <https://drive.google.com/file/d/1U5WGWeSsMFiEkhsl_vL4NiMma-LLh02t/view?usp=drive_linkmd5sum>
>> :
>> 8054b4b8e62c0171476b40206d044590  Hierarchical.bit
>> 
>> libavcodec/vvc/refs.c                       |  5 +--
>> tests/fate/vvc.mak                          |  1 +
>> tests/ref/fate/vvc-conformance-Hierarchical | 35 +++++++++++++++++++++
>> 3 files changed, 37 insertions(+), 4 deletions(-)
>> create mode 100644 tests/ref/fate/vvc-conformance-Hierarchical
>> 
>> diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
>> index bebcef7fd6..d5aa2ff28d 100644
>> --- a/libavcodec/vvc/refs.c
>> +++ b/libavcodec/vvc/refs.c
>> @@ -193,10 +193,7 @@ int ff_vvc_set_new_ref(VVCContext *s, VVCFrameContext
>> *fc, AVFrame **frame)
>>     if (s->no_output_before_recovery_flag && (IS_RASL(s) ||
>> !GDR_IS_RECOVERED(s)))
>>         ref->flags = VVC_FRAME_FLAG_SHORT_REF;
>>     else if (ph->r->ph_pic_output_flag)
>> -        ref->flags = VVC_FRAME_FLAG_OUTPUT;
>> -
>> -    if (!ph->r->ph_non_ref_pic_flag)
>> -        ref->flags |= VVC_FRAME_FLAG_SHORT_REF;
>> 
> Could you clarify why we need to remove the above line?
> Not every frame needs to be an output frame; some are used solely for
> reference

It should be kept. Here is v3.

https://ffmpeg.org/pipermail/ffmpeg-devel/2024-September/333534.html

> 
> Could you help split this into two separate patches?
> 1.  Fix the issue.
> 2. Add the FATE test.
> 
> Thank you
> 
>> +        ref->flags = VVC_FRAME_FLAG_OUTPUT | VVC_FRAME_FLAG_SHORT_REF;
>> 
>>     ref->poc      = poc;
>>     ref->sequence = s->seq_decode;
>> diff --git a/tests/fate/vvc.mak b/tests/fate/vvc.mak
>> index 5335460263..7fd0a47214 100644
>> --- a/tests/fate/vvc.mak
>> +++ b/tests/fate/vvc.mak
>> @@ -1,5 +1,6 @@
>> VVC_SAMPLES_8BIT =        \
>>     CodingToolsSets_A_2   \
>> +    Hierarchical          \
>> 
>> VVC_SAMPLES_10BIT =       \
>>     APSALF_A_2            \
>> diff --git a/tests/ref/fate/vvc-conformance-Hierarchical
>> b/tests/ref/fate/vvc-conformance-Hierarchical
>> new file mode 100644
>> index 0000000000..0797305b9a
>> --- /dev/null
>> +++ b/tests/ref/fate/vvc-conformance-Hierarchical
>> @@ -0,0 +1,35 @@
>> +#tb 0: 1/25
>> +#media_type 0: video
>> +#codec_id 0: rawvideo
>> +#dimensions 0: 480x320
>> +#sar 0: 0/1
>> +0,          0,          0,        1,   230400, 0x3293f7f1
>> +0,          1,          1,        1,   230400, 0xe2570fa4
>> +0,          2,          2,        1,   230400, 0xecd608fb
>> +0,          3,          3,        1,   230400, 0xea46f9f4
>> +0,          4,          4,        1,   230400, 0xb715d24a
>> +0,          5,          5,        1,   230400, 0x69faaf46
>> +0,          6,          6,        1,   230400, 0xf9a362db
>> +0,          7,          7,        1,   230400, 0x2dcd19ca
>> +0,          8,          8,        1,   230400, 0xf8fda185
>> +0,          9,          9,        1,   230400, 0x48a35bfd
>> +0,         10,         10,        1,   230400, 0x27efe832
>> +0,         11,         11,        1,   230400, 0x74279617
>> +0,         12,         12,        1,   230400, 0x91935248
>> +0,         13,         13,        1,   230400, 0x29b621e6
>> +0,         14,         14,        1,   230400, 0x89b1ec0b
>> +0,         15,         15,        1,   230400, 0x898fdba1
>> +0,         16,         16,        1,   230400, 0xc6d18e6f
>> +0,         17,         17,        1,   230400, 0xedff651b
>> +0,         18,         18,        1,   230400, 0x677e2260
>> +0,         19,         19,        1,   230400, 0x930918ef
>> +0,         20,         20,        1,   230400, 0x70da2c30
>> +0,         21,         21,        1,   230400, 0x699a3b9d
>> +0,         22,         22,        1,   230400, 0xff3b1b3a
>> +0,         23,         23,        1,   230400, 0xca11d9a5
>> +0,         24,         24,        1,   230400, 0x904394e0
>> +0,         25,         25,        1,   230400, 0x392e5445
>> +0,         26,         26,        1,   230400, 0x6191f4d8
>> +0,         27,         27,        1,   230400, 0xa7d7be12
>> +0,         28,         28,        1,   230400, 0xbb29752c
>> +0,         29,         29,        1,   230400, 0x14ff297e
>> --
>> 2.42.0
>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
>> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
index bebcef7fd6..d5aa2ff28d 100644
--- a/libavcodec/vvc/refs.c
+++ b/libavcodec/vvc/refs.c
@@ -193,10 +193,7 @@  int ff_vvc_set_new_ref(VVCContext *s, VVCFrameContext *fc, AVFrame **frame)
     if (s->no_output_before_recovery_flag && (IS_RASL(s) || !GDR_IS_RECOVERED(s)))
         ref->flags = VVC_FRAME_FLAG_SHORT_REF;
     else if (ph->r->ph_pic_output_flag)
-        ref->flags = VVC_FRAME_FLAG_OUTPUT;
-
-    if (!ph->r->ph_non_ref_pic_flag)
-        ref->flags |= VVC_FRAME_FLAG_SHORT_REF;
+        ref->flags = VVC_FRAME_FLAG_OUTPUT | VVC_FRAME_FLAG_SHORT_REF;
 
     ref->poc      = poc;
     ref->sequence = s->seq_decode;
diff --git a/tests/fate/vvc.mak b/tests/fate/vvc.mak
index 5335460263..7fd0a47214 100644
--- a/tests/fate/vvc.mak
+++ b/tests/fate/vvc.mak
@@ -1,5 +1,6 @@ 
 VVC_SAMPLES_8BIT =        \
     CodingToolsSets_A_2   \
+    Hierarchical          \
 
 VVC_SAMPLES_10BIT =       \
     APSALF_A_2            \
diff --git a/tests/ref/fate/vvc-conformance-Hierarchical b/tests/ref/fate/vvc-conformance-Hierarchical
new file mode 100644
index 0000000000..0797305b9a
--- /dev/null
+++ b/tests/ref/fate/vvc-conformance-Hierarchical
@@ -0,0 +1,35 @@ 
+#tb 0: 1/25
+#media_type 0: video
+#codec_id 0: rawvideo
+#dimensions 0: 480x320
+#sar 0: 0/1
+0,          0,          0,        1,   230400, 0x3293f7f1
+0,          1,          1,        1,   230400, 0xe2570fa4
+0,          2,          2,        1,   230400, 0xecd608fb
+0,          3,          3,        1,   230400, 0xea46f9f4
+0,          4,          4,        1,   230400, 0xb715d24a
+0,          5,          5,        1,   230400, 0x69faaf46
+0,          6,          6,        1,   230400, 0xf9a362db
+0,          7,          7,        1,   230400, 0x2dcd19ca
+0,          8,          8,        1,   230400, 0xf8fda185
+0,          9,          9,        1,   230400, 0x48a35bfd
+0,         10,         10,        1,   230400, 0x27efe832
+0,         11,         11,        1,   230400, 0x74279617
+0,         12,         12,        1,   230400, 0x91935248
+0,         13,         13,        1,   230400, 0x29b621e6
+0,         14,         14,        1,   230400, 0x89b1ec0b
+0,         15,         15,        1,   230400, 0x898fdba1
+0,         16,         16,        1,   230400, 0xc6d18e6f
+0,         17,         17,        1,   230400, 0xedff651b
+0,         18,         18,        1,   230400, 0x677e2260
+0,         19,         19,        1,   230400, 0x930918ef
+0,         20,         20,        1,   230400, 0x70da2c30
+0,         21,         21,        1,   230400, 0x699a3b9d
+0,         22,         22,        1,   230400, 0xff3b1b3a
+0,         23,         23,        1,   230400, 0xca11d9a5
+0,         24,         24,        1,   230400, 0x904394e0
+0,         25,         25,        1,   230400, 0x392e5445
+0,         26,         26,        1,   230400, 0x6191f4d8
+0,         27,         27,        1,   230400, 0xa7d7be12
+0,         28,         28,        1,   230400, 0xbb29752c
+0,         29,         29,        1,   230400, 0x14ff297e