Re: [FFmpeg-devel] [PATCH 5/9] avcodec/cbs_h266_syntax_template: Check bit depth with range extension
On 20/09/2024 01:54, James Almer wrote:
> On 9/19/2024 9:34 PM, Michael Niedermayer wrote:
>> On Thu, Sep 19, 2024 at 08:53:07PM -0300, James Almer wrote:
>>> On 9/19/2024 7:56 PM, Michael Niedermayer wrote:
Fixes: shift exponent 62 is too large for 32-bit type 'int'
Fixes:
71020/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VVC_fuzzer-6444916325023744
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavcodec/cbs_h266_syntax_template.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libavcodec/cbs_h266_syntax_template.c
b/libavcodec/cbs_h266_syntax_template.c
index a8f5af04d02..1c26563 100644
--- a/libavcodec/cbs_h266_syntax_template.c
+++ b/libavcodec/cbs_h266_syntax_template.c
@@ -1041,6 +1041,9 @@ static int
FUNC(sps_range_extension)(CodedBitstreamContext *ctx, RWContext *rw,
{
int err;
+ if (current->sps_bitdepth_minus8 < 10)
>>>
>>> sps_bitdepth_minus8 can only be between 0 and 8, so this is basically
>>> making
>>> it abort on any and every sample with SPS range extension.
>>
>> + if (current->sps_bitdepth_minus8 < 10 - 8)
>
> Ok, this is different.
>
>>
>> Its supposed to check this:
>> "When BitDepth is less
>> than or equal to 10, the value of sps_range_extension_flag shall be
>> equal to 0."
>
> Should be "<= (10 - 8)" then, and LGTM.
>
LGTM, although nit: I think intent would be clearer and the code would
correspond better with the standard if the check was moved to the parent
function next to the flag itself.
--
Frank
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] [PATCH] avcodec/cbs_h266: Fix copy paste mistake
Hi Marvin,
Thanks for this patch and sorry for not getting around to it sooner.
Patch LGTM, yes it is a mistake I made when copy-pasting the logic for
the width -- sorry about that!
Thanks again,
Frank
On 31/08/2024 22:25, Marvin Scholz wrote:
> The us macro expect the range_max here, which seems should be
> MAX_UINT_BITS(hlen) here.
>
> Fix CID1618757 Copy-paste error
> ---
>
> This code is non-trivial to understand so I might be wrong
> about this, it would be great if someone actually familiar
> with this can have a look if my assesment is correct here.
>
> Also if it isn't, a comment here might help to clarify things.
>
> libavcodec/cbs_h266_syntax_template.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/cbs_h266_syntax_template.c
> b/libavcodec/cbs_h266_syntax_template.c
> index a8f5af04d0..0704da1d40 100644
> --- a/libavcodec/cbs_h266_syntax_template.c
> +++ b/libavcodec/cbs_h266_syntax_template.c
> @@ -1208,7 +1208,7 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
> RWContext *rw,
> win_top_edge_ctus >
> current->sps_subpic_ctu_top_left_y[i]
> ? win_top_edge_ctus -
> current->sps_subpic_ctu_top_left_y[i]
> : 0,
> - MAX_UINT_BITS(wlen), 1, i);
> + MAX_UINT_BITS(hlen), 1, i);
> } else {
> infer(sps_subpic_height_minus1[i],
>tmp_height_val -
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4] avfilter: add pu21 filter
Hi,
Thanks for your patch.
On 23/08/2024 20:15, Rajiv Harlalka wrote:
> Adds the PU21 encoding filter for high dynamic range images and video
> quality assessment
> Mantiuk, R., & Azimi, M. PU21: A novel perceptually uniform encoding
> for adapting existing quality metrics for HDR
> https://github.com/gfxdisp/pu21
>
> Signed-off-by: Rajiv Harlalka
> ---
> Changelog| 1 +
> libavfilter/Makefile | 1 +
> libavfilter/allfilters.c | 1 +
> libavfilter/version.h| 4 +-
> libavfilter/vf_pu21.c| 175 +++
> 5 files changed, 180 insertions(+), 2 deletions(-)
> create mode 100644 libavfilter/vf_pu21.c
>
> diff --git a/Changelog b/Changelog
> index 571b27ad98..552aa6028a 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -18,6 +18,7 @@ version :
> - D3D12VA HEVC encoder
> - Cropping metadata parsing and writing in Matroska and MP4/MOV de/muxers
> - Intel QSV-accelerated VVC decoding
> +- Addition of vf_pu21 encoding filter
>
>
> version 7.0:
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 63088e9286..102315f5bf 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -121,6 +121,7 @@ OBJS-$(CONFIG_AXCORRELATE_FILTER)+=
> af_axcorrelate.o
> OBJS-$(CONFIG_AZMQ_FILTER) += f_zmq.o
> OBJS-$(CONFIG_BANDPASS_FILTER) += af_biquads.o
> OBJS-$(CONFIG_BANDREJECT_FILTER) += af_biquads.o
> +OBJS-$(CONFIG_PU21_FILTER) += vf_pu21.o
This line should be inserted such that the list is in alphabetical order.
> OBJS-$(CONFIG_BASS_FILTER) += af_biquads.o
> OBJS-$(CONFIG_BIQUAD_FILTER) += af_biquads.o
> OBJS-$(CONFIG_BS2B_FILTER) += af_bs2b.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index 63600e9b58..ced076626c 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -231,6 +231,7 @@ extern const AVFilter ff_vf_convolution_opencl;
> extern const AVFilter ff_vf_convolve;
> extern const AVFilter ff_vf_copy;
> extern const AVFilter ff_vf_coreimage;
> +extern const AVFilter ff_vf_pu21;
This line should be inserted such that the list is in alphabetical order.
> extern const AVFilter ff_vf_corr;
> extern const AVFilter ff_vf_cover_rect;
> extern const AVFilter ff_vf_crop;
> diff --git a/libavfilter/version.h b/libavfilter/version.h
> index d8cd8a2cfb..7e0eb9af97 100644
> --- a/libavfilter/version.h
> +++ b/libavfilter/version.h
> @@ -31,8 +31,8 @@
>
> #include "version_major.h"
>
> -#define LIBAVFILTER_VERSION_MINOR 2
> -#define LIBAVFILTER_VERSION_MICRO 102
> +#define LIBAVFILTER_VERSION_MINOR 3
> +#define LIBAVFILTER_VERSION_MICRO 100
>
>
> #define LIBAVFILTER_VERSION_INT AV_VERSION_INT(LIBAVFILTER_VERSION_MAJOR, \
> diff --git a/libavfilter/vf_pu21.c b/libavfilter/vf_pu21.c
> new file mode 100644
> index 00..64804ce60b
> --- /dev/null
> +++ b/libavfilter/vf_pu21.c
> @@ -0,0 +1,175 @@
> +/*
> + * 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-1301
> USA
> + */
> +
> +#include "libavfilter/avfilter.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/pixdesc.h"
> +#include "libavutil/mem.h"
> +#include "libavutil/imgutils.h"
> +#include "libavfilter/formats.h"
> +#include "libavutil/internal.h"
> +#include "video.h"
> +#include "libavutil/libm.h"
I think some of these includes are unused.
> +
> +enum {
> +BANDING,
> +BANDING_GLARE,
> +PEAKS,
> +PEAKS_GLARE
> +};
> +
> +typedef struct PU21Context {
> +const AVClass* class;
> +double L_min, L_max;
L_min and L_max are unused
> +int multiplier;
> +int mode;
> +
> +int depth;
> +int nb_planes;
> +} PU21Context;
> +
> +const float pu21_params[4][7] = {
> +// Reference: "PU21: A novel perceptually uniform encoding for adapting
> existing quality metrics for HDR"
> +// Rafał K. Mantiuk and M. Azimi, Picture Coding Symposium 2021
> +// https://github.com/gfxdisp/pu21
> +{1.070275272f, 0.4088273932f, 0.153224308f, 0.2520326168f, 1.063512885f,
> 1.14115047f, 521.4527484f}, // BANDING
> +{0.353487901f, 0.3734658629f, 8.277049286e-05f, 0.9062562627f,
> 0.09150303166f, 0.9099517204f, 596.
Re: [FFmpeg-devel] [PATCH v2] lavc/vvc: Remove assertions on qPy_{a, b}
On 28/08/2024 16:13, Anton Khirnov wrote:
> Quoting Frank Plowman (2024-08-25 13:50:41)
>> These assertions are not violated, even by illegal bitstreams.
>
> I don't follow this argument, not being violated by any reachable
> runtime path is exactly how a correctly written assert should behave.
>
The point I was trying to make is that I have not seen the assertion
violated (besides the false positives qPy_{a,b} = 63) in my fuzz
testing. Along with the fact the derivation procedure for qPy_{a,b} is
not all that complicated and largely contained in this one function, one
might consider the assertions unnecessary.
That being said, I would prefer not to remove the assertions and instead
to apply the first version of this patch. It seems recent discussion on
the thread of v1 is leaning back that way as well.
--
Frank
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2] lavc/vvc: Validate explicit subpic locations
Implement the missing requirements from H.266 (V3) p. 106 on the
position and size of subpictures whose dimensions are provided
explicitly.
Signed-off-by: Frank Plowman
---
Changes since v1 (20240824092827.68912-1-p...@frankplowman.com):
* Use temporary variables and AV_CEIL_RSHIFT to make bound calculations
more readable.
* Fix bounds on size. The calculated values are minimums, not
maximums as in v1. Additionally, fix an integer overflow in the bound
calculations.
libavcodec/cbs_h266_syntax_template.c | 49 +--
1 file changed, 38 insertions(+), 11 deletions(-)
diff --git a/libavcodec/cbs_h266_syntax_template.c
b/libavcodec/cbs_h266_syntax_template.c
index 9c37996947..a8f5af04d0 100644
--- a/libavcodec/cbs_h266_syntax_template.c
+++ b/libavcodec/cbs_h266_syntax_template.c
@@ -1061,7 +1061,7 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
RWContext *rw,
unsigned int ctb_log2_size_y, min_cb_log2_size_y,
min_qt_log2_size_intra_y, min_qt_log2_size_inter_y,
ctb_size_y, max_num_merge_cand, tmp_width_val, tmp_height_val;
-uint8_t qp_bd_offset;
+uint8_t qp_bd_offset, sub_width_c, sub_height_c;
static const uint8_t h266_sub_width_c[] = {
1, 2, 2, 1
@@ -1089,6 +1089,9 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
RWContext *rw,
u(3, sps_max_sublayers_minus1, 0, VVC_MAX_SUBLAYERS - 1);
u(2, sps_chroma_format_idc, 0, 3);
+sub_width_c = h266_sub_width_c[current->sps_chroma_format_idc];
+sub_height_c = h266_sub_height_c[current->sps_chroma_format_idc];
+
u(2, sps_log2_ctu_size_minus5, 0, 3);
ctb_log2_size_y = current->sps_log2_ctu_size_minus5 + 5;
ctb_size_y = 1 << ctb_log2_size_y;
@@ -1110,8 +1113,6 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
RWContext *rw,
flag(sps_conformance_window_flag);
if (current->sps_conformance_window_flag) {
-uint8_t sub_width_c = h266_sub_width_c[current->sps_chroma_format_idc];
-uint8_t sub_height_c =
h266_sub_height_c[current->sps_chroma_format_idc];
uint16_t width = current->sps_pic_width_max_in_luma_samples /
sub_width_c;
uint16_t height = current->sps_pic_height_max_in_luma_samples /
sub_height_c;
ue(sps_conf_win_left_offset, 0, width);
@@ -1160,19 +1161,37 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
RWContext *rw,
}
for (i = 1; i <= current->sps_num_subpics_minus1; i++) {
if (!current->sps_subpic_same_size_flag) {
-if (current->sps_pic_width_max_in_luma_samples >
ctb_size_y)
-ubs(wlen, sps_subpic_ctu_top_left_x[i], 1, i);
-else
+if (current->sps_pic_width_max_in_luma_samples >
ctb_size_y) {
+const unsigned int win_right_edge =
+current->sps_pic_width_max_in_luma_samples
+ - current->sps_conf_win_right_offset * sub_width_c;
+us(wlen, sps_subpic_ctu_top_left_x[i], 0,
+ AV_CEIL_RSHIFT(win_right_edge, ctb_log2_size_y) - 1,
+ 1, i);
+} else
infer(sps_subpic_ctu_top_left_x[i], 0);
if (current->sps_pic_height_max_in_luma_samples >
-ctb_size_y)
-ubs(hlen, sps_subpic_ctu_top_left_y[i], 1, i);
-else
+ctb_size_y) {
+const unsigned int win_bottom_edge =
+current->sps_pic_height_max_in_luma_samples
+ - current->sps_conf_win_bottom_offset * sub_height_c;
+us(hlen, sps_subpic_ctu_top_left_y[i], 0,
+ AV_CEIL_RSHIFT(win_bottom_edge, ctb_log2_size_y) -
1,
+ 1, i);
+} else
infer(sps_subpic_ctu_top_left_y[i], 0);
if (i < current->sps_num_subpics_minus1 &&
current->sps_pic_width_max_in_luma_samples >
ctb_size_y) {
-ubs(wlen, sps_subpic_width_minus1[i], 1, i);
+const unsigned int win_left_edge =
+current->sps_conf_win_left_offset * sub_width_c;
+const unsigned int win_left_edge_ctus =
+AV_CEIL_RSHIFT(win_left_edge, ctb_log2_size_y);
+us(wlen, sps_subpic_width_minus1[i],
+ win_left_edge_ctus >
current->sps_subpic_ctu_top_left_x[i]
+ ? win_left_edge_ctus -
current->sps_subpic_ctu_top_left_x[i]
+ : 0,
+ MAX_UINT_BITS
Re: [FFmpeg-devel] [PATCH] lavc/vvc: Fix assertion bound on qPy_{a, b}
On 25/08/2024 09:11, Nuo Mi wrote: > On Sat, Aug 24, 2024 at 5:28 PM Frank Plowman wrote: > >> Signed-off-by: Frank Plowman >> --- >> libavcodec/vvc/ctu.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/libavcodec/vvc/ctu.c b/libavcodec/vvc/ctu.c >> index 139d3dded5..9ac3684ec9 100644 >> --- a/libavcodec/vvc/ctu.c >> +++ b/libavcodec/vvc/ctu.c >> @@ -115,8 +115,8 @@ static int get_qp_y_pred(const VVCLocalContext *lc) >> else >> qPy_a = fc->tab.qp[LUMA][(x_cb - 1) + y_cb * min_cb_width]; >> >> -av_assert2(qPy_a >= -fc->ps.sps->qp_bd_offset && qPy_a < 63); >> -av_assert2(qPy_b >= -fc->ps.sps->qp_bd_offset && qPy_b < 63); >> +av_assert2(qPy_a >= -fc->ps.sps->qp_bd_offset && qPy_a <= 63); >> +av_assert2(qPy_b >= -fc->ps.sps->qp_bd_offset && qPy_b <= 63); >> > Hi Frank, > Thank you for the patch. > Perhaps we can consider removing the assert, as other processes guarantee > the range, correct?" Yeah it seems that way. v2 sent. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2] lavc/vvc: Remove assertions on qPy_{a, b}
These assertions are not violated, even by illegal bitstreams. Additionally, the upper bound should be <= 63, rather than < 63. Signed-off-by: Frank Plowman --- Changes since v1 (https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240824092842.69365-1-p...@frankplowman.com/): * Remove the assertions entirely, rather than change the upper bound libavcodec/vvc/ctu.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/libavcodec/vvc/ctu.c b/libavcodec/vvc/ctu.c index 139d3dded5..70b0f05118 100644 --- a/libavcodec/vvc/ctu.c +++ b/libavcodec/vvc/ctu.c @@ -115,9 +115,6 @@ static int get_qp_y_pred(const VVCLocalContext *lc) else qPy_a = fc->tab.qp[LUMA][(x_cb - 1) + y_cb * min_cb_width]; -av_assert2(qPy_a >= -fc->ps.sps->qp_bd_offset && qPy_a < 63); -av_assert2(qPy_b >= -fc->ps.sps->qp_bd_offset && qPy_b < 63); - return (qPy_a + qPy_b + 1) >> 1; } -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Fix assertion bound on qPy_{a,b}
Signed-off-by: Frank Plowman --- libavcodec/vvc/ctu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libavcodec/vvc/ctu.c b/libavcodec/vvc/ctu.c index 139d3dded5..9ac3684ec9 100644 --- a/libavcodec/vvc/ctu.c +++ b/libavcodec/vvc/ctu.c @@ -115,8 +115,8 @@ static int get_qp_y_pred(const VVCLocalContext *lc) else qPy_a = fc->tab.qp[LUMA][(x_cb - 1) + y_cb * min_cb_width]; -av_assert2(qPy_a >= -fc->ps.sps->qp_bd_offset && qPy_a < 63); -av_assert2(qPy_b >= -fc->ps.sps->qp_bd_offset && qPy_b < 63); +av_assert2(qPy_a >= -fc->ps.sps->qp_bd_offset && qPy_a <= 63); +av_assert2(qPy_b >= -fc->ps.sps->qp_bd_offset && qPy_b <= 63); return (qPy_a + qPy_b + 1) >> 1; } -- 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Validate explicit subpic locations
Implement the missing requirements from H.266 (V3) p. 106 on the width
and height of subpictures whose dimensions are provided explicitly.
Signed-off-by: Frank Plowman
---
libavcodec/cbs_h266_syntax_template.c | 35 +--
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/libavcodec/cbs_h266_syntax_template.c
b/libavcodec/cbs_h266_syntax_template.c
index 9c37996947..6e5ff55e20 100644
--- a/libavcodec/cbs_h266_syntax_template.c
+++ b/libavcodec/cbs_h266_syntax_template.c
@@ -1061,7 +1061,7 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
RWContext *rw,
unsigned int ctb_log2_size_y, min_cb_log2_size_y,
min_qt_log2_size_intra_y, min_qt_log2_size_inter_y,
ctb_size_y, max_num_merge_cand, tmp_width_val, tmp_height_val;
-uint8_t qp_bd_offset;
+uint8_t qp_bd_offset, sub_width_c, sub_height_c;
static const uint8_t h266_sub_width_c[] = {
1, 2, 2, 1
@@ -1089,6 +1089,9 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
RWContext *rw,
u(3, sps_max_sublayers_minus1, 0, VVC_MAX_SUBLAYERS - 1);
u(2, sps_chroma_format_idc, 0, 3);
+sub_width_c = h266_sub_width_c[current->sps_chroma_format_idc];
+sub_height_c = h266_sub_height_c[current->sps_chroma_format_idc];
+
u(2, sps_log2_ctu_size_minus5, 0, 3);
ctb_log2_size_y = current->sps_log2_ctu_size_minus5 + 5;
ctb_size_y = 1 << ctb_log2_size_y;
@@ -1110,8 +1113,6 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
RWContext *rw,
flag(sps_conformance_window_flag);
if (current->sps_conformance_window_flag) {
-uint8_t sub_width_c = h266_sub_width_c[current->sps_chroma_format_idc];
-uint8_t sub_height_c =
h266_sub_height_c[current->sps_chroma_format_idc];
uint16_t width = current->sps_pic_width_max_in_luma_samples /
sub_width_c;
uint16_t height = current->sps_pic_height_max_in_luma_samples /
sub_height_c;
ue(sps_conf_win_left_offset, 0, width);
@@ -1161,18 +1162,33 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
RWContext *rw,
for (i = 1; i <= current->sps_num_subpics_minus1; i++) {
if (!current->sps_subpic_same_size_flag) {
if (current->sps_pic_width_max_in_luma_samples >
ctb_size_y)
-ubs(wlen, sps_subpic_ctu_top_left_x[i], 1, i);
+us(wlen, sps_subpic_ctu_top_left_x[i], 0,
+ (current->sps_pic_width_max_in_luma_samples
+ - current->sps_conf_win_right_offset *
sub_width_c
+ + ctb_size_y - 1)
+ / ctb_size_y - 1,
+ 1, i);
else
infer(sps_subpic_ctu_top_left_x[i], 0);
if (current->sps_pic_height_max_in_luma_samples >
ctb_size_y)
-ubs(hlen, sps_subpic_ctu_top_left_y[i], 1, i);
+us(hlen, sps_subpic_ctu_top_left_y[i], 0,
+ (current->sps_pic_height_max_in_luma_samples
+ - current->sps_conf_win_bottom_offset *
sub_height_c
+ + ctb_size_y - 1)
+ / ctb_size_y - 1,
+ 1, i);
else
infer(sps_subpic_ctu_top_left_y[i], 0);
if (i < current->sps_num_subpics_minus1 &&
current->sps_pic_width_max_in_luma_samples >
ctb_size_y) {
-ubs(wlen, sps_subpic_width_minus1[i], 1, i);
+us(wlen, sps_subpic_width_minus1[i], 0,
+ (current->sps_conf_win_left_offset * sub_width_c
+ + ctb_size_y - 1)
+ / ctb_size_y
+ - current->sps_subpic_ctu_top_left_x[i] - 2,
+ 1, i);
} else {
infer(sps_subpic_width_minus1[i],
tmp_width_val -
@@ -1181,7 +1197,12 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
RWContext *rw,
if (i < current->sps_num_subpics_minus1 &&
current->sps_pic_height_max_in_luma_samples >
ctb_size_y) {
-ubs(hlen, sps_subpic_height_minus1[i], 1, i);
+us(hlen, sps_subpic_height_minus1[i], 0,
+ (current->sps_conf_win_top_offset * sub_height_c
+ + ctb_size_y - 1)
+ / ctb_size_y
+ - current->sps_subpic_ctu_top_left_y[i] - 2,
+ 1, i);
} else {
Re: [FFmpeg-devel] [PATCH] lavc/vvc: Prevent OOB access in subpic_tiles
Hi, Thanks for your review. On 24/08/2024 04:40, Nuo Mi wrote: > Hi Frank, > thank you for the patch > On Fri, Aug 23, 2024 at 7:45 PM Frank Plowman wrote: > >> The previous logic relied on the subpicture boundaries coinciding with >> the tile boundaries. Per 6.3.1 of H.266 (V3), vertical subpicture >> boundaries are always tile boundaries however the same cannot be said >> for horizontal subpicture boundaries. > > From the spec: > "One or both of the following conditions shall be fulfilled for each > subpicture and tile: > > – All CTUs in a subpicture belong to the same tile. > > – All CTUs in a tile belong to the same subpicture." > > This suggests that the subpicture boundary coincides with a tile boundary, > right? Not always. As an example, consider a picture with only a single tile but split into two subpictures, one atop the other: Tiles Subpics Slices +---++---++---+ | || || | | |+---++---+ | || || | +---++---++---+ The first of the conditions is met for both subpics: all CTUs of either subpic belong to the same tile. It is clear to see, however, that the bottom boundary of the top subpicture and the top boundary of the bottom subpicture do not coincide with a tile boundary. In conjunction with the restrictions on subpic and slice layout and those on slice and tile layout, you get a guarantee that the vertical boundaries of subpics coincide with vertical boundaries of tiles, but there is no guaranteed relationship between the horizontal boundaries of subpics and tile boundaries. >> Furthermore, it is possible to >> construct an illegal bitstream where vertical subpicture boundaries are >> not coincident with tile boundaries. In these cases, the condition of >> the while loop would never be satisfied resulting in an OOB read on >> col_bd/row_bd. >> > Can we implement early checks to reject invalid streams? > If the picture boundaries are incorrect, it indicates a serious error in > the bitstream. > My first attempt at solving this problem took this approach. I found that it was not trivial to write an algorithm which ensures all the relationships set out in 6.3.1. As far as I can tell, the restrictions are only ever set out in prose in that section and never as more formal requirements on bitstream conformance. Additionally, these checks appear to be missing from VTM, so there is no reference implementation to refer to (aside: VTM is quite happy to encode and decode bitstreams with invalid partitioning layouts). As a result, I decided to take the approach of identifying where the relationships were used and making changes there instead. Even if the partitioning structure were validated elsewhere however, the change to the horizontal boundaries would still be needed as set out above. I also think the change to the vertical boundaries does no harm and serves to prevent an OOB access in the case that validation fails, however unlikely that may be. -- All the best, Frank ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] avfilter: add pu21 filter
Thank you for your patch. Some comments inline.
On 23/08/2024 10:07, Rajiv Harlalka wrote:
> Adds the PU21 encoding filter for high dynamic range images and video
> quality assessment
> Mantiuk, R., & Azimi, M. PU21: A novel perceptually uniform encoding for
> adapting existing quality metrics for HDR
> https://github.com/gfxdisp/pu21
> ---
> Changelog| 1 +
> libavfilter/Makefile | 1 +
> libavfilter/allfilters.c | 1 +
> libavfilter/version.h| 4 +-
> libavfilter/vf_pu21.c| 210 +++
> 5 files changed, 215 insertions(+), 2 deletions(-)
> create mode 100644 libavfilter/vf_pu21.c
>
> diff --git a/Changelog b/Changelog
> index 571b27ad98..552aa6028a 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -18,6 +18,7 @@ version :
> - D3D12VA HEVC encoder
> - Cropping metadata parsing and writing in Matroska and MP4/MOV de/muxers
> - Intel QSV-accelerated VVC decoding
> +- Addition of vf_pu21 encoding filter
>
>
> version 7.0:
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 63088e9286..102315f5bf 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -121,6 +121,7 @@ OBJS-$(CONFIG_AXCORRELATE_FILTER)+=
> af_axcorrelate.o
> OBJS-$(CONFIG_AZMQ_FILTER) += f_zmq.o
> OBJS-$(CONFIG_BANDPASS_FILTER) += af_biquads.o
> OBJS-$(CONFIG_BANDREJECT_FILTER) += af_biquads.o
> +OBJS-$(CONFIG_PU21_FILTER) += vf_pu21.o
> OBJS-$(CONFIG_BASS_FILTER) += af_biquads.o
> OBJS-$(CONFIG_BIQUAD_FILTER) += af_biquads.o
> OBJS-$(CONFIG_BS2B_FILTER) += af_bs2b.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index 63600e9b58..ced076626c 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -231,6 +231,7 @@ extern const AVFilter ff_vf_convolution_opencl;
> extern const AVFilter ff_vf_convolve;
> extern const AVFilter ff_vf_copy;
> extern const AVFilter ff_vf_coreimage;
> +extern const AVFilter ff_vf_pu21;
> extern const AVFilter ff_vf_corr;
> extern const AVFilter ff_vf_cover_rect;
> extern const AVFilter ff_vf_crop;
> diff --git a/libavfilter/version.h b/libavfilter/version.h
> index d8cd8a2cfb..7e0eb9af97 100644
> --- a/libavfilter/version.h
> +++ b/libavfilter/version.h
> @@ -31,8 +31,8 @@
>
> #include "version_major.h"
>
> -#define LIBAVFILTER_VERSION_MINOR 2
> -#define LIBAVFILTER_VERSION_MICRO 102
> +#define LIBAVFILTER_VERSION_MINOR 3
> +#define LIBAVFILTER_VERSION_MICRO 100
>
>
> #define LIBAVFILTER_VERSION_INT AV_VERSION_INT(LIBAVFILTER_VERSION_MAJOR, \
> diff --git a/libavfilter/vf_pu21.c b/libavfilter/vf_pu21.c
> new file mode 100644
> index 00..c3b56f9c1b
> --- /dev/null
> +++ b/libavfilter/vf_pu21.c
> @@ -0,0 +1,210 @@
> +
Nit: empty line at top of file.
You should have a copyright disclaimer here.
> +#include "libavfilter/avfilter.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/pixdesc.h"
> +#include "libavutil/mem.h"
> +#include "libavutil/imgutils.h"
> +#include "libavfilter/formats.h"
> +#include "libavutil/internal.h"
> +#include "video.h"
> +
> +static enum pu21_mode {
The static keyword here is ignored and a compilation warning.
> + BANDING,
> + BANDING_GLARE,
> + PEAKS,
> + PEAKS_GLARE
> +};
> +
> +typedef struct PU21Context {
> + const AVClass* class;
> + double L_min, L_max;
> + int multiplier;
> + int mode;
> +
> + int depth;
> + int nb_planes;
> +} PU21Context;
> +
> +const float pu21_params[4][7] = {
> + // Reference: "PU21: A novel perceptually uniform encoding for adapting
> existing quality metrics for HDR"
> + // Rafał K. Mantiuk and M. Azimi, Picture Coding Symposium 2021
> + // https://github.com/gfxdisp/pu21
> + {1.070275272f, 0.4088273932f, 0.153224308f, 0.2520326168f, 1.063512885f,
> 1.14115047f, 521.4527484f}, // BANDING
> + {0.353487901f, 0.3734658629f, 8.277049286e-05f, 0.9062562627f,
> 0.09150303166f, 0.9099517204f, 596.3148142f}, // BANDING_GLARE
> + {1.043882782f, 0.6459495343f, 0.3194584211f, 0.374025247f, 1.114783422f,
> 1.095360363f, 384.9217577f}, // PEAKS
> + {816.885024f, 1479.463946f, 0.001253215609f, 0.9329636822f,
> 0.06746643971f, 1.573435413f, 419.6006374f} // PEAKS_GLARE
> +};
> +
> +
> +#define OFFSET(x) offsetof(PU21Context, x)
> +#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
> +static const AVOption pu21_options[] = {
> +{ "L_min", "Set the minimum luminance value", OFFSET(L_min),
> AV_OPT_TYPE_DOUBLE, {.dbl = 0.005}, 0.005, 1, FLAGS },
> +{ "L_max", "Set the maximum luminance value", OFFSET(L_max),
> AV_OPT_TYPE_DOUBLE, {.dbl = 1}, 0.005, 1, FLAGS },
L_min and L_max aren't used anywhere.
> +{ "multiplier", "Set the parameters for the encoding",
> OFFSET(multiplier), AV_OPT_TYPE_INT, {.i64 = 1}, 1, 1, FLAGS},
This description could be more helpful.
> +{ "mode", "Set the mode of
[FFmpeg-devel] [PATCH] lavc/vvc: Remove experimental flag
This reverts commit 110d8549d575aae6b2f627cd63e2eb7082ab8926.
I have been working through fixing bugs, particularly crashes I've
found using a fuzzer, in the VVC decoder for the past few months.
While I won't claim it is now bug-free, it is considerably more
resilient than it was and I think in a position to have the
experimental flag removed for release 7.1.
Additionally, most of the Main 10 features of VVC which were missing
version of the decoder released in 7.0 have now been implemented.
This includes the most major missing features: IBC, subpictures and RPR.
Signed-off-by: Frank Plowman
---
libavcodec/vvc/dec.c | 3 +--
tests/fate/vvc.mak | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c
index 2c80f0c461..edf2607f50 100644
--- a/libavcodec/vvc/dec.c
+++ b/libavcodec/vvc/dec.c
@@ -1108,8 +1108,7 @@ const FFCodec ff_vvc_decoder = {
.close = vvc_decode_free,
FF_CODEC_DECODE_CB(vvc_decode_frame),
.flush = vvc_decode_flush,
-.p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY |
AV_CODEC_CAP_OTHER_THREADS |
- AV_CODEC_CAP_EXPERIMENTAL,
+.p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY |
AV_CODEC_CAP_OTHER_THREADS,
.caps_internal = FF_CODEC_CAP_EXPORTS_CROPPING |
FF_CODEC_CAP_INIT_CLEANUP |
FF_CODEC_CAP_AUTO_THREADS,
.p.profiles = NULL_IF_CONFIG_SMALL(ff_vvc_profiles),
diff --git a/tests/fate/vvc.mak b/tests/fate/vvc.mak
index 3e762ec65e..5335460263 100644
--- a/tests/fate/vvc.mak
+++ b/tests/fate/vvc.mak
@@ -41,7 +41,7 @@ $(foreach VAR,$(FATE_VVC_VARS), $(eval VVC_TESTS_$(VAR) :=
$(addprefix fate-vvc-
$(VVC_TESTS_8BIT): SCALE_OPTS := -pix_fmt yuv420p
$(VVC_TESTS_10BIT): SCALE_OPTS := -pix_fmt yuv420p10le -vf scale
$(VVC_TESTS_444_10BIT): SCALE_OPTS := -pix_fmt yuv444p10le -vf scale
-fate-vvc-conformance-%: CMD = framecrc -c:v vvc -strict experimental -i
$(TARGET_SAMPLES)/vvc-conformance/$(subst fate-vvc-conformance-,,$(@)).bit
$(SCALE_OPTS)
+fate-vvc-conformance-%: CMD = framecrc -c:v vvc -i
$(TARGET_SAMPLES)/vvc-conformance/$(subst fate-vvc-conformance-,,$(@)).bit
$(SCALE_OPTS)
FATE_VVC-$(call FRAMECRC, VVC, VVC, VVC_PARSER) += $(VVC_TESTS_8BIT)
FATE_VVC-$(call FRAMECRC, VVC, VVC, VVC_PARSER SCALE_FILTER) +=\
--
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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] tools/target_{bsf, dec}_fuzzer: Stop skipping end of input
For the final input in a file, the while loop above will not break and
as a result
data + sizeof(fuzz_tag) == end
when it exits. There is an off-by-one error in the changed line,
which is meant to handle this case. The result is that the final 8
bytes of all input files are skipped.
Signed-off-by: Frank Plowman
---
tools/target_bsf_fuzzer.c | 2 +-
tools/target_dec_fuzzer.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/target_bsf_fuzzer.c b/tools/target_bsf_fuzzer.c
index 44a4d1467d..c3761bb33e 100644
--- a/tools/target_bsf_fuzzer.c
+++ b/tools/target_bsf_fuzzer.c
@@ -131,7 +131,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t
size) {
break;
data++;
}
-if (data + sizeof(fuzz_tag) > end)
+if (data + sizeof(fuzz_tag) >= end)
data = end;
res = av_new_packet(pkt, data - last);
diff --git a/tools/target_dec_fuzzer.c b/tools/target_dec_fuzzer.c
index 794b5b92cc..550d34fc61 100644
--- a/tools/target_dec_fuzzer.c
+++ b/tools/target_dec_fuzzer.c
@@ -485,7 +485,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t
size) {
break;
data++;
}
-if (data + sizeof(fuzz_tag) > end)
+if (data + sizeof(fuzz_tag) >= end)
data = end;
res = av_new_packet(parsepkt, data - last);
--
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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Prevent OOB access in subpic_tiles
The previous logic relied on the subpicture boundaries coinciding with the tile boundaries. Per 6.3.1 of H.266 (V3), vertical subpicture boundaries are always tile boundaries however the same cannot be said for horizontal subpicture boundaries. Furthermore, it is possible to construct an illegal bitstream where vertical subpicture boundaries are not coincident with tile boundaries. In these cases, the condition of the while loop would never be satisfied resulting in an OOB read on col_bd/row_bd. Patch fixes this issue by replacing != with <, thereby not requiring subpicture boundaries and tile boundaries to be coincident. Signed-off-by: Frank Plowman --- libavcodec/vvc/ps.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c index 58496c9fba..ff9a6c7a15 100644 --- a/libavcodec/vvc/ps.c +++ b/libavcodec/vvc/ps.c @@ -384,10 +384,10 @@ static void subpic_tiles(int *tile_x, int *tile_y, int *tile_x_end, int *tile_y_ *tile_x = *tile_y = 0; -while (pps->col_bd[*tile_x] != rx) +while (pps->col_bd[*tile_x] < rx) (*tile_x)++; -while (pps->row_bd[*tile_y] != ry) +while (pps->row_bd[*tile_y] < ry) (*tile_y)++; *tile_x_end = (*tile_x); -- 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/4 v2] avcodec: add LCEVC decoding support via LCEVCdec
On 21/07/2024 23:53, James Almer wrote:
> Signed-off-by: James Almer
> ---
> configure | 3 +
> doc/general_contents.texi | 13 ++
> libavcodec/Makefile | 1 +
> libavcodec/lcevcdec.c | 276 ++
> libavcodec/lcevcdec.h | 44 ++
> 5 files changed, 337 insertions(+)
> create mode 100644 libavcodec/lcevcdec.c
> create mode 100644 libavcodec/lcevcdec.h
>
> diff --git a/configure b/configure
> index f6f5c29fea..d1f32684a6 100755
> --- a/configure
> +++ b/configure
> @@ -225,6 +225,7 @@ External library support:
>--enable-libcdio enable audio CD grabbing with libcdio [no]
>--enable-libcodec2 enable codec2 en/decoding using libcodec2 [no]
>--enable-libdav1denable AV1 decoding via libdav1d [no]
> + --enable-liblcevc_decenable LCEVC decoding via liblcevc_dec [no]
>--enable-libdavs2enable AVS2 decoding via libdavs2 [no]
>--enable-libdc1394 enable IIDC-1394 grabbing using libdc1394
> and libraw1394 [no]
> @@ -1914,6 +1915,7 @@ EXTERNAL_LIBRARY_LIST="
> libcelt
> libcodec2
> libdav1d
> +liblcevc_dec
> libdc1394
> libflite
> libfontconfig
> @@ -6854,6 +6856,7 @@ enabled libcelt && require libcelt
> celt/celt.h celt_decode -lcelt0 &&
> enabled libcaca && require_pkg_config libcaca caca caca.h
> caca_create_canvas
> enabled libcodec2 && require libcodec2 codec2/codec2.h codec2_create
> -lcodec2
> enabled libdav1d && require_pkg_config libdav1d "dav1d >= 0.5.0"
> "dav1d/dav1d.h" dav1d_version
> +enabled liblcevc_dec && require_pkg_config liblcevc_dec "lcevc_dec >=
> 2.0.0" "LCEVC/lcevc_dec.h" LCEVC_CreateDecoder
> enabled libdavs2 && require_pkg_config libdavs2 "davs2 >= 1.6.0"
> davs2.h davs2_decoder_open
> enabled libdc1394 && require_pkg_config libdc1394 libdc1394-2
> dc1394/dc1394.h dc1394_new
> enabled libdrm&& check_pkg_config libdrm libdrm xf86drm.h
> drmGetVersion
> diff --git a/doc/general_contents.texi b/doc/general_contents.texi
> index e7cf4f8239..ecaf3979ce 100644
> --- a/doc/general_contents.texi
> +++ b/doc/general_contents.texi
> @@ -245,6 +245,19 @@ Go to @url{https://github.com/google/liblc3/} and follow
> the instructions for
> installing the library.
> Then pass @code{--enable-liblc3} to configure to enable it.
>
> +@section LCEVCdec
> +
> +FFmpeg can make use of the liblcevc_dec library for LCEVC enhacement layer
> +decoding on supported bitstreams.
> +
> +Go to @url{https://github.com/v-novaltd/LCEVCdec} and follow the instructions
> +for installing the library. Then pass @code{--enable-libvpx} to configure to
^
Should be --enable-liblcevc_dec
> +enable it.
> +
> +@float NOTE
> +LCEVCdec is under the BSD-3-Clause-Clear License.
> +@end float
> +
> @section OpenH264
>
> FFmpeg can make use of the OpenH264 library for H.264 decoding and encoding.
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 771e2b597e..71bc3c8075 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -121,6 +121,7 @@ OBJS-$(CONFIG_INTRAX8) += intrax8.o
> intrax8dsp.o msmpeg4_vc1_dat
> OBJS-$(CONFIG_IVIDSP) += ivi_dsp.o
> OBJS-$(CONFIG_JNI) += ffjni.o jni.o
> OBJS-$(CONFIG_JPEGTABLES) += jpegtables.o
> +OBJS-$(CONFIG_LIBLCEVC_DEC)+= lcevcdec.o
> OBJS-$(CONFIG_LCMS2) += fflcms2.o
> OBJS-$(CONFIG_LLAUDDSP)+= lossless_audiodsp.o
> OBJS-$(CONFIG_LLVIDDSP)+= lossless_videodsp.o
> diff --git a/libavcodec/lcevcdec.c b/libavcodec/lcevcdec.c
> new file mode 100644
> index 00..4edb0b72dc
> --- /dev/null
> +++ b/libavcodec/lcevcdec.c
> @@ -0,0 +1,276 @@
> +/*
> + * 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-1301
> USA
> + */
> +
> +#include "libavutil/avassert.h"
> +#include "libavutil/frame.h"
> +#include "libavutil/imgutils.h"
> +#include "libavutil/log.h"
> +#include "libavutil/mem.h"
> +#include "decode.h"
> +#include "lcevcdec.h"
> +
> +static LCEVC_Color
Re: [FFmpeg-devel] [PATCH] lavc/vvc: Always set flags for the current picture
On 27/06/2024 13:52, Nuo Mi wrote:
> On Mon, Jun 24, 2024 at 11:30 PM Frank Plowman
> wrote:
>
>> ff_vvc_frame_rpl uses the flags to detect whether a frame is in use.
>> Therefore, in the case of a CVSS AU (RASL/GDR with
>> NoOutputBeforeRecoveryFlag) with ph_non_ref_pic_flag = 1, the frame
>> would be freed before it is used. Fix this by always marking the
>> current frame with VVC_FRAME_FLAG_SHORT_REF, as is done by the HEVC
>> decoder.
>>
>> Additionally, add an assert0 to mitigate the effects of a frame being
>> freed before it is used.
>>
>> Signed-off-by: Frank Plowman
>> ---
>> libavcodec/vvc/refs.c | 2 +-
>> libavcodec/vvc/thread.c | 2 ++
>> 2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
>> index 8b7ba639a3..26a5b0b34c 100644
>> --- a/libavcodec/vvc/refs.c
>> +++ b/libavcodec/vvc/refs.c
>> @@ -191,7 +191,7 @@ int ff_vvc_set_new_ref(VVCContext *s, VVCFrameContext
>> *fc, AVFrame **frame)
>> fc->ref = ref;
>>
>> if (s->no_output_before_recovery_flag && (IS_RASL(s) ||
>> !GDR_IS_RECOVERED(s)))
>> -ref->flags = 0;
>> +ref->flags = VVC_FRAME_FLAG_SHORT_REF;
>> else if (ph->r->ph_pic_output_flag)
>> ref->flags = VVC_FRAME_FLAG_OUTPUT;
>>
>> diff --git a/libavcodec/vvc/thread.c b/libavcodec/vvc/thread.c
>> index 5b01dd2d20..e87ed5b676 100644
>> --- a/libavcodec/vvc/thread.c
>> +++ b/libavcodec/vvc/thread.c
>> @@ -801,6 +801,8 @@ int ff_vvc_frame_wait(VVCContext *s, VVCFrameContext
>> *fc)
>> {
>> VVCFrameThread *ft = fc->ft;
>>
>> +av_assert0(fc->ref->progress);
>> +
>>
> Hi Frank,
> Thank you for the patch.
>
> We have a mission to remove all assert0s in the decoder, right? :)
> If it's only for debugging, maybe we don't need it.
>
I think assertions do have a role besides debugging in mitigating the
effects of a bug, however in this case I think it is probably not
necessary. v2 sent.
--
Frank
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2] lavc/vvc: Always set flags for the current picture
ff_vvc_frame_rpl uses the flags to detect whether a frame is in use. Therefore, in the case of a CVSS AU (RASL/GDR with NoOutputBeforeRecoveryFlag) with ph_non_ref_pic_flag = 1, the frame would be freed before it is used. Fix this by always marking the current frame with VVC_FRAME_FLAG_SHORT_REF, as is done by the HEVC decoder. Signed-off-by: Frank Plowman --- libavcodec/vvc/refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c index 8b7ba639a3..26a5b0b34c 100644 --- a/libavcodec/vvc/refs.c +++ b/libavcodec/vvc/refs.c @@ -191,7 +191,7 @@ int ff_vvc_set_new_ref(VVCContext *s, VVCFrameContext *fc, AVFrame **frame) fc->ref = ref; if (s->no_output_before_recovery_flag && (IS_RASL(s) || !GDR_IS_RECOVERED(s))) -ref->flags = 0; +ref->flags = VVC_FRAME_FLAG_SHORT_REF; else if (ph->r->ph_pic_output_flag) ref->flags = VVC_FRAME_FLAG_OUTPUT; -- 2.45.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/vvcdec: Fix compiling with MSVC 2022 17.8 and older
On 26/06/2024 09:20, Martin Storsjö wrote:
> Versions of MSVC older than 17.9 error out here with the following
> error:
>
> src/libavcodec/vvc/filter.c(815): error C2059: syntax error: '}'
> src/libavcodec/vvc/filter.c(832): error C2065: 'all_zero_bs': undeclared
> identifier
> src/libavcodec/vvc/filter.c(836): error C2065: 'all_zero_bs': undeclared
> identifier
> ---
> libavcodec/vvc/filter.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/vvc/filter.c b/libavcodec/vvc/filter.c
> index 2cadaaaf22..7ffcb29f47 100644
> --- a/libavcodec/vvc/filter.c
> +++ b/libavcodec/vvc/filter.c
> @@ -812,7 +812,7 @@ static void vvc_deblock(const VVCLocalContext *lc, int
> x0, int y0, const int rs,
> for (int y = y0; y < y_end; y += (DEBLOCK_STEP << vs)) {
> for (int x = x0 ? x0 : grid; x < x_end; x += grid) {
> const uint8_t horizontal_ctu_edge = !vertical && !(x %
> ctb_size);
> -int32_t bs[4], beta[4], tc[4] = { }, all_zero_bs = 1;
> +int32_t bs[4], beta[4], tc[4] = { 0 }, all_zero_bs = 1;
> uint8_t max_len_p[4], max_len_q[4];
>
> for (int i = 0; i < DEBLOCK_STEP >> (2 - vs); i++) {
LGTM.
--
Frank
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 5/5] lavc/hevcdec: improve check for PPS changing between slices
On 26/06/2024 13:43, Anton Khirnov wrote:
> Compare actual PPS objects rather than just PPS ID, as the former might
> change while the latter stays the same.
>
> Reported-by: Michael Niedermayer
> ---
> libavcodec/hevc/hevcdec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
> index 01d32086f2..fd143cddab 100644
> --- a/libavcodec/hevc/hevcdec.c
> +++ b/libavcodec/hevc/hevcdec.c
> @@ -603,7 +603,7 @@ static int hls_slice_header(SliceHeader *sh, const
> HEVCContext *s, GetBitContext
> av_log(s->avctx, AV_LOG_ERROR, "PPS id out of range: %d\n", pps_id);
> return AVERROR_INVALIDDATA;
> }
> -if (!sh->first_slice_in_pic_flag && pps_id != sh->pps_id) {
> +if (!sh->first_slice_in_pic_flag && s->ps.pps_list[pps_id] != s->pps) {
> av_log(s->avctx, AV_LOG_ERROR, "PPS changed between slices.\n");
> return AVERROR_INVALIDDATA;
> }
LGTM.
--
Frank
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Don't discard return codes
Signed-off-by: Frank Plowman
---
libavcodec/vvc/thread.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/libavcodec/vvc/thread.c b/libavcodec/vvc/thread.c
index c90c316661..118d56b67e 100644
--- a/libavcodec/vvc/thread.c
+++ b/libavcodec/vvc/thread.c
@@ -443,8 +443,11 @@ static int run_inter(VVCContext *s, VVCLocalContext *lc,
VVCTask *t)
{
VVCFrameContext *fc = lc->fc;
const CTU *ctu = fc->tab.ctus + t->rs;
+int ret;
-ff_vvc_predict_inter(lc, t->rs);
+ret = ff_vvc_predict_inter(lc, t->rs);
+if (ret < 0)
+return ret;
if (ctu->has_dmvr)
report_frame_progress(fc, t->ry, VVC_PROGRESS_MV);
@@ -454,9 +457,7 @@ static int run_inter(VVCContext *s, VVCLocalContext *lc,
VVCTask *t)
static int run_recon(VVCContext *s, VVCLocalContext *lc, VVCTask *t)
{
-ff_vvc_reconstruct(lc, t->rs, t->rx, t->ry);
-
-return 0;
+return ff_vvc_reconstruct(lc, t->rs, t->rx, t->ry);
}
static int run_lmcs(VVCContext *s, VVCLocalContext *lc, VVCTask *t)
--
2.45.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/vvc: Validate IBC block vector
On 25/06/2024 15:19, Nuo Mi wrote:
> On Mon, Jun 24, 2024 at 9:31 PM Frank Plowman wrote:
>
>> From H.266 (V3) (09/2023) p. 321:
>>
>> It is a requirement of bitstream conformance that the luma block
>> vector bvL shall obey the following constraints:
>> - CtbSizeY is greater than or equal to
>> ((yCb + (bvL[ 1 ] >> 4)) & (CtbSizeY − 1)) + cbHeight
>>
>> This patch checks this is true, which fixes crashes on fuzzed
>> bitstreams.
>>
>> Signed-off-by: Frank Plowman
>> ---
>> libavcodec/vvc/intra.c | 25 ++---
>> libavcodec/vvc/thread.c | 4 +---
>> 2 files changed, 23 insertions(+), 6 deletions(-)
>>
>> diff --git a/libavcodec/vvc/intra.c b/libavcodec/vvc/intra.c
>> index f77a012f09..11371db797 100644
>> --- a/libavcodec/vvc/intra.c
>> +++ b/libavcodec/vvc/intra.c
>> @@ -624,15 +624,26 @@ static void intra_block_copy(const VVCLocalContext
>> *lc, const int c_idx)
>> }
>> }
>>
>> -static void vvc_predict_ibc(const VVCLocalContext *lc)
>> +static int vvc_predict_ibc(const VVCLocalContext *lc)
>> {
>> -const H266RawSPS *rsps = lc->fc->ps.sps->r;
>> +const VVCFrameContext *fc = lc->fc;
>> +const VVCSPS *sps = lc->fc->ps.sps;
>> +const H266RawSPS *rsps= sps->r;
>> +const CodingUnit *cu = lc->cu;
>> +const Mv *bv = &cu->pu.mi.mv[L0][0];
>> +
>> +if (sps->ctb_size_y < ((cu->y0 + (bv->y >> 4)) & (sps->ctb_size_y -
>> 1)) + cu->cb_height) {
>> +av_log(fc->log_ctx, AV_LOG_ERROR, "IBC region spans multiple
>> CTBs.\n");
>> +return AVERROR_INVALIDDATA;
>> +}
>
> Hi Frank,
> Thank you for the patch.
> Could we wrap it as a function and call it in ff_vvc_mvp_ibc and
> ff_vvc_luma_mv_merge_ibc?
> This will help us detect errors earlier.
>
I agree that's a better place to do the check. I didn't realise the
code paths were already so separate for IBC there. v2 sent.
Cheers,
--
Frank
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2] lavc/vvc: Validate IBC block vector
From H.266 (V3) (09/2023) p. 321:
It is a requirement of bitstream conformance that the luma block
vector bvL shall obey the following constraints:
- CtbSizeY is greater than or equal to
((yCb + (bvL[ 1 ] >> 4)) & (CtbSizeY − 1)) + cbHeight
This patch checks this is true, which fixes crashes on fuzzed
bitstreams.
Signed-off-by: Frank Plowman
---
Changes since v1:
* Perform this check when the IBC block vectors are derived, rather than
when they are first used.
libavcodec/vvc/ctu.c | 19 +++
libavcodec/vvc/mvs.c | 21 +++--
libavcodec/vvc/mvs.h | 4 ++--
3 files changed, 36 insertions(+), 8 deletions(-)
diff --git a/libavcodec/vvc/ctu.c b/libavcodec/vvc/ctu.c
index 752a06e2f5..1a4ceafc25 100644
--- a/libavcodec/vvc/ctu.c
+++ b/libavcodec/vvc/ctu.c
@@ -1444,20 +1444,25 @@ static void merge_data_block(VVCLocalContext *lc)
}
}
-static void merge_data_ibc(VVCLocalContext *lc)
+static int merge_data_ibc(VVCLocalContext *lc)
{
const VVCFrameContext* fc = lc->fc;
const VVCSPS* sps = fc->ps.sps;
MotionInfo *mi= &lc->cu->pu.mi;
int merge_idx = 0;
+int ret;
mi->pred_flag = PF_IBC;
if (sps->max_num_ibc_merge_cand > 1)
merge_idx = ff_vvc_merge_idx(lc);
-ff_vvc_luma_mv_merge_ibc(lc, merge_idx, &mi->mv[L0][0]);
+ret = ff_vvc_luma_mv_merge_ibc(lc, merge_idx, &mi->mv[L0][0]);
+if (ret)
+return ret;
ff_vvc_store_mv(lc, mi);
+
+return 0;
}
static int hls_merge_data(VVCLocalContext *lc)
@@ -1466,11 +1471,14 @@ static int hls_merge_data(VVCLocalContext *lc)
const VVCPH *ph= &fc->ps.ph;
const CodingUnit *cu= lc->cu;
PredictionUnit *pu = &lc->cu->pu;
+int ret;
pu->merge_gpm_flag = 0;
pu->mi.num_sb_x = pu->mi.num_sb_y = 1;
if (cu->pred_mode == MODE_IBC) {
-merge_data_ibc(lc);
+ret = merge_data_ibc(lc);
+if (ret)
+return ret;
} else {
if (ph->max_num_subblock_merge_cand > 0 && cu->cb_width >= 8 &&
cu->cb_height >= 8)
pu->merge_subblock_flag = ff_vvc_merge_subblock_flag(lc);
@@ -1596,6 +1604,7 @@ static int mvp_data_ibc(VVCLocalContext *lc)
int mvp_l0_flag = 0;
int amvr_shift= 4;
Mv *mv= &mi->mv[L0][0];
+int ret;
mi->pred_flag = PF_IBC;
mi->num_sb_x = 1;
@@ -1607,7 +1616,9 @@ static int mvp_data_ibc(VVCLocalContext *lc)
if (sps->r->sps_amvr_enabled_flag && (mv->x || mv->y))
amvr_shift = ff_vvc_amvr_shift(lc, pu->inter_affine_flag,
cu->pred_mode, 1);
-ff_vvc_mvp_ibc(lc, mvp_l0_flag, amvr_shift, mv);
+ret = ff_vvc_mvp_ibc(lc, mvp_l0_flag, amvr_shift, mv);
+if (ret)
+return ret;
ff_vvc_store_mv(lc, mi);
return 0;
diff --git a/libavcodec/vvc/mvs.c b/libavcodec/vvc/mvs.c
index 42564b3e6f..1788a7150b 100644
--- a/libavcodec/vvc/mvs.c
+++ b/libavcodec/vvc/mvs.c
@@ -1695,17 +1695,34 @@ static void ibc_merge_candidates(VVCLocalContext *lc,
const int merge_idx, Mv *m
memset(mv, 0, sizeof(*mv));
}
-void ff_vvc_mvp_ibc(VVCLocalContext *lc, const int mvp_l0_flag, const int
amvr_shift, Mv *mv)
+static int ibc_check_mv(VVCLocalContext *lc, Mv *mv)
+{
+const VVCFrameContext *fc = lc->fc;
+const VVCSPS *sps = lc->fc->ps.sps;
+const CodingUnit *cu = lc->cu;
+const Mv *bv = &cu->pu.mi.mv[L0][0];
+
+if (sps->ctb_size_y < ((cu->y0 + (bv->y >> 4)) & (sps->ctb_size_y - 1)) +
cu->cb_height) {
+av_log(fc->log_ctx, AV_LOG_ERROR, "IBC region spans multiple CTBs.\n");
+return AVERROR_INVALIDDATA;
+}
+
+return 0;
+}
+
+int ff_vvc_mvp_ibc(VVCLocalContext *lc, const int mvp_l0_flag, const int
amvr_shift, Mv *mv)
{
LOCAL_ALIGNED_8(Mv, mvp, [1]);
ibc_merge_candidates(lc, mvp_l0_flag, mvp);
ibc_add_mvp(mv, mvp, amvr_shift);
+return ibc_check_mv(lc, mv);
}
-void ff_vvc_luma_mv_merge_ibc(VVCLocalContext *lc, const int merge_idx, Mv *mv)
+int ff_vvc_luma_mv_merge_ibc(VVCLocalContext *lc, const int merge_idx, Mv *mv)
{
ibc_merge_candidates(lc, merge_idx, mv);
+return ibc_check_mv(lc, mv);
}
static int affine_mvp_constructed_cp(NeighbourContext *ctx,
diff --git a/libavcodec/vvc/mvs.h b/libavcodec/vvc/mvs.h
index 3f0c8b08e9..b2242b2a4d 100644
--- a/libavcodec/vvc/mvs.h
+++ b/libavcodec/vvc/mvs.h
@@ -30,9 +30,9 @@ void ff_vvc_clip_mv(Mv *mv);
void ff_vvc_mv_scale(Mv *dst, const Mv *src, int td, int tb);
void ff_vvc_luma_mv_merge_mode(VVCLocalContext *lc, int merge_idx, int
ciip_flag, MvField *mv);
void ff_vvc_luma_mv_merge_gpm(VVCLocalContext *lc, const int merge_gpm_idx[2],
MvField *mv);
-void ff_vvc_luma_mv_merge_ibc(VVC
[FFmpeg-devel] [PATCH] lavc/vvc: Always set flags for the current picture
ff_vvc_frame_rpl uses the flags to detect whether a frame is in use.
Therefore, in the case of a CVSS AU (RASL/GDR with
NoOutputBeforeRecoveryFlag) with ph_non_ref_pic_flag = 1, the frame
would be freed before it is used. Fix this by always marking the
current frame with VVC_FRAME_FLAG_SHORT_REF, as is done by the HEVC
decoder.
Additionally, add an assert0 to mitigate the effects of a frame being
freed before it is used.
Signed-off-by: Frank Plowman
---
libavcodec/vvc/refs.c | 2 +-
libavcodec/vvc/thread.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
index 8b7ba639a3..26a5b0b34c 100644
--- a/libavcodec/vvc/refs.c
+++ b/libavcodec/vvc/refs.c
@@ -191,7 +191,7 @@ int ff_vvc_set_new_ref(VVCContext *s, VVCFrameContext *fc,
AVFrame **frame)
fc->ref = ref;
if (s->no_output_before_recovery_flag && (IS_RASL(s) ||
!GDR_IS_RECOVERED(s)))
-ref->flags = 0;
+ref->flags = VVC_FRAME_FLAG_SHORT_REF;
else if (ph->r->ph_pic_output_flag)
ref->flags = VVC_FRAME_FLAG_OUTPUT;
diff --git a/libavcodec/vvc/thread.c b/libavcodec/vvc/thread.c
index 5b01dd2d20..e87ed5b676 100644
--- a/libavcodec/vvc/thread.c
+++ b/libavcodec/vvc/thread.c
@@ -801,6 +801,8 @@ int ff_vvc_frame_wait(VVCContext *s, VVCFrameContext *fc)
{
VVCFrameThread *ft = fc->ft;
+av_assert0(fc->ref->progress);
+
ff_mutex_lock(&ft->lock);
while (atomic_load(&ft->nb_scheduled_tasks) ||
atomic_load(&ft->nb_scheduled_listeners))
--
2.45.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Validate IBC block vector
From H.266 (V3) (09/2023) p. 321:
It is a requirement of bitstream conformance that the luma block
vector bvL shall obey the following constraints:
- CtbSizeY is greater than or equal to
((yCb + (bvL[ 1 ] >> 4)) & (CtbSizeY − 1)) + cbHeight
This patch checks this is true, which fixes crashes on fuzzed
bitstreams.
Signed-off-by: Frank Plowman
---
libavcodec/vvc/intra.c | 25 ++---
libavcodec/vvc/thread.c | 4 +---
2 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/libavcodec/vvc/intra.c b/libavcodec/vvc/intra.c
index f77a012f09..11371db797 100644
--- a/libavcodec/vvc/intra.c
+++ b/libavcodec/vvc/intra.c
@@ -624,15 +624,26 @@ static void intra_block_copy(const VVCLocalContext *lc,
const int c_idx)
}
}
-static void vvc_predict_ibc(const VVCLocalContext *lc)
+static int vvc_predict_ibc(const VVCLocalContext *lc)
{
-const H266RawSPS *rsps = lc->fc->ps.sps->r;
+const VVCFrameContext *fc = lc->fc;
+const VVCSPS *sps = lc->fc->ps.sps;
+const H266RawSPS *rsps= sps->r;
+const CodingUnit *cu = lc->cu;
+const Mv *bv = &cu->pu.mi.mv[L0][0];
+
+if (sps->ctb_size_y < ((cu->y0 + (bv->y >> 4)) & (sps->ctb_size_y - 1)) +
cu->cb_height) {
+av_log(fc->log_ctx, AV_LOG_ERROR, "IBC region spans multiple CTBs.\n");
+return AVERROR_INVALIDDATA;
+}
intra_block_copy(lc, LUMA);
if (lc->cu->tree_type == SINGLE_TREE && rsps->sps_chroma_format_idc) {
intra_block_copy(lc, CB);
intra_block_copy(lc, CR);
}
+
+return 0;
}
static void ibc_fill_vir_buf(const VVCLocalContext *lc, const CodingUnit *cu)
@@ -678,7 +689,10 @@ int ff_vvc_reconstruct(VVCLocalContext *lc, const int rs,
const int rx, const in
if (cu->ciip_flag)
ff_vvc_predict_ciip(lc);
else if (cu->pred_mode == MODE_IBC)
-vvc_predict_ibc(lc);
+ret = vvc_predict_ibc(lc);
+if (ret)
+goto fail;
+
if (cu->coded_flag) {
ret = reconstruct(lc);
} else {
@@ -687,10 +701,15 @@ int ff_vvc_reconstruct(VVCLocalContext *lc, const int rs,
const int rx, const in
if (sps->r->sps_chroma_format_idc && cu->tree_type !=
DUAL_TREE_LUMA)
add_reconstructed_area(lc, CHROMA, cu->x0, cu->y0,
cu->cb_width, cu->cb_height);
}
+if (ret)
+goto fail;
+
if (sps->r->sps_ibc_enabled_flag)
ibc_fill_vir_buf(lc, cu);
cu = cu->next;
}
+
+fail:
ff_vvc_ctu_free_cus(ctu);
return ret;
}
diff --git a/libavcodec/vvc/thread.c b/libavcodec/vvc/thread.c
index 8777d380bf..5b01dd2d20 100644
--- a/libavcodec/vvc/thread.c
+++ b/libavcodec/vvc/thread.c
@@ -454,9 +454,7 @@ static int run_inter(VVCContext *s, VVCLocalContext *lc,
VVCTask *t)
static int run_recon(VVCContext *s, VVCLocalContext *lc, VVCTask *t)
{
-ff_vvc_reconstruct(lc, t->rs, t->rx, t->ry);
-
-return 0;
+return ff_vvc_reconstruct(lc, t->rs, t->rx, t->ry);
}
static int run_lmcs(VVCContext *s, VVCLocalContext *lc, VVCTask *t)
--
2.45.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] fate/vvc: add vvc-conformance-RPR_A_4
On 10/06/2024 18:46, Frank Plowman wrote: > BeforeAfter > - > make fate-vvc CPU Time (No ASM) 131.52s 134.83s > libavcodec/vvc/* Line Coverage 95.3%96.9% > inter_template.c Line Coverage 74.3%88.2% > inter.c Line Coverage 85.3%99.2% > > Signed-off-by: Frank Plowman > --- > tests/fate/vvc.mak | 1 + > tests/ref/fate/vvc-conformance-RPR_A_4 | 9 + > 2 files changed, 10 insertions(+) > create mode 100644 tests/ref/fate/vvc-conformance-RPR_A_4 > > diff --git a/tests/fate/vvc.mak b/tests/fate/vvc.mak > index 0ff287eea6..3e762ec65e 100644 > --- a/tests/fate/vvc.mak > +++ b/tests/fate/vvc.mak > @@ -14,6 +14,7 @@ VVC_SAMPLES_10BIT = \ > POC_A_1 \ > PPS_B_1 \ > RAP_A_1 \ > +RPR_A_4 \ > SAO_A_3 \ > SCALING_A_1 \ > SLICES_A_3\ > diff --git a/tests/ref/fate/vvc-conformance-RPR_A_4 > b/tests/ref/fate/vvc-conformance-RPR_A_4 > new file mode 100644 > index 00..58ae0f3861 > --- /dev/null > +++ b/tests/ref/fate/vvc-conformance-RPR_A_4 > @@ -0,0 +1,9 @@ > +#tb 0: 1/25 > +#media_type 0: video > +#codec_id 0: rawvideo > +#dimensions 0: 832x480 > +#sar 0: 0/1 > +0, 0, 0,1, 1198080, 0x2c12c2be > +0, 1, 1,1, 1198080, 0x47275378 > +0, 2, 2,1, 1198080, 0x5d7b0327 > +0, 3, 3,1, 1198080, 0x0b15318a Ping ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/vvc: Invalidate PPSs which refer to a changed SPS
n 15/06/2024 13:24, Nuo Mi wrote: > On Sat, Jun 15, 2024 at 2:35 PM Christophe Gisquet < > christophe.gisq...@gmail.com> wrote: > >> Le ven. 14 juin 2024, 11:39, Frank Plowman a >> écrit : >> >>> When the SPS associated with a particular SPS ID changes, invalidate all >>> the PPSs which use that SPS ID. Fixes crashes with illegal bitstreams. >>> This is done in the CBS, rather than in libavcodec/vvc/ps.c like the SPS >>> ID reuse validation, as parts of the CBS parsing process for PPSs >>> depend on the SPS being referred to. >>> >> >> I am uncertain about this. I have no definite knowledge nor proof, but I >> would have thought these are persistent, IE it's legal to update some of >> them, their validity depending on something else. >> > >> Wondering if the tested streams are thus conformant. >> >> But I don't know the actual rule. Maybe finding an EOB/EOS NUT? Related to >> some particular shape of a clean random access point, that would require >> retransmitting VPS/SPS/PPS/APS/... ? >> >> Asking Benjamin Bross might be a better option here. >> > Hi Chris, > spec said sps should not change in a CVS. Frank has some patches to fix a > similar issue. > https://github.com/FFmpeg/FFmpeg/commit/2d79ae3f8a3306d24afe43ba505693a8dbefd21b > > > Hi Frank, > Did it crash before your error hand code in ps.c? > Could you send me the clip? > > Thank you > Hi both, Thank you for your reviews. An example of a crashing bitstream which is fixed by this patch is ID 295 available here: https://github.com/ffvvc/tests/pull/43. The relevant part of the bitstream is a sequence of NAL units AU (decode_order=5) 18. SPS sps_seq_parameter_set_id = 0 sps_ctb_log2_size_y = 5 19. PPS pps_pic_parameter_set_id = 0 pps_seq_parameter_set_id = 0 20. IDR_N_LP ph_pic_order_cnt_lsb = 0 NoOutputBeforeRecoveryFlag = 1 ph_pic_parameter_set_id = 0 AU (decode_order=6) 21. AUD 22. VPS 23. SPS sps_seq_parameter_set_id = 0 sps_ctb_log2_size_y = 7 24. PREFIX_APS 25. IDR_N_LP ph_pic_order_cnt_lsb = 0 NoOutputBeforeRecoveryFlag = 1 ph_pic_parameter_set_id = 0 The layout of SPSs alone is legal (not covered by the checks introduced in 2d79ae3f8a3306d24afe43ba505693a8dbefd21b) as the second AU is a CLVSS AU. As a result, the bitstream crashes both before and after 2d79ae3f8a3306d24afe43ba505693a8dbefd21b. What this patch does is produce an error when the VCL NAL unit in the second AU (25.) tries to use PPS ID 0, as the SPS NAL unit that PPS was defined with reference to (18.) is no longer available. Christophe, is my interpretation of your point correct when I say you are suggesting that the above sequence may be legal, so long as the PPS still satisfies the new bounds etc. derived from the second SPS? I did consider this, and I think it may be possible to implement by delaying CBS element validation and inference until libavcodec/vvc/ps.c. However, there are no bitstreams in the conformance suite which contain such a structure and this is different to how the native HEVC decoder behaves (see libavcodec/hevc/ps.c:72). All the best, -- Frank ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] rtp enc/dec update for vvc
Hi, Thanks for the patch. Unfortunately it looks to be corrupted and does not apply. Also, it looks as though you submitted five near-identical patches. I would suggest you try directing patches to your own mailbox and re-applying them while debugging the formatting issues, rather than sending lots of corrupted patches to the ML. A couple more comments inline. Cheers, Frank On 14/06/2024 03:35, ftaft2000 wrote: > Signed-off-by: ftaft2000 > --- > .gitignore | 1 + > configure | 4 + > libavcodec/Makefile | 1 + > libavcodec/allcodecs.c | 1 + > libavcodec/libvvenc.c | 566 + > libavformat/Makefile | 1 + > libavformat/rtpdec.c | 1 + > libavformat/rtpdec_formats.h | 1 + > libavformat/rtpdec_vvc.c | 349 > libavformat/rtpenc.c | 2 + > libavformat/rtpenc_h264_hevc.c | 94 +- > libavformat/sdp.c | 182 +++ > 12 files changed, 1197 insertions(+), 6 deletions(-) > create mode 100644 libavcodec/libvvenc.c > create mode 100644 libavformat/rtpdec_vvc.c > > diff --git a/.gitignore b/.gitignore > index e810d11107..d7441a6cdc 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -41,3 +41,4 @@ > /src > /mapfile > /tools/python/__pycache__/ > +/build I don't think this should be here - this is specific to your environment. If you want to ignore something locally, you can use .git/info/exclude > \ No newline at end of file > diff --git a/configure b/configure > index 83284427df..d331688eb4 100755 > --- a/configure > +++ b/configure > @@ -296,6 +296,7 @@ External library support: > --enable-libwebp enable WebP encoding via libwebp [no] > --enable-libx264 enable H.264 encoding via x264 [no] > --enable-libx265 enable HEVC encoding via x265 [no] > + --enable-libvvenc enable H.266/VVC encoding via vvenc [no] This looks like you had the VVenC patchset applied when you created this commit. If your patch depends on the VVenC patchset, it will have to wait until that is applied (which could well be in the next day or two). > --enable-libxeve enable EVC encoding via libxeve [no] > --enable-libxevd enable EVC decoding via libxevd [no] > --enable-libxavs enable AVS encoding via xavs [no] > @@ -1867,6 +1868,7 @@ EXTERNAL_LIBRARY_GPL_LIST=" > libvidstab > libx264 > libx265 > + libvvenc > libxavs > libxavs2 > libxvid > @@ -3569,6 +3571,7 @@ libx264rgb_encoder_deps="libx264" > libx264rgb_encoder_select="libx264_encoder" > libx265_encoder_deps="libx265" > libx265_encoder_select="atsc_a53 dovi_rpuenc" > +libvvenc_encoder_deps="libvvenc" > libxavs_encoder_deps="libxavs" > libxavs2_encoder_deps="libxavs2" > libxevd_decoder_deps="libxevd" > @@ -7041,6 +7044,7 @@ enabled libx264 && require_pkg_config > libx264 x264 "stdint.h x264.h" x > check_cpp_condition libx262 x264.h > "X264_MPEG2" > enabled libx265 && require_pkg_config libx265 x265 x265.h > x265_api_get && > require_cpp_condition libx265 x265.h > "X265_BUILD >= 89" > +enabled libvvenc && require_pkg_config libvvenc "libvvenc >= > 1.6.1" "vvenc/vvenc.h" vvenc_get_version > enabled libxavs && require libxavs "stdint.h xavs.h" > xavs_encoder_encode "-lxavs $pthreads_extralibs $libm_extralibs" > enabled libxavs2 && require_pkg_config libxavs2 "xavs2 >= > 1.3.0" "stdint.h xavs2.h" xavs2_api_get > enabled libxevd && require_pkg_config libxevd "xevd >= 0.4.1" > "xevd.h" xevd_decode > diff --git a/libavcodec/Makefile b/libavcodec/Makefile > index 1a44352906..2e98e1c72f 100644 > --- a/libavcodec/Makefile > +++ b/libavcodec/Makefile > @@ -1155,6 +1155,7 @@ OBJS-$(CONFIG_LIBWEBP_ANIM_ENCODER) += > libwebpenc_common.o libwebpenc_anim > OBJS-$(CONFIG_LIBX262_ENCODER) += libx264.o > OBJS-$(CONFIG_LIBX264_ENCODER) += libx264.o > OBJS-$(CONFIG_LIBX265_ENCODER) += libx265.o > +OBJS-$(CONFIG_LIBVVENC_ENCODER) += libvvenc.o > OBJS-$(CONFIG_LIBXAVS_ENCODER) += libxavs.o > OBJS-$(CONFIG_LIBXAVS2_ENCODER) += libxavs2.o > OBJS-$(CONFIG_LIBXEVD_DECODER) += libxevd.o > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c > index b102a8069e..7650abebe4 100644 > --- a/libavcodec/allcodecs.c > +++ b/libavcodec/allcodecs.c > @@ -807,6 +807,7 @@ extern const FFCodec ff_libx262_encoder; > extern const FFCodec ff_libx264_encoder; > extern const FFCodec ff_libx264rgb_encoder; > extern FFCodec ff_libx265_encoder; > +extern const FFCodec ff_libvvenc_encoder; > extern const FFCodec ff_libxeve_encoder; > extern const FFCodec ff_libxevd_decoder; > extern const FFCodec ff_libxavs_encoder; > diff --git a/libavcodec/libvvenc.c b/libavcodec/libvvenc.c > new file mode 100644 > inde
[FFmpeg-devel] [PATCH] lavc/vvc: Invalidate PPSs which refer to a changed SPS
When the SPS associated with a particular SPS ID changes, invalidate all
the PPSs which use that SPS ID. Fixes crashes with illegal bitstreams.
This is done in the CBS, rather than in libavcodec/vvc/ps.c like the SPS
ID reuse validation, as parts of the CBS parsing process for PPSs
depend on the SPS being referred to.
Signed-off-by: Frank Plowman
---
libavcodec/cbs_h2645.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
index c5f167334d..e2389f124e 100644
--- a/libavcodec/cbs_h2645.c
+++ b/libavcodec/cbs_h2645.c
@@ -789,9 +789,28 @@ static int cbs_h26 ## h26n ## _replace_ ##
ps_var(CodedBitstreamContext *ctx, \
}
cbs_h266_replace_ps(6, VPS, vps, vps_video_parameter_set_id)
-cbs_h266_replace_ps(6, SPS, sps, sps_seq_parameter_set_id)
cbs_h266_replace_ps(6, PPS, pps, pps_pic_parameter_set_id)
+static int cbs_h266_replace_sps(CodedBitstreamContext *ctx,
+CodedBitstreamUnit *unit)
+{
+CodedBitstreamH266Context *priv = ctx->priv_data;
+H266RawSPS *sps = unit->content;
+unsigned int id = sps->sps_seq_parameter_set_id;
+int err = ff_cbs_make_unit_refcounted(ctx, unit);
+if (err < 0)
+return err;
+av_assert0(unit->content_ref);
+if (priv->sps[id] && memcmp(priv->sps[id], unit->content_ref,
sizeof(*priv->sps[id]))) {
+for (unsigned int i = 0; i < VVC_MAX_PPS_COUNT; i++) {
+if (priv->pps[i] && priv->pps[i]->pps_seq_parameter_set_id == id)
+ff_refstruct_unref(&priv->pps[i]);
+}
+}
+ff_refstruct_replace(&priv->sps[id], unit->content_ref);
+return 0;
+}
+
static int cbs_h266_replace_ph(CodedBitstreamContext *ctx,
CodedBitstreamUnit *unit,
H266RawPictureHeader *ph)
--
2.45.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] fate/vvc: add vvc-conformance-RPR_A_Alibaba_4
On 10/06/2024 18:31, James Almer wrote: > On 6/10/2024 2:23 PM, Frank Plowman wrote: >> On 10/06/2024 14:26, Nuo Mi wrote: >>> Hi Frank, >>> Thank you for the patch. >>> Could we follow the naming conventions used for other clips? >>> >> Hi, >> >> I did it this way because I do not have access to the FATE server, so >> somebody had to upload the clip for me and I felt it was easier if they >> did not have to rename the clip. Also there is already a mixture of VVC >> clip titles with and without the vendor's name. >> >> If someone renames the clip on the FATE server it is easy to send a v2. > > I just renamed it to RPR_A_4.bit Thank you James. v2 sent. -- Frank ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2] fate/vvc: add vvc-conformance-RPR_A_4
BeforeAfter - make fate-vvc CPU Time (No ASM) 131.52s 134.83s libavcodec/vvc/* Line Coverage 95.3%96.9% inter_template.c Line Coverage 74.3%88.2% inter.c Line Coverage 85.3%99.2% Signed-off-by: Frank Plowman --- tests/fate/vvc.mak | 1 + tests/ref/fate/vvc-conformance-RPR_A_4 | 9 + 2 files changed, 10 insertions(+) create mode 100644 tests/ref/fate/vvc-conformance-RPR_A_4 diff --git a/tests/fate/vvc.mak b/tests/fate/vvc.mak index 0ff287eea6..3e762ec65e 100644 --- a/tests/fate/vvc.mak +++ b/tests/fate/vvc.mak @@ -14,6 +14,7 @@ VVC_SAMPLES_10BIT = \ POC_A_1 \ PPS_B_1 \ RAP_A_1 \ +RPR_A_4 \ SAO_A_3 \ SCALING_A_1 \ SLICES_A_3\ diff --git a/tests/ref/fate/vvc-conformance-RPR_A_4 b/tests/ref/fate/vvc-conformance-RPR_A_4 new file mode 100644 index 00..58ae0f3861 --- /dev/null +++ b/tests/ref/fate/vvc-conformance-RPR_A_4 @@ -0,0 +1,9 @@ +#tb 0: 1/25 +#media_type 0: video +#codec_id 0: rawvideo +#dimensions 0: 832x480 +#sar 0: 0/1 +0, 0, 0,1, 1198080, 0x2c12c2be +0, 1, 1,1, 1198080, 0x47275378 +0, 2, 2,1, 1198080, 0x5d7b0327 +0, 3, 3,1, 1198080, 0x0b15318a -- 2.45.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] fate/vvc: add vvc-conformance-RPR_A_Alibaba_4
On 10/06/2024 14:26, Nuo Mi wrote: > Hi Frank, > Thank you for the patch. > Could we follow the naming conventions used for other clips? > Hi, I did it this way because I do not have access to the FATE server, so somebody had to upload the clip for me and I felt it was easier if they did not have to rename the clip. Also there is already a mixture of VVC clip titles with and without the vendor's name. If someone renames the clip on the FATE server it is easy to send a v2. Cheers, -- Frank ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] fate/vvc: add vvc-conformance-RPR_A_Alibaba_4
BeforeAfter - make fate-vvc CPU Time (No ASM) 131.52s 134.83s libavcodec/vvc/* Line Coverage 95.3%96.9% inter_template.c Line Coverage 74.3%88.2% inter.c Line Coverage 85.3%99.2% Signed-off-by: Frank Plowman --- tests/fate/vvc.mak | 1 + tests/ref/fate/vvc-conformance-RPR_A_Alibaba_4 | 9 + 2 files changed, 10 insertions(+) create mode 100644 tests/ref/fate/vvc-conformance-RPR_A_Alibaba_4 diff --git a/tests/fate/vvc.mak b/tests/fate/vvc.mak index 0ff287eea6..93b86f15f0 100644 --- a/tests/fate/vvc.mak +++ b/tests/fate/vvc.mak @@ -14,6 +14,7 @@ VVC_SAMPLES_10BIT = \ POC_A_1 \ PPS_B_1 \ RAP_A_1 \ +RPR_A_Alibaba_4 \ SAO_A_3 \ SCALING_A_1 \ SLICES_A_3\ diff --git a/tests/ref/fate/vvc-conformance-RPR_A_Alibaba_4 b/tests/ref/fate/vvc-conformance-RPR_A_Alibaba_4 new file mode 100644 index 00..58ae0f3861 --- /dev/null +++ b/tests/ref/fate/vvc-conformance-RPR_A_Alibaba_4 @@ -0,0 +1,9 @@ +#tb 0: 1/25 +#media_type 0: video +#codec_id 0: rawvideo +#dimensions 0: 832x480 +#sar 0: 0/1 +0, 0, 0,1, 1198080, 0x2c12c2be +0, 1, 1,1, 1198080, 0x47275378 +0, 2, 2,1, 1198080, 0x5d7b0327 +0, 3, 3,1, 1198080, 0x0b15318a -- 2.45.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v4] lavc/vvc: Prevent overflow in chroma QP derivation
On the top of p. 112 in VVC (09/2023):
It is a requirement of bitstream conformance that the values of
qpInVal[ i ][ j ] and qpOutVal[ i ][ j ] shall be in the range
of −QpBdOffset to 63, inclusive for i in the range of 0 to
numQpTables − 1, inclusive, and j in the range of 0 to
sps_num_points_in_qp_table_minus1[ i ] + 1, inclusive.
Additionally, don't discard the return code from sps_chroma_qp_table.
Signed-off-by: Frank Plowman
---
Changes since v3:
* Add comment noting why qp_{in,out} are not tested themselves.
libavcodec/vvc/ps.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c
index 1b23675c98..92368eafc2 100644
--- a/libavcodec/vvc/ps.c
+++ b/libavcodec/vvc/ps.c
@@ -101,9 +101,14 @@ static int sps_chroma_qp_table(VVCSPS *sps)
qp_out[0] = qp_in[0] = r->sps_qp_table_start_minus26[i] + 26;
for (int j = 0; j < num_points_in_qp_table; j++ ) {
+const uint8_t delta_qp_out = (r->sps_delta_qp_in_val_minus1[i][j]
^ r->sps_delta_qp_diff_val[i][j]);
delta_qp_in[j] = r->sps_delta_qp_in_val_minus1[i][j] + 1;
+// Note: we cannot check qp_{in,out}[j+1] here as qp_*[j] +
delta_qp_*
+// may not fit in an 8-bit signed integer.
+if (qp_in[j] + delta_qp_in[j] > 63 || qp_out[j] + delta_qp_out >
63)
+return AVERROR(EINVAL);
qp_in[j+1] = qp_in[j] + delta_qp_in[j];
-qp_out[j+1] = qp_out[j] + (r->sps_delta_qp_in_val_minus1[i][j] ^
r->sps_delta_qp_diff_val[i][j]);
+qp_out[j+1] = qp_out[j] + delta_qp_out;
}
sps->chroma_qp_table[i][qp_in[0] + off] = qp_out[0];
for (int k = qp_in[0] - 1 + off; k >= 0; k--)
@@ -186,8 +191,11 @@ static int sps_derive(VVCSPS *sps, void *log_ctx)
sps_inter(sps);
sps_partition_constraints(sps);
sps_ladf(sps);
-if (r->sps_chroma_format_idc != 0)
-sps_chroma_qp_table(sps);
+if (r->sps_chroma_format_idc != 0) {
+ret = sps_chroma_qp_table(sps);
+if (ret < 0)
+return ret;
+}
return 0;
}
--
2.45.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 2/2] lavc/vvc: Prevent overflow in chroma QP derivation
Hi,
Thanks for your review.
On 05/06/2024 14:50, Nuo Mi wrote:
> Hi Frank,
> Thank you for the patch
>
> On Wed, Jun 5, 2024 at 5:24 PM Frank Plowman wrote:
>
>> On the top of p. 112 in VVC (09/2023):
>>
>> It is a requirement of bitstream conformance that the values of
>> qpInVal[ i ][ j ] and qpOutVal[ i ][ j ] shall be in the range
>> of −QpBdOffset to 63, inclusive for i in the range of 0 to
>>
> Then, why do we not check −QpBdOffset?
sps_delta_qp_in_val_minus1 is unsigned, therefore we would only need to
check the first elements qp{In,Out}Val[i][0], both of which are set to
sps_qp_table_start_minus26[i] + 26.
sps_qp_table_start_minus26[i] is already constrained to the range
[-26-QpBdOffset..36] (see VVC (09/2023) p. 111 and
libavcodec/cbs_h266_syntax_template.c:1387).
I don't get why the standard reiterates the constraint here, it seems
redundant.
>
>> numQpTables − 1, inclusive, and j in the range of 0 to
>> sps_num_points_in_qp_table_minus1[ i ] + 1, inclusive.
>>
>> Signed-off-by: Frank Plowman
>> ---
>> libavcodec/vvc/ps.c | 7 ++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c
>> index bfc3c121fd..c4f64d5da7 100644
>> --- a/libavcodec/vvc/ps.c
>> +++ b/libavcodec/vvc/ps.c
>> @@ -101,9 +101,14 @@ static int sps_chroma_qp_table(VVCSPS *sps)
>>
>> qp_out[0] = qp_in[0] = r->sps_qp_table_start_minus26[i] + 26;
>> for (int j = 0; j < num_points_in_qp_table; j++ ) {
>> +const uint8_t delta_qp_out =
>> (r->sps_delta_qp_in_val_minus1[i][j] ^ r->sps_delta_qp_diff_val[i][j]);
>> delta_qp_in[j] = r->sps_delta_qp_in_val_minus1[i][j] + 1;
>> +if (qp_in[j] + delta_qp_in[j] > 63)
>> +return AVERROR(EINVAL);
>> qp_in[j+1] = qp_in[j] + delta_qp_in[j];
>> -qp_out[j+1] = qp_out[j] +
>> (r->sps_delta_qp_in_val_minus1[i][j] ^ r->sps_delta_qp_diff_val[i][j]);
>> +if (qp_out[j] + delta_qp_out > 63)
>> +return AVERROR(EINVAL);
>> +qp_out[j+1] = qp_out[j] + delta_qp_out;
>>
> Instead of changing so many lines, we can add 2 lines here
> if (qp_in[j+1] < 63 || qp_out[j+1] < 63)
> return AVERROR(EINVAL);
v3 sent with this tweak & squashing the other patch.
>
>> }
>> sps->chroma_qp_table[i][qp_in[0] + off] = qp_out[0];
>> for (int k = qp_in[0] - 1 + off; k >= 0; k--)
>> --
>> 2.45.1
>>
Cheers,
--
Frank
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3] lavc/vvc: Prevent overflow in chroma QP derivation
On the top of p. 112 in VVC (09/2023):
It is a requirement of bitstream conformance that the values of
qpInVal[ i ][ j ] and qpOutVal[ i ][ j ] shall be in the range
of −QpBdOffset to 63, inclusive for i in the range of 0 to
numQpTables − 1, inclusive, and j in the range of 0 to
sps_num_points_in_qp_table_minus1[ i ] + 1, inclusive.
Additionally, don't discard the return code from sps_chroma_qp_table.
Signed-off-by: Frank Plowman
---
Changes since v2:
* Squash discarded return code patch and QP overflow patch.
* Combine QpIn and QpOut validation into a single if statement.
libavcodec/vvc/ps.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c
index 1b23675c98..ea5d0e9959 100644
--- a/libavcodec/vvc/ps.c
+++ b/libavcodec/vvc/ps.c
@@ -101,9 +101,12 @@ static int sps_chroma_qp_table(VVCSPS *sps)
qp_out[0] = qp_in[0] = r->sps_qp_table_start_minus26[i] + 26;
for (int j = 0; j < num_points_in_qp_table; j++ ) {
+const uint8_t delta_qp_out = (r->sps_delta_qp_in_val_minus1[i][j]
^ r->sps_delta_qp_diff_val[i][j]);
delta_qp_in[j] = r->sps_delta_qp_in_val_minus1[i][j] + 1;
+if (qp_in[j] + delta_qp_in[j] > 63 || qp_out[j] + delta_qp_out >
63)
+return AVERROR(EINVAL);
qp_in[j+1] = qp_in[j] + delta_qp_in[j];
-qp_out[j+1] = qp_out[j] + (r->sps_delta_qp_in_val_minus1[i][j] ^
r->sps_delta_qp_diff_val[i][j]);
+qp_out[j+1] = qp_out[j] + delta_qp_out;
}
sps->chroma_qp_table[i][qp_in[0] + off] = qp_out[0];
for (int k = qp_in[0] - 1 + off; k >= 0; k--)
@@ -186,8 +189,11 @@ static int sps_derive(VVCSPS *sps, void *log_ctx)
sps_inter(sps);
sps_partition_constraints(sps);
sps_ladf(sps);
-if (r->sps_chroma_format_idc != 0)
-sps_chroma_qp_table(sps);
+if (r->sps_chroma_format_idc != 0) {
+ret = sps_chroma_qp_table(sps);
+if (ret < 0)
+return ret;
+}
return 0;
}
--
2.45.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 2/2] lavc/vvc: Prevent overflow in chroma QP derivation
On the top of p. 112 in VVC (09/2023):
It is a requirement of bitstream conformance that the values of
qpInVal[ i ][ j ] and qpOutVal[ i ][ j ] shall be in the range
of −QpBdOffset to 63, inclusive for i in the range of 0 to
numQpTables − 1, inclusive, and j in the range of 0 to
sps_num_points_in_qp_table_minus1[ i ] + 1, inclusive.
Signed-off-by: Frank Plowman
---
libavcodec/vvc/ps.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c
index bfc3c121fd..c4f64d5da7 100644
--- a/libavcodec/vvc/ps.c
+++ b/libavcodec/vvc/ps.c
@@ -101,9 +101,14 @@ static int sps_chroma_qp_table(VVCSPS *sps)
qp_out[0] = qp_in[0] = r->sps_qp_table_start_minus26[i] + 26;
for (int j = 0; j < num_points_in_qp_table; j++ ) {
+const uint8_t delta_qp_out = (r->sps_delta_qp_in_val_minus1[i][j]
^ r->sps_delta_qp_diff_val[i][j]);
delta_qp_in[j] = r->sps_delta_qp_in_val_minus1[i][j] + 1;
+if (qp_in[j] + delta_qp_in[j] > 63)
+return AVERROR(EINVAL);
qp_in[j+1] = qp_in[j] + delta_qp_in[j];
-qp_out[j+1] = qp_out[j] + (r->sps_delta_qp_in_val_minus1[i][j] ^
r->sps_delta_qp_diff_val[i][j]);
+if (qp_out[j] + delta_qp_out > 63)
+return AVERROR(EINVAL);
+qp_out[j+1] = qp_out[j] + delta_qp_out;
}
sps->chroma_qp_table[i][qp_in[0] + off] = qp_out[0];
for (int k = qp_in[0] - 1 + off; k >= 0; k--)
--
2.45.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 1/2] lavc/vvc: Use sps_chroma_qp_table return code
Signed-off-by: Frank Plowman
---
libavcodec/vvc/ps.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c
index 1b23675c98..bfc3c121fd 100644
--- a/libavcodec/vvc/ps.c
+++ b/libavcodec/vvc/ps.c
@@ -186,8 +186,11 @@ static int sps_derive(VVCSPS *sps, void *log_ctx)
sps_inter(sps);
sps_partition_constraints(sps);
sps_ladf(sps);
-if (r->sps_chroma_format_idc != 0)
-sps_chroma_qp_table(sps);
+if (r->sps_chroma_format_idc != 0) {
+ret = sps_chroma_qp_table(sps);
+if (ret < 0)
+return ret;
+}
return 0;
}
--
2.45.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Prevent overflow in chroma QP derivation
On the top of p. 112 in VVC (09/2023):
It is a requirement of bitstream conformance that the values of
qpInVal[ i ][ j ] and qpOutVal[ i ][ j ] shall be in the range
of −QpBdOffset to 63, inclusive for i in the range of 0 to
numQpTables − 1, inclusive, and j in the range of 0 to
sps_num_points_in_qp_table_minus1[ i ] + 1, inclusive.
Signed-off-by: Frank Plowman
---
libavcodec/vvc/ps.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c
index 1b23675c98..b024caf460 100644
--- a/libavcodec/vvc/ps.c
+++ b/libavcodec/vvc/ps.c
@@ -101,9 +101,14 @@ static int sps_chroma_qp_table(VVCSPS *sps)
qp_out[0] = qp_in[0] = r->sps_qp_table_start_minus26[i] + 26;
for (int j = 0; j < num_points_in_qp_table; j++ ) {
+const uint8_t delta_qp_out = (r->sps_delta_qp_in_val_minus1[i][j]
^ r->sps_delta_qp_diff_val[i][j]);
delta_qp_in[j] = r->sps_delta_qp_in_val_minus1[i][j] + 1;
+if (qp_in[j] + delta_qp_in[j] > 63)
+return AVERROR_INVALIDDATA;
qp_in[j+1] = qp_in[j] + delta_qp_in[j];
-qp_out[j+1] = qp_out[j] + (r->sps_delta_qp_in_val_minus1[i][j] ^
r->sps_delta_qp_diff_val[i][j]);
+if (qp_out[j] + delta_qp_out > 63)
+return AVERROR_INVALIDDATA;
+qp_out[j+1] = qp_out[j] + delta_qp_out;
}
sps->chroma_qp_table[i][qp_in[0] + off] = qp_out[0];
for (int k = qp_in[0] - 1 + off; k >= 0; k--)
--
2.45.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Reallocate pixel buffers if pixel shift changes
Allocations in the following lines depend on the pixel shift, and so these buffers must be reallocated if the pixel shift changes. Patch fixes segmentation faults in fuzzed bitstreams. Signed-off-by: Frank Plowman --- libavcodec/vvc/dec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c index e53ad4e607..f5603306f3 100644 --- a/libavcodec/vvc/dec.c +++ b/libavcodec/vvc/dec.c @@ -214,7 +214,8 @@ static void pixel_buffer_nz_tl_init(TabList *l, VVCFrameContext *fc) const int c_end = chroma_idc ? VVC_MAX_SAMPLE_ARRAYS : 1; const int changed= fc->tab.sz.chroma_format_idc != chroma_idc || fc->tab.sz.width != width || fc->tab.sz.height != height || -fc->tab.sz.ctu_width != ctu_width || fc->tab.sz.ctu_height != ctu_height; +fc->tab.sz.ctu_width != ctu_width || fc->tab.sz.ctu_height != ctu_height || +fc->tab.sz.pixel_shift != ps; tl_init(l, 0, changed); -- 2.45.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Don't free uninitialised pic arrays
The picture arrays are not initialised at the same time as the frame
context itself, but rather when the relevant frame begins being decoded.
As such, situations can arise where the frame context is being freed but
the picture arrays have not yet been initialised. This could lead to
various UB and ultimately crashes. Patch prevents this by adding an
initialised flag associated with the picture arrays.
Signed-off-by: Frank Plowman
---
libavcodec/vvc/dec.c | 7 +++
libavcodec/vvc/dec.h | 2 ++
2 files changed, 9 insertions(+)
diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c
index e53ad4e607..32e5bc0cd8 100644
--- a/libavcodec/vvc/dec.c
+++ b/libavcodec/vvc/dec.c
@@ -327,6 +327,9 @@ static void free_cus(VVCFrameContext *fc)
static void pic_arrays_free(VVCFrameContext *fc)
{
+if (!fc->tab.initialised)
+return;
+
free_cus(fc);
frame_context_for_each_tl(fc, tl_free);
ff_refstruct_pool_uninit(&fc->rpl_tab_pool);
@@ -380,6 +383,8 @@ static int pic_arrays_init(VVCContext *s, VVCFrameContext
*fc)
fc->tab.sz.bs_width = (fc->ps.pps->width >> 2) + 1;
fc->tab.sz.bs_height = (fc->ps.pps->height >> 2) + 1;
+fc->tab.initialised = 1;
+
return 0;
}
@@ -627,6 +632,8 @@ static av_cold int frame_context_init(VVCFrameContext *fc,
AVCodecContext *avctx
if (!fc->tu_pool)
return AVERROR(ENOMEM);
+fc->tab.initialised = 0;
+
return 0;
}
diff --git a/libavcodec/vvc/dec.h b/libavcodec/vvc/dec.h
index 1e0b76f283..1721ba3a15 100644
--- a/libavcodec/vvc/dec.h
+++ b/libavcodec/vvc/dec.h
@@ -212,6 +212,8 @@ typedef struct VVCFrameContext {
int bs_height;
int ibc_buffer_width; ///< IbcBufWidth
} sz;
+
+int initialised;
} tab;
} VVCFrameContext;
--
2.45.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Validate temporal MVP references
Per VVCv3 p. 157, the collocated reference picture used in temporal
motion vector prediction must have RprConstraintsActiveFlag equal to
zero and the same CTU size as the current picture. Add these checks,
fixing crashes decoding some fuzzed bitstreams.
Additionally, only set up the collocated reference picture if it is
actually going to be used (i.e. if ph_temporal_mvp_enabled_flag is 1),
else legal RPR bitstreams will fail the new checks.
Co-authored-by: Nuo Mi
Signed-off-by: Frank Plowman
---
libavcodec/vvc/refs.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
index fb42963034..8b7ba639a3 100644
--- a/libavcodec/vvc/refs.c
+++ b/libavcodec/vvc/refs.c
@@ -506,9 +506,14 @@ int ff_vvc_slice_rpl(VVCContext *s, VVCFrameContext *fc,
SliceContext *sc)
return ret;
}
}
-if ((!rsh->sh_collocated_from_l0_flag) == lx &&
-rsh->sh_collocated_ref_idx < rpl->nb_refs)
-fc->ref->collocated_ref =
rpl->refs[rsh->sh_collocated_ref_idx].ref;
+if (ph->r->ph_temporal_mvp_enabled_flag &&
+(!rsh->sh_collocated_from_l0_flag) == lx &&
+rsh->sh_collocated_ref_idx < rpl->nb_refs) {
+const VVCRefPic *refp = rpl->refs + rsh->sh_collocated_ref_idx;
+if (refp->is_scaled || refp->ref->sps->ctb_log2_size_y !=
sps->ctb_log2_size_y)
+return AVERROR_INVALIDDATA;
+fc->ref->collocated_ref = refp->ref;
+}
}
return 0;
}
--
2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavf/dash: Forward strict flag to component demuxers
Thanks for your review Andreas.
On 20/05/2024 21:41, Andreas Rheinhardt wrote:
> Frank Plowman:
>> Before the patch, opening a DASH file containing streams which require
>> experimental decoders was problematic. No matter where the -strict -2
>> was put on the command line, the option was not passed to the demuxer
>> for that component. This resulted in an error, prompting the user to
>> add the -strict -2 flag, which is already present. Decoding appeared to
>> continue correctly however.
>>
>> Patch removes the error message by creating an options object for the
>> demuxer created for the component, which inherits from the parent
>> demuxer.
>>
>> Signed-off-by: Frank Plowman
>> ---
>> PS: Can anyone think of other options which should be propagated to the
>> component demuxers?
>>
>> libavformat/dashdec.c | 7 ++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
>> index 555e21bf69..40abb5ebba 100644
>> --- a/libavformat/dashdec.c
>> +++ b/libavformat/dashdec.c
>> @@ -1911,13 +1911,18 @@ static int
>> reopen_demux_for_component(AVFormatContext *s, struct representation
>> if (ret < 0)
>> goto fail;
>> if (pls->n_fragments) {
>> +AVDictionary *stream_info_opts = NULL;
>> +
>> #if FF_API_R_FRAME_RATE
>> if (pls->framerate.den) {
>> for (i = 0; i < pls->ctx->nb_streams; i++)
>> pls->ctx->streams[i]->r_frame_rate = pls->framerate;
>> }
>> #endif
>> -ret = avformat_find_stream_info(pls->ctx, NULL);
>> +
>> +av_dict_set_int(&stream_info_opts, "strict",
>> s->strict_std_compliance, 0);
>> +
>> +ret = avformat_find_stream_info(pls->ctx, &stream_info_opts);
>> if (ret < 0)
>> goto fail;
>> }
>
> The loop over pls->ctx indicates that pls->ctx->nb_streams can be > 1
> before avformat_find_stream_info(). But then using a single AVDictionary
> is wrong, as avformat_find_stream_info() expects an array of
> pls->ctx->nb_streams AVDictionary*.
Thanks, v2 sent which addresses this.
>
> Furthermore, the mixing between AVFormatContext and AVCodecContext
> options here does not seem good (e.g. for ordinary demuxers setting
> strict_std_compliance does not affect the AVCodecContext's values at all).
Can you see an alternative? In the case of fftools, the -strict flag is
always set for both the codec_opts and format_opts but yes I see the
concern assuming this logic in libav*.
--
Frank
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2] lavf/dash: Forward strict flag to component demuxers
Before the patch, opening a DASH file containing streams which require
experimental decoders was problematic. No matter where the -strict -2
was put on the command line, the option was not passed to the demuxer
for that component. This resulted in an error, prompting the user to
add the -strict -2 flag, which is already present. Decoding appeared to
continue correctly however.
Patch removes the error message by creating an options object for the
demuxer created for the component, which inherits from the parent
demuxer.
Signed-off-by: Frank Plowman
---
Changes since v1: Use nb_streams stream_info_opts, not just one.
libavformat/dashdec.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 555e21bf69..9c5ef5801a 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -1911,13 +1911,28 @@ static int reopen_demux_for_component(AVFormatContext
*s, struct representation
if (ret < 0)
goto fail;
if (pls->n_fragments) {
+AVDictionary **stream_info_opts;
+
+stream_info_opts = av_calloc(pls->ctx->nb_streams,
sizeof(*stream_info_opts));
+if (!stream_info_opts)
+goto fail;
+
#if FF_API_R_FRAME_RATE
if (pls->framerate.den) {
-for (i = 0; i < pls->ctx->nb_streams; i++)
+for (i = 0; i < pls->ctx->nb_streams; i++) {
pls->ctx->streams[i]->r_frame_rate = pls->framerate;
+av_dict_set_int(&stream_info_opts[i], "strict",
s->strict_std_compliance, 0);
+}
+
}
#endif
-ret = avformat_find_stream_info(pls->ctx, NULL);
+
+ret = avformat_find_stream_info(pls->ctx, stream_info_opts);
+
+for (i = 0; i < pls->ctx->nb_streams; i++)
+av_dict_free(&stream_info_opts[i]);
+av_dict_free(stream_info_opts);
+
if (ret < 0)
goto fail;
}
--
2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavf/dash: Forward strict flag to component demuxers
Before the patch, opening a DASH file containing streams which require
experimental decoders was problematic. No matter where the -strict -2
was put on the command line, the option was not passed to the demuxer
for that component. This resulted in an error, prompting the user to
add the -strict -2 flag, which is already present. Decoding appeared to
continue correctly however.
Patch removes the error message by creating an options object for the
demuxer created for the component, which inherits from the parent
demuxer.
Signed-off-by: Frank Plowman
---
PS: Can anyone think of other options which should be propagated to the
component demuxers?
libavformat/dashdec.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 555e21bf69..40abb5ebba 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -1911,13 +1911,18 @@ static int reopen_demux_for_component(AVFormatContext
*s, struct representation
if (ret < 0)
goto fail;
if (pls->n_fragments) {
+AVDictionary *stream_info_opts = NULL;
+
#if FF_API_R_FRAME_RATE
if (pls->framerate.den) {
for (i = 0; i < pls->ctx->nb_streams; i++)
pls->ctx->streams[i]->r_frame_rate = pls->framerate;
}
#endif
-ret = avformat_find_stream_info(pls->ctx, NULL);
+
+av_dict_set_int(&stream_info_opts, "strict", s->strict_std_compliance,
0);
+
+ret = avformat_find_stream_info(pls->ctx, &stream_info_opts);
if (ret < 0)
goto fail;
}
--
2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 14/24] avfilter/Makefile: Add scale(2ref)->framesync dependency
On 05/05/2024 14:40, Andreas Rheinhardt wrote: > Forgotten in e82a3997cdd6c0894869b33ba42430ac3c57fb3b. > > Signed-off-by: Andreas Rheinhardt > --- > libavfilter/Makefile | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavfilter/Makefile b/libavfilter/Makefile > index cb45697251..5543b6bf81 100644 > --- a/libavfilter/Makefile > +++ b/libavfilter/Makefile > @@ -456,7 +456,7 @@ OBJS-$(CONFIG_ROBERTS_OPENCL_FILTER) += > vf_convolution_opencl.o opencl.o > opencl/convolution.o > OBJS-$(CONFIG_ROTATE_FILTER) += vf_rotate.o > OBJS-$(CONFIG_SAB_FILTER)+= vf_sab.o > -OBJS-$(CONFIG_SCALE_FILTER) += vf_scale.o scale_eval.o > +OBJS-$(CONFIG_SCALE_FILTER) += vf_scale.o scale_eval.o > framesync.o > OBJS-$(CONFIG_SCALE_CUDA_FILTER) += vf_scale_cuda.o scale_eval.o > \ > vf_scale_cuda.ptx.o > cuda/load_helper.o > OBJS-$(CONFIG_SCALE_NPP_FILTER) += vf_scale_npp.o scale_eval.o > @@ -464,7 +464,7 @@ OBJS-$(CONFIG_SCALE_QSV_FILTER) += > vf_vpp_qsv.o > OBJS-$(CONFIG_SCALE_VAAPI_FILTER)+= vf_scale_vaapi.o > scale_eval.o vaapi_vpp.o > OBJS-$(CONFIG_SCALE_VT_FILTER) += vf_scale_vt.o scale_eval.o > OBJS-$(CONFIG_SCALE_VULKAN_FILTER) += vf_scale_vulkan.o vulkan.o > vulkan_filter.o > -OBJS-$(CONFIG_SCALE2REF_FILTER) += vf_scale.o scale_eval.o > +OBJS-$(CONFIG_SCALE2REF_FILTER) += vf_scale.o scale_eval.o > framesync.o > OBJS-$(CONFIG_SCALE2REF_NPP_FILTER) += vf_scale_npp.o scale_eval.o > OBJS-$(CONFIG_SCDET_FILTER) += vf_scdet.o > OBJS-$(CONFIG_SCHARR_FILTER) += vf_convolution.o LGTM. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavf/vf_scale: Add framesync dependency to Makefile
On 07/05/2024 22:04, Frank Plowman wrote: > Since e82a3997cdd6c0894869b33ba42430ac3c57fb3b, the scale filter has > depended on FFFrameSync. Reflect this in the Makefile, fixing compile > errors under some configurations. > > Signed-off-by: Frank Plowman > --- > libavfilter/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavfilter/Makefile b/libavfilter/Makefile > index 8571e9e2af..5f80900a53 100644 > --- a/libavfilter/Makefile > +++ b/libavfilter/Makefile > @@ -455,7 +455,7 @@ OBJS-$(CONFIG_ROBERTS_OPENCL_FILTER) += > vf_convolution_opencl.o opencl.o > opencl/convolution.o > OBJS-$(CONFIG_ROTATE_FILTER) += vf_rotate.o > OBJS-$(CONFIG_SAB_FILTER)+= vf_sab.o > -OBJS-$(CONFIG_SCALE_FILTER) += vf_scale.o scale_eval.o > +OBJS-$(CONFIG_SCALE_FILTER) += vf_scale.o scale_eval.o > framesync.o > OBJS-$(CONFIG_SCALE_CUDA_FILTER) += vf_scale_cuda.o scale_eval.o > \ > vf_scale_cuda.ptx.o > cuda/load_helper.o > OBJS-$(CONFIG_SCALE_NPP_FILTER) += vf_scale_npp.o scale_eval.o Duplicate of https://ffmpeg.org/pipermail/ffmpeg-devel/2024-May/326801.html. -- Frank ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavf/vf_scale: Add framesync dependency to Makefile
Since e82a3997cdd6c0894869b33ba42430ac3c57fb3b, the scale filter has depended on FFFrameSync. Reflect this in the Makefile, fixing compile errors under some configurations. Signed-off-by: Frank Plowman --- libavfilter/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavfilter/Makefile b/libavfilter/Makefile index 8571e9e2af..5f80900a53 100644 --- a/libavfilter/Makefile +++ b/libavfilter/Makefile @@ -455,7 +455,7 @@ OBJS-$(CONFIG_ROBERTS_OPENCL_FILTER) += vf_convolution_opencl.o opencl.o opencl/convolution.o OBJS-$(CONFIG_ROTATE_FILTER) += vf_rotate.o OBJS-$(CONFIG_SAB_FILTER)+= vf_sab.o -OBJS-$(CONFIG_SCALE_FILTER) += vf_scale.o scale_eval.o +OBJS-$(CONFIG_SCALE_FILTER) += vf_scale.o scale_eval.o framesync.o OBJS-$(CONFIG_SCALE_CUDA_FILTER) += vf_scale_cuda.o scale_eval.o \ vf_scale_cuda.ptx.o cuda/load_helper.o OBJS-$(CONFIG_SCALE_NPP_FILTER) += vf_scale_npp.o scale_eval.o -- 2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/vvcdec: ff_vvc_frame_submit, avoid initializing task twice.
On 21/04/2024 15:52, Nuo Mi wrote:
> For some error bitstreams, a CTU belongs to two slices/entry points.
> If the decoder initializes and submmits the CTU task twice, it may crash the
> program
> or cause it to enter an infinite loop.
>
> Reported-by: Frank Plowman
> ---
> libavcodec/vvc/dec.c| 7 +--
> libavcodec/vvc/thread.c | 43 -
> libavcodec/vvc/thread.h | 2 +-
> 3 files changed, 36 insertions(+), 16 deletions(-)
>
> diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c
> index 6aeec27eaf..4f7d184e43 100644
> --- a/libavcodec/vvc/dec.c
> +++ b/libavcodec/vvc/dec.c
> @@ -893,10 +893,13 @@ static int wait_delayed_frame(VVCContext *s, AVFrame
> *output, int *got_output)
>
> static int submit_frame(VVCContext *s, VVCFrameContext *fc, AVFrame *output,
> int *got_output)
> {
> -int ret;
> +int ret = ff_vvc_frame_submit(s, fc);
> +if (ret < 0)
> +return ret;
> +
> s->nb_frames++;
> s->nb_delayed++;
> -ff_vvc_frame_submit(s, fc);
> +
> if (s->nb_delayed >= s->nb_fcs) {
> if ((ret = wait_delayed_frame(s, output, got_output)) < 0)
> return ret;
> diff --git a/libavcodec/vvc/thread.c b/libavcodec/vvc/thread.c
> index 01c3ff75b1..3b27811db2 100644
> --- a/libavcodec/vvc/thread.c
> +++ b/libavcodec/vvc/thread.c
> @@ -124,11 +124,17 @@ static void task_init(VVCTask *t, VVCTaskStage stage,
> VVCFrameContext *fc, const
> atomic_store(&t->target_inter_score, 0);
> }
>
> -static void task_init_parse(VVCTask *t, SliceContext *sc, EntryPoint *ep,
> const int ctu_idx)
> +static int task_init_parse(VVCTask *t, SliceContext *sc, EntryPoint *ep,
> const int ctu_idx)
> {
> +if (t->sc) {
> +// the task already inited, error bitstream
> +return AVERROR_INVALIDDATA;
> +}
> t->sc = sc;
> t->ep = ep;
> t->ctu_idx = ctu_idx;
> +
> +return 0;
> }
>
> static uint8_t task_add_score(VVCTask *t, const VVCTaskStage stage)
> @@ -758,24 +764,35 @@ static void submit_entry_point(VVCContext *s,
> VVCFrameThread *ft, SliceContext *
> frame_thread_add_score(s, ft, t->rx, t->ry, VVC_TASK_STAGE_PARSE);
> }
>
> -void ff_vvc_frame_submit(VVCContext *s, VVCFrameContext *fc)
> +int ff_vvc_frame_submit(VVCContext *s, VVCFrameContext *fc)
> {
> VVCFrameThread *ft = fc->ft;
>
> -for (int i = 0; i < fc->nb_slices; i++) {
> -SliceContext *sc = fc->slices[i];
> -for (int j = 0; j < sc->nb_eps; j++) {
> -EntryPoint *ep = sc->eps + j;
> -for (int k = ep->ctu_start; k < ep->ctu_end; k++) {
> -const int rs = sc->sh.ctb_addr_in_curr_slice[k];
> -VVCTask *t = ft->tasks + rs;
> -
> -task_init_parse(t, sc, ep, k);
> -check_colocation(s, t);
> +// We'll handle this in two passes:
> +// Pass 0 to initialize tasks with parser, this will help detect bit
> stream error
> +// Pass 1 to shedule location check and submit the entry point
> +for (int pass = 0; pass < 2; pass++) {
> +for (int i = 0; i < fc->nb_slices; i++) {
> +SliceContext *sc = fc->slices[i];
> +for (int j = 0; j < sc->nb_eps; j++) {
> +EntryPoint *ep = sc->eps + j;
> +for (int k = ep->ctu_start; k < ep->ctu_end; k++) {
> +const int rs = sc->sh.ctb_addr_in_curr_slice[k];
> +VVCTask *t = ft->tasks + rs;
> +if (pass) {
> +check_colocation(s, t);
> +} else {
> +const int ret = task_init_parse(t, sc, ep, k);
> +if (ret < 0)
> +return ret;
> +}
> +}
> +if (pass)
> +submit_entry_point(s, ft, sc, ep);
> }
> -submit_entry_point(s, ft, sc, ep);
> }
> }
> +return 0;
> }
>
> int ff_vvc_frame_wait(VVCContext *s, VVCFrameContext *fc)
> diff --git a/libavcodec/vvc/thread.h b/libavcodec/vvc/thread.h
> index 55bb4ea244..8ac59b2ecf 100644
> --- a/libavcodec/vvc/thread.h
> +++ b/libavcodec/vvc/thread.h
> @@ -30,7 +30,7 @@ void ff_vvc_executor_free(struct AVExecutor **e);
>
> int ff_vvc_frame_thread_init(VVCFrameContext *fc);
> void ff_vvc_frame_thread_free(VVCFrameContext *fc);
> -void ff_vvc_frame_submit(VVCContext *s, VVCFrameContext *fc);
>
[FFmpeg-devel] [PATCH] lavc/videodsp: Remove emulated_edge_mc duplicate @param
emulated_edge_mc has no parameter called dst_stride. The description is
a duplicate of the description for dst_linesize.
Signed-off-by: Frank Plowman
---
libavcodec/videodsp.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/libavcodec/videodsp.h b/libavcodec/videodsp.h
index e8960b609d..d30c98b9ae 100644
--- a/libavcodec/videodsp.h
+++ b/libavcodec/videodsp.h
@@ -43,8 +43,6 @@ typedef struct VideoDSPContext {
* the border samples.
*
* @param dst destination buffer
- * @param dst_stride number of bytes between 2 vertically adjacent samples
- * in destination buffer
* @param src source buffer
* @param dst_linesize number of bytes between 2 vertically adjacent
* samples in the destination buffer
--
2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/vvc: Skip enhancement layer NAL units
On 18/04/2024 21:23, James Almer wrote:
> On 4/18/2024 5:19 PM, Frank Plowman wrote:
>> On 18/04/2024 20:43, James Almer wrote:
>>> On 4/18/2024 3:59 PM, Frank Plowman wrote:
>>>> The native VVC decoder does not yet support quality/spatial/multiview
>>>> scalability. Bitstreams requiring this feature could cause crashes.
>>>> Patch fixes this by skipping NAL units which are not in the base layer,
>>>> warning the user while doing so.
>>>>
>>>> Signed-off-by: Frank Plowman
>>>> ---
>>>> libavcodec/vvc/dec.c | 5 +
>>>> 1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c
>>>> index a4fc40b40a..5249ddf989 100644
>>>> --- a/libavcodec/vvc/dec.c
>>>> +++ b/libavcodec/vvc/dec.c
>>>> @@ -785,6 +785,11 @@ static int decode_nal_unit(VVCContext *s,
>>>> VVCFrameContext *fc, const H2645NAL *n
>>>> s->temporal_id = nal->temporal_id;
>>>> + if (nal->nuh_layer_id > 0) {
>>>> + avpriv_report_missing_feature(fc->log_ctx, "Multilayer");
>>>
>>> A more verbose message would be better.
>>>
>>
>> Thanks for your review James.
>>
>> Would "Quality/spatial/multiview scalability" be better or do you have
>> some other suggestion? The message is put in a template by
>> avpriv_report_missing_feature, e.g.
>>
>> [vvc @ 0x61803c80] Multilayer is not implemented. Update your FFmpeg
>> version to the newest one from Git. If the problem still occurs, it
>> means that your file has a feature which has not been implemented.
>>
>> so the string passed to avpriv_report_missing_feature must be a noun.
>
> Maybe "Decoding of multilayer bitstreams".
Agreed, that's a bit clearer. v2 sent.
Thanks,
--
Frank
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2] lavc/vvc: Skip enhancement layer NAL units
The native VVC decoder does not yet support quality/spatial/multiview
scalability. Bitstreams requiring this feature could cause crashes.
Patch fixes this by skipping NAL units which are not in the base layer,
warning the user while doing so.
Signed-off-by: Frank Plowman
---
Changes since v1:
* Change missing feature string from "Multilayer" to "Decoding of
multilayer bitstreams" for clarity.
libavcodec/vvc/dec.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c
index a4fc40b40a..6aeec27eaf 100644
--- a/libavcodec/vvc/dec.c
+++ b/libavcodec/vvc/dec.c
@@ -785,6 +785,12 @@ static int decode_nal_unit(VVCContext *s, VVCFrameContext
*fc, const H2645NAL *n
s->temporal_id = nal->temporal_id;
+if (nal->nuh_layer_id > 0) {
+avpriv_report_missing_feature(fc->log_ctx,
+"Decoding of multilayer bitstreams");
+return AVERROR_PATCHWELCOME;
+}
+
switch (unit->type) {
case VVC_VPS_NUT:
case VVC_SPS_NUT:
--
2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/vvc: Skip enhancement layer NAL units
On 18/04/2024 20:43, James Almer wrote:
> On 4/18/2024 3:59 PM, Frank Plowman wrote:
>> The native VVC decoder does not yet support quality/spatial/multiview
>> scalability. Bitstreams requiring this feature could cause crashes.
>> Patch fixes this by skipping NAL units which are not in the base layer,
>> warning the user while doing so.
>>
>> Signed-off-by: Frank Plowman
>> ---
>> libavcodec/vvc/dec.c | 5 +
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c
>> index a4fc40b40a..5249ddf989 100644
>> --- a/libavcodec/vvc/dec.c
>> +++ b/libavcodec/vvc/dec.c
>> @@ -785,6 +785,11 @@ static int decode_nal_unit(VVCContext *s,
>> VVCFrameContext *fc, const H2645NAL *n
>> s->temporal_id = nal->temporal_id;
>> + if (nal->nuh_layer_id > 0) {
>> + avpriv_report_missing_feature(fc->log_ctx, "Multilayer");
>
> A more verbose message would be better.
>
Thanks for your review James.
Would "Quality/spatial/multiview scalability" be better or do you have
some other suggestion? The message is put in a template by
avpriv_report_missing_feature, e.g.
[vvc @ 0x61803c80] Multilayer is not implemented. Update your FFmpeg
version to the newest one from Git. If the problem still occurs, it
means that your file has a feature which has not been implemented.
so the string passed to avpriv_report_missing_feature must be a noun.
--
Frank
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Skip enhancement layer NAL units
The native VVC decoder does not yet support quality/spatial/multiview
scalability. Bitstreams requiring this feature could cause crashes.
Patch fixes this by skipping NAL units which are not in the base layer,
warning the user while doing so.
Signed-off-by: Frank Plowman
---
libavcodec/vvc/dec.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c
index a4fc40b40a..5249ddf989 100644
--- a/libavcodec/vvc/dec.c
+++ b/libavcodec/vvc/dec.c
@@ -785,6 +785,11 @@ static int decode_nal_unit(VVCContext *s, VVCFrameContext
*fc, const H2645NAL *n
s->temporal_id = nal->temporal_id;
+if (nal->nuh_layer_id > 0) {
+avpriv_report_missing_feature(fc->log_ctx, "Multilayer");
+return AVERROR_PATCHWELCOME;
+}
+
switch (unit->type) {
case VVC_VPS_NUT:
case VVC_SPS_NUT:
--
2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] 5 year plan & Inovation
On 17/04/2024 16:22, Michael Niedermayer wrote: > On Wed, Apr 17, 2024 at 04:22:03PM +0200, Lynne wrote: >> Apr 17, 2024, 15:58 by mich...@niedermayer.cc: >>> Some ideas and why they would help FFmpeg: >>> >>> [...] >>> >> >> Just no. >> >>> * ffchat >>> (expand into realtime chat / zoom) this would >>> bring in more users and developers, and we basically have almost >>> >> >> Better leave that for others. >> There's an infinite amount of discord clones already. > > iam not following that genre that much ... > so let me ask > are there any that > * preserve privacy (discord is not secure/private) > * allow audio / video / text chat > * scalable > * need no central server > > ? > This is what Matrix (https://matrix.org/) is attempting as I understand it, among others I'm sure. This is very ambitious, and I suspect outside most FFmpeg developers' specialisms. -- Frank ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] cannot compile the ffmpeg latest on local
On 16/04/2024 11:09, B-2014 Ariyan Kashyap wrote: > Getting this err while running ./configure --enable-shared > > ./configure --enable-shared > nasm/yasm not found or too old. Use --disable-x86asm for a crippled build. > > If you think configure made a mistake, make sure you are using the latest > version from Git. If the latest version fails, report the problem to the > ffmpeg-u...@ffmpeg.org mailing list or IRC #ffmpeg on irc.libera.chat. > Include the log file "ffbuild/config.log" produced by configure as this > will help > solve the problem. Please do not direct your usage questions to this, the ffmpeg-devel, mailing list. Instead, as this message says, use the #ffmpeg channel on liberachat. -- Frank ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Increase size of ctb_size_y
sps_log2_ctu_size_minus5 is between 0 and 2, with 3 reserved for future
use. The VVC decoder allows sps_log2_ctu_size_minus5 to be 3, and so
ctb_size_y should be at least 16 bits to prevent overflows. An
alternative patch would leave sps_log2_ctu_size_minus5 as 8 bits and
disallow sps_log2_ctu_size_minus5 = 3.
Signed-off-by: Frank Plowman
---
libavcodec/vvc/ps.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/vvc/ps.h b/libavcodec/vvc/ps.h
index 78f1687fef..6656a06320 100644
--- a/libavcodec/vvc/ps.h
+++ b/libavcodec/vvc/ps.h
@@ -69,7 +69,7 @@ typedef struct VVCSPS {
uint8_t bit_depth; ///<
BitDepth
uint8_t qp_bd_offset; ///<
QpBdOffset
uint8_t ctb_log2_size_y;///<
CtbLog2SizeY
-uint8_t ctb_size_y; ///<
CtbSizeY
+uint16_tctb_size_y; ///<
CtbSizeY
uint8_t min_cb_log2_size_y; ///<
MinCbLog2SizeY
uint8_t min_cb_size_y; ///<
MinCbSizeY
uint8_t max_tb_size_y; ///<
MaxTbSizeY
--
2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/vvc/ps: reset sps_id_used on PS uninit
On 09/04/2024 14:36, Nuo Mi wrote: > On Mon, Apr 8, 2024 at 11:15 PM Frank Plowman wrote: > >> On 08/04/2024 15:12, Nuo Mi wrote: >>> On Mon, Apr 8, 2024 at 4:37 PM Frank Plowman >> wrote: >>> >>>> On 07/04/2024 15:48, James Almer wrote: >>>>> On 4/7/2024 10:38 AM, Nuo Mi wrote: >>>>>> On Sun, Apr 7, 2024 at 11:05 AM James Almer >> wrote: >>>>>> >>>>>>> Signed-off-by: James Almer >>>>>>> --- >>>>>>> libavcodec/vvc/ps.c | 1 + >>>>>>> 1 file changed, 1 insertion(+) >>>>>>> >>>>>>> diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c >>>>>>> index 3c71c34bae..83ee75fb62 100644 >>>>>>> --- a/libavcodec/vvc/ps.c >>>>>>> +++ b/libavcodec/vvc/ps.c >>>>>>> @@ -912,6 +912,7 @@ void ff_vvc_ps_uninit(VVCParamSets *ps) >>>>>>> ff_refstruct_unref(&ps->sps_list[i]); >>>>>>> for (int i = 0; i < FF_ARRAY_ELEMS(ps->pps_list); i++) >>>>>>> ff_refstruct_unref(&ps->pps_list[i]); >>>>>>> +ps->sps_id_used = 0; >>>>>>> >>>>>> Hi James, >>>>>> thank you for the patch. >>>>>> >>>>>> Is this really necessary? >>>>>> vvc_ps_uninit will be called by vvc_decode_free, >>>>>> We are not supposed to use any member of VVCParamSets after >>>>>> vvc_decode_free. >>>>> >>>>> My bad, i thought it was also called on every flush() call. >>>>> >>>>> Something like the following: >>>>> >>>>>> diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c >>>>>> index eb447604fe..463536512e 100644 >>>>>> --- a/libavcodec/vvc/dec.c >>>>>> +++ b/libavcodec/vvc/dec.c >>>>>> @@ -963,6 +963,8 @@ static av_cold void >>>>>> vvc_decode_flush(AVCodecContext *avctx) >>>>>> ff_vvc_flush_dpb(last); >>>>>> } >>>>>> >>>>>> +s->ps->sps_id_used = 0; >>>>>> + >>>>>> s->eos = 1; >>>>>> } >>>>> >>>>> Should be done on FFCodec.flush() (like when seeking) as the previous >>>>> state is no longer valid, right? >>>> >>>> Yes I agree, I think this is needed. Cases where the random access >>>> point has no leading pictures should be covered by the existing logic as >>>> these always fall at the start of a CVS, but I think this is needed to >>>> cover the case in which there are leading pictures. >>>> >>> This patch isn't necessary. >>> Leading pictures won't carry SPS. >>> IDR, CRA, and GDR will carry SPS, but they will also start a new CVS, >> which >>> will covered by the current logic. >>> >> >> I may be misunderstanding the spec, NoOutputBeforeRecoveryFlag is set >> for pictures which have no leading pictures, no? In any case take, for >> instance, a CRA picture. In most cases, CRA pictures have >> NoOutputBeforeRecoveryFlag=0, therefore are not CLVSS pictures and >> sps_id_used is not reset by the existing logic. Do we not need to reset >> sps_id_used when seeking to a CRA then? >> > After seeking, we'll set s->last_eos to 1. > For a CRA, decode_recovery_flag will set s->no_output_before_recovery_flag > to s->last_eos. > So no_output_before_recovery_flag will be 1, not 0. I see, my mistake. Yes in this case I do not think sps_id_used must be explicitly reset. -- Frank ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Fix out-of-bounds array access
The 2 which has been changed to an 8 in the array length expression is
the maximum value of sps_bitdepth_minus8. This was missed when updating
to VVCv2, which increased this maximum from 2 to 8.
Signed-off-by: Frank Plowman
---
libavcodec/vvc/intra.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/libavcodec/vvc/intra.c b/libavcodec/vvc/intra.c
index 5ac7d02c80..def113239b 100644
--- a/libavcodec/vvc/intra.c
+++ b/libavcodec/vvc/intra.c
@@ -339,18 +339,22 @@ static void derive_qp(const VVCLocalContext *lc, const
TransformUnit *tu, Transf
//8.7.3 Scaling process for transform coefficients
static av_always_inline int derive_scale(const TransformBlock *tb, const int
sh_dep_quant_used_flag)
{
-static const uint8_t rem6[63 + 2 * 6 + 1] = {
+static const uint8_t rem6[63 + 8 * 6 + 1] = {
0, 1, 2, 3, 4, 5, 0, 1, 2, 3, 4, 5, 0, 1, 2, 3, 4, 5, 0, 1, 2,
3, 4, 5, 0, 1, 2, 3, 4, 5, 0, 1, 2, 3, 4, 5, 0, 1, 2, 3, 4, 5,
0, 1, 2, 3, 4, 5, 0, 1, 2, 3, 4, 5, 0, 1, 2, 3, 4, 5, 0, 1, 2, 3,
-4, 5, 0, 1, 2, 3, 4, 5, 0, 1, 2, 3
+4, 5, 0, 1, 2, 3, 4, 5, 0, 1, 2, 3, 4, 5, 0, 1, 2, 3, 4, 5, 0, 1,
+2, 3, 4, 5, 0, 1, 2, 3, 4, 5, 0, 1, 2, 3, 4, 5, 0, 1, 2, 3, 4, 5,
+0, 1, 2, 3,
};
-static const uint8_t div6[63 + 2 * 6 + 1] = {
+static const uint8_t div6[63 + 8 * 6 + 1] = {
0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 2, 3, 3, 3,
3, 3, 3, 4, 4, 4, 4, 4, 4, 5, 5, 5, 5, 5, 5, 6, 6, 6, 6, 6, 6,
7, 7, 7, 7, 7, 7, 8, 8, 8, 8, 8, 8, 9, 9, 9, 9, 9, 9, 10, 10, 10, 10,
-10, 10, 11, 11, 11, 11, 11, 11, 12, 12, 12, 12
+10, 10, 11, 11, 11, 11, 11, 11, 12, 12, 12, 12, 12, 12, 13, 13, 13, 13,
+13, 13, 14, 14, 14, 14, 14, 14, 15, 15, 15, 15, 15, 15, 16, 16, 16, 16,
+16, 16, 17, 17, 17, 17, 17, 17, 18, 18, 18, 18,
};
const static int level_scale[2][6] = {
--
2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Avoid overflow in coeff scale intermediate
Make intermediate result 64-bits to avoid an overflow before the right
shift.
Signed-off-by: Frank Plowman
---
libavcodec/vvc/intra.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/vvc/intra.c b/libavcodec/vvc/intra.c
index e515fb9710..5ac7d02c80 100644
--- a/libavcodec/vvc/intra.c
+++ b/libavcodec/vvc/intra.c
@@ -416,7 +416,7 @@ static const uint8_t* derive_scale_m(const VVCLocalContext
*lc, const TransformB
static av_always_inline int scale_coeff(const TransformBlock *tb, int coeff,
const int scale, const int scale_m, const int log2_transform_range)
{
-coeff = (coeff * scale * scale_m + tb->bd_offset) >> tb->bd_shift;
+coeff = ((int64_t) coeff * scale * scale_m + tb->bd_offset) >>
tb->bd_shift;
coeff = av_clip_intp2(coeff, log2_transform_range);
return coeff;
}
--
2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Fix buffer overread in CABAC
The size variable here is taken as gospel for the bounds of the input
buffer in later logic. Clamp it to ensure that the returned region
does not extend past that allocated in the underlying GetBitContext,
even in the case entry point offsets are signalled in the bitstream.
Also assert this for good measure.
Signed-off-by: Frank Plowman
---
libavcodec/vvc/dec.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c
index 27ffbb741d..a4fc40b40a 100644
--- a/libavcodec/vvc/dec.c
+++ b/libavcodec/vvc/dec.c
@@ -497,9 +497,11 @@ static void ep_init_cabac_decoder(SliceContext *sc, const
int index,
skipped++;
}
size = end - start;
+size = av_clip(size, 0, get_bits_left(gb) / 8);
} else {
size = get_bits_left(gb) / 8;
}
+av_assert0(gb->buffer + get_bits_count(gb) / 8 + size <= gb->buffer_end);
ff_init_cabac_decoder (&ep->cc, gb->buffer + get_bits_count(gb) / 8, size);
skip_bits(gb, size * 8);
}
--
2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc_parser: Fix integer overflow calculating framerate
num_units_in_tick and time_scale are both 32-bit unsigned integers.
Storing them as ints was causing overflows.
Signed-off-by: Frank Plowman
---
libavcodec/vvc_parser.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavcodec/vvc_parser.c b/libavcodec/vvc_parser.c
index a6a5be27ae..e3501fa139 100644
--- a/libavcodec/vvc_parser.c
+++ b/libavcodec/vvc_parser.c
@@ -191,8 +191,8 @@ static void set_parser_ctx(AVCodecParserContext *s,
AVCodecContext *avctx,
if (sps->sps_ptl_dpb_hrd_params_present_flag &&
sps->sps_timing_hrd_params_present_flag) {
-int num = sps->sps_general_timing_hrd_parameters.num_units_in_tick;
-int den = sps->sps_general_timing_hrd_parameters.time_scale;
+uint32_t num =
sps->sps_general_timing_hrd_parameters.num_units_in_tick;
+uint32_t den = sps->sps_general_timing_hrd_parameters.time_scale;
if (num != 0 && den != 0)
av_reduce(&avctx->framerate.den, &avctx->framerate.num,
--
2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Fix left shifts of negative values
All these variables lie in the range [-12..12]
Signed-off-by: Frank Plowman
---
libavcodec/vvc/ps.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c
index 3c71c34bae..1b23675c98 100644
--- a/libavcodec/vvc/ps.c
+++ b/libavcodec/vvc/ps.c
@@ -1196,12 +1196,12 @@ static void sh_deblock_offsets(VVCSH *sh)
const H266RawSliceHeader *r = sh->r;
if (!r->sh_deblocking_filter_disabled_flag) {
-sh->deblock.beta_offset[LUMA] = r->sh_luma_beta_offset_div2 << 1;
-sh->deblock.tc_offset[LUMA] = r->sh_luma_tc_offset_div2 << 1;
-sh->deblock.beta_offset[CB] = r->sh_cb_beta_offset_div2 << 1;
-sh->deblock.tc_offset[CB] = r->sh_cb_tc_offset_div2 << 1;
-sh->deblock.beta_offset[CR] = r->sh_cr_beta_offset_div2 << 1;
-sh->deblock.tc_offset[CR] = r->sh_cr_tc_offset_div2 << 1;
+sh->deblock.beta_offset[LUMA] = r->sh_luma_beta_offset_div2 * 2;
+sh->deblock.tc_offset[LUMA] = r->sh_luma_tc_offset_div2 * 2;
+sh->deblock.beta_offset[CB] = r->sh_cb_beta_offset_div2 * 2;
+sh->deblock.tc_offset[CB] = r->sh_cb_tc_offset_div2 * 2;
+sh->deblock.beta_offset[CR] = r->sh_cr_beta_offset_div2 * 2;
+sh->deblock.tc_offset[CR] = r->sh_cr_tc_offset_div2 * 2;
}
}
--
2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/vvc/ps: reset sps_id_used on PS uninit
On 08/04/2024 15:12, Nuo Mi wrote: > On Mon, Apr 8, 2024 at 4:37 PM Frank Plowman wrote: > >> On 07/04/2024 15:48, James Almer wrote: >>> On 4/7/2024 10:38 AM, Nuo Mi wrote: >>>> On Sun, Apr 7, 2024 at 11:05 AM James Almer wrote: >>>> >>>>> Signed-off-by: James Almer >>>>> --- >>>>> libavcodec/vvc/ps.c | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c >>>>> index 3c71c34bae..83ee75fb62 100644 >>>>> --- a/libavcodec/vvc/ps.c >>>>> +++ b/libavcodec/vvc/ps.c >>>>> @@ -912,6 +912,7 @@ void ff_vvc_ps_uninit(VVCParamSets *ps) >>>>> ff_refstruct_unref(&ps->sps_list[i]); >>>>> for (int i = 0; i < FF_ARRAY_ELEMS(ps->pps_list); i++) >>>>> ff_refstruct_unref(&ps->pps_list[i]); >>>>> +ps->sps_id_used = 0; >>>>> >>>> Hi James, >>>> thank you for the patch. >>>> >>>> Is this really necessary? >>>> vvc_ps_uninit will be called by vvc_decode_free, >>>> We are not supposed to use any member of VVCParamSets after >>>> vvc_decode_free. >>> >>> My bad, i thought it was also called on every flush() call. >>> >>> Something like the following: >>> >>>> diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c >>>> index eb447604fe..463536512e 100644 >>>> --- a/libavcodec/vvc/dec.c >>>> +++ b/libavcodec/vvc/dec.c >>>> @@ -963,6 +963,8 @@ static av_cold void >>>> vvc_decode_flush(AVCodecContext *avctx) >>>> ff_vvc_flush_dpb(last); >>>> } >>>> >>>> +s->ps->sps_id_used = 0; >>>> + >>>> s->eos = 1; >>>> } >>> >>> Should be done on FFCodec.flush() (like when seeking) as the previous >>> state is no longer valid, right? >> >> Yes I agree, I think this is needed. Cases where the random access >> point has no leading pictures should be covered by the existing logic as >> these always fall at the start of a CVS, but I think this is needed to >> cover the case in which there are leading pictures. >> > This patch isn't necessary. > Leading pictures won't carry SPS. > IDR, CRA, and GDR will carry SPS, but they will also start a new CVS, which > will covered by the current logic. > I may be misunderstanding the spec, NoOutputBeforeRecoveryFlag is set for pictures which have no leading pictures, no? In any case take, for instance, a CRA picture. In most cases, CRA pictures have NoOutputBeforeRecoveryFlag=0, therefore are not CLVSS pictures and sps_id_used is not reset by the existing logic. Do we not need to reset sps_id_used when seeking to a CRA then? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/vvc/ps: reset sps_id_used on PS uninit
On 07/04/2024 15:48, James Almer wrote: > On 4/7/2024 10:38 AM, Nuo Mi wrote: >> On Sun, Apr 7, 2024 at 11:05 AM James Almer wrote: >> >>> Signed-off-by: James Almer >>> --- >>> libavcodec/vvc/ps.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c >>> index 3c71c34bae..83ee75fb62 100644 >>> --- a/libavcodec/vvc/ps.c >>> +++ b/libavcodec/vvc/ps.c >>> @@ -912,6 +912,7 @@ void ff_vvc_ps_uninit(VVCParamSets *ps) >>> ff_refstruct_unref(&ps->sps_list[i]); >>> for (int i = 0; i < FF_ARRAY_ELEMS(ps->pps_list); i++) >>> ff_refstruct_unref(&ps->pps_list[i]); >>> + ps->sps_id_used = 0; >>> >> Hi James, >> thank you for the patch. >> >> Is this really necessary? >> vvc_ps_uninit will be called by vvc_decode_free, >> We are not supposed to use any member of VVCParamSets after >> vvc_decode_free. > > My bad, i thought it was also called on every flush() call. > > Something like the following: > >> diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c >> index eb447604fe..463536512e 100644 >> --- a/libavcodec/vvc/dec.c >> +++ b/libavcodec/vvc/dec.c >> @@ -963,6 +963,8 @@ static av_cold void >> vvc_decode_flush(AVCodecContext *avctx) >> ff_vvc_flush_dpb(last); >> } >> >> + s->ps->sps_id_used = 0; >> + >> s->eos = 1; >> } > > Should be done on FFCodec.flush() (like when seeking) as the previous > state is no longer valid, right? Yes I agree, I think this is needed. Cases where the random access point has no leading pictures should be covered by the existing logic as these always fall at the start of a CVS, but I think this is needed to cover the case in which there are leading pictures. Thanks, -- Frank ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2] lavc/vvc: Error if SPS ID is duplicated within CVS
Key line from the spec is:
"All SPS NAL units with a particular value of sps_seq_parameter_set_id
in a CVS shall have the same content."
Prior to this patch, the VVC decoder's behaviour on encountering a
duplicated SPS ID (within the entire bitstream, not restricted to
a CVS) was simply to replace the entry in the SPS lookup table with the
new data. Illegal bitstreams with multiple SPSs in the same CVS sharing
an ID but differing elsewhere could cause all manner of issues.
The patch tracks which SPS IDs have been used in the given CVS using the
new sps_id_used field of VVCParamSets. If it encounters an SPS with an
ID already in use and whose content differs from the previous SPS, it
throws an AVERROR_INVALIDDATA.
Signed-off-by: Frank Plowman
---
Changes since v1:
* Track which SPS IDs are in use using a bit field rather than a bool
array.
* Move sps_id_used to the end of VVCParamSets.
* Style: remove blank line & add braces for clarity.
libavcodec/vvc/vvc_ps.c | 28
libavcodec/vvc/vvc_ps.h | 2 ++
2 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/libavcodec/vvc/vvc_ps.c b/libavcodec/vvc/vvc_ps.c
index 301fa16400..7e967180d2 100644
--- a/libavcodec/vvc/vvc_ps.c
+++ b/libavcodec/vvc/vvc_ps.c
@@ -219,14 +219,22 @@ fail:
return NULL;
}
-static int decode_sps(VVCParamSets *ps, const H266RawSPS *rsps, void *log_ctx)
+static int decode_sps(VVCParamSets *ps, const H266RawSPS *rsps, void *log_ctx,
int is_clvss)
{
const int sps_id= rsps->sps_seq_parameter_set_id;
const VVCSPS *old_sps = ps->sps_list[sps_id];
const VVCSPS *sps;
-if (old_sps && old_sps->r == rsps)
-return 0;
+if (is_clvss) {
+ps->sps_id_used = 0;
+}
+
+if (old_sps) {
+if (old_sps->r == rsps || !memcmp(old_sps->r, rsps,
sizeof(*old_sps->r)))
+return 0;
+else if (ps->sps_id_used & (1 << sps_id))
+return AVERROR_INVALIDDATA;
+}
sps = sps_alloc(rsps, log_ctx);
if (!sps)
@@ -234,6 +242,7 @@ static int decode_sps(VVCParamSets *ps, const H266RawSPS
*rsps, void *log_ctx)
ff_refstruct_unref(&ps->sps_list[sps_id]);
ps->sps_list[sps_id] = sps;
+ps->sps_id_used |= (1 << sps_id);
return 0;
}
@@ -610,7 +619,7 @@ static int decode_pps(VVCParamSets *ps, const H266RawPPS
*rpps)
return ret;
}
-static int decode_ps(VVCParamSets *ps, const CodedBitstreamH266Context *h266,
void *log_ctx)
+static int decode_ps(VVCParamSets *ps, const CodedBitstreamH266Context *h266,
void *log_ctx, int is_clvss)
{
const H266RawPictureHeader *ph = h266->ph;
const H266RawPPS *rpps;
@@ -628,7 +637,7 @@ static int decode_ps(VVCParamSets *ps, const
CodedBitstreamH266Context *h266, vo
if (!rsps)
return AVERROR_INVALIDDATA;
-ret = decode_sps(ps, rsps, log_ctx);
+ret = decode_sps(ps, rsps, log_ctx, is_clvss);
if (ret < 0)
return ret;
@@ -867,13 +876,16 @@ int ff_vvc_decode_frame_ps(VVCFrameParamSets *fps, struct
VVCContext *s)
int ret = 0;
VVCParamSets *ps= &s->ps;
const CodedBitstreamH266Context *h266 = s->cbc->priv_data;
+int is_clvss;
+
+decode_recovery_flag(s);
+is_clvss = IS_CLVSS(s);
-ret = decode_ps(ps, h266, s->avctx);
+ret = decode_ps(ps, h266, s->avctx, is_clvss);
if (ret < 0)
return ret;
-decode_recovery_flag(s);
-ret = decode_frame_ps(fps, ps, h266, s->poc_tid0, IS_CLVSS(s));
+ret = decode_frame_ps(fps, ps, h266, s->poc_tid0, is_clvss);
decode_recovery_poc(s, &fps->ph);
return ret;
}
diff --git a/libavcodec/vvc/vvc_ps.h b/libavcodec/vvc/vvc_ps.h
index f60d8b81c6..710673ebed 100644
--- a/libavcodec/vvc/vvc_ps.h
+++ b/libavcodec/vvc/vvc_ps.h
@@ -214,6 +214,8 @@ typedef struct VVCParamSets {
const VVCALF*alf_list[VVC_MAX_ALF_COUNT]; ///< RefStruct
reference
const H266RawAPS*lmcs_list[VVC_MAX_LMCS_COUNT]; ///< RefStruct
reference
const VVCScalingList*scaling_list[VVC_MAX_SL_COUNT];///< RefStruct
reference
+// Bit field of SPS IDs used in the current CVS
+ uint16_t sps_id_used;
} VVCParamSets;
typedef struct VVCFrameParamSets {
--
2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Error if SPS ID is duplicated within CVS
Key line from the spec is:
"All SPS NAL units with a particular value of sps_seq_parameter_set_id
in a CVS shall have the same content."
Prior to this patch, the VVC decoder's behaviour on encountering a
duplicated SPS ID (within the entire bitstream, not restricted to
a CVS) was simply to replace the entry in the SPS lookup table with the
new data. Illegal bitstreams with multiple SPSs in the same CVS sharing
an ID but differing elsewhere could cause all manner of issues.
The patch tracks which SPS IDs have been used in the given CVS using the
new sps_id_used field of VVCParamSets. If it encounters an SPS with an
ID already in use and whose content differs from the previous SPS, it
throws an AVERROR_INVALIDDATA.
---
libavcodec/vvc/vvc_ps.c | 29 +
libavcodec/vvc/vvc_ps.h | 1 +
2 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/libavcodec/vvc/vvc_ps.c b/libavcodec/vvc/vvc_ps.c
index 301fa16400..7a8daaa6ad 100644
--- a/libavcodec/vvc/vvc_ps.c
+++ b/libavcodec/vvc/vvc_ps.c
@@ -219,14 +219,23 @@ fail:
return NULL;
}
-static int decode_sps(VVCParamSets *ps, const H266RawSPS *rsps, void *log_ctx)
+static int decode_sps(VVCParamSets *ps, const H266RawSPS *rsps, void *log_ctx,
int is_clvss)
{
const int sps_id= rsps->sps_seq_parameter_set_id;
const VVCSPS *old_sps = ps->sps_list[sps_id];
const VVCSPS *sps;
-if (old_sps && old_sps->r == rsps)
-return 0;
+if (is_clvss) {
+for (int sps_id = 0; sps_id < VVC_MAX_SPS_COUNT; ++sps_id)
+ps->sps_id_used[sps_id] = 0;
+}
+
+if (old_sps)
+if (old_sps->r == rsps || !memcmp(old_sps->r, rsps,
sizeof(*old_sps->r)))
+return 0;
+else if (ps->sps_id_used[sps_id])
+return AVERROR_INVALIDDATA;
+
sps = sps_alloc(rsps, log_ctx);
if (!sps)
@@ -234,6 +243,7 @@ static int decode_sps(VVCParamSets *ps, const H266RawSPS
*rsps, void *log_ctx)
ff_refstruct_unref(&ps->sps_list[sps_id]);
ps->sps_list[sps_id] = sps;
+ps->sps_id_used[sps_id] = 1;
return 0;
}
@@ -610,7 +620,7 @@ static int decode_pps(VVCParamSets *ps, const H266RawPPS
*rpps)
return ret;
}
-static int decode_ps(VVCParamSets *ps, const CodedBitstreamH266Context *h266,
void *log_ctx)
+static int decode_ps(VVCParamSets *ps, const CodedBitstreamH266Context *h266,
void *log_ctx, int is_clvss)
{
const H266RawPictureHeader *ph = h266->ph;
const H266RawPPS *rpps;
@@ -628,7 +638,7 @@ static int decode_ps(VVCParamSets *ps, const
CodedBitstreamH266Context *h266, vo
if (!rsps)
return AVERROR_INVALIDDATA;
-ret = decode_sps(ps, rsps, log_ctx);
+ret = decode_sps(ps, rsps, log_ctx, is_clvss);
if (ret < 0)
return ret;
@@ -867,13 +877,16 @@ int ff_vvc_decode_frame_ps(VVCFrameParamSets *fps, struct
VVCContext *s)
int ret = 0;
VVCParamSets *ps= &s->ps;
const CodedBitstreamH266Context *h266 = s->cbc->priv_data;
+int is_clvss;
-ret = decode_ps(ps, h266, s->avctx);
+decode_recovery_flag(s);
+is_clvss = IS_CLVSS(s);
+
+ret = decode_ps(ps, h266, s->avctx, is_clvss);
if (ret < 0)
return ret;
-decode_recovery_flag(s);
-ret = decode_frame_ps(fps, ps, h266, s->poc_tid0, IS_CLVSS(s));
+ret = decode_frame_ps(fps, ps, h266, s->poc_tid0, is_clvss);
decode_recovery_poc(s, &fps->ph);
return ret;
}
diff --git a/libavcodec/vvc/vvc_ps.h b/libavcodec/vvc/vvc_ps.h
index f60d8b81c6..b293043de8 100644
--- a/libavcodec/vvc/vvc_ps.h
+++ b/libavcodec/vvc/vvc_ps.h
@@ -210,6 +210,7 @@ typedef struct VVCLMCS {
typedef struct VVCParamSets {
const VVCSPS*sps_list[VVC_MAX_SPS_COUNT]; ///< RefStruct
reference
+ intsps_id_used[VVC_MAX_SPS_COUNT];
const VVCPPS*pps_list[VVC_MAX_PPS_COUNT]; ///< RefStruct
reference
const VVCALF*alf_list[VVC_MAX_ALF_COUNT]; ///< RefStruct
reference
const H266RawAPS*lmcs_list[VVC_MAX_LMCS_COUNT]; ///< RefStruct
reference
--
2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/vvc: Only read split_cu_flag if a split is allowed
On 02/04/2024 22:48, Frank Plowman wrote:
Add a check to ensure some split is possible before reading the
split_cu_flag. This is present in the spec, in VVCv3 section 7.3.11.4.
Its omission could lead to infinite loops and ultimately crashing due to
stack overflow.
---
libavcodec/vvc/vvc_ctu.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/libavcodec/vvc/vvc_ctu.c b/libavcodec/vvc/vvc_ctu.c
index 8ba12c8d9f..32d8bc8f5c 100644
--- a/libavcodec/vvc/vvc_ctu.c
+++ b/libavcodec/vvc/vvc_ctu.c
@@ -2095,6 +2095,7 @@ static int hls_coding_tree(VVCLocalContext *lc,
const int ch_type = tree_type_curr == DUAL_TREE_CHROMA;
int ret;
VVCAllowedSplit allowed;
+int split_cu_flag;
if (pps->r->pps_cu_qp_delta_enabled_flag && qg_on_y && cb_sub_div <= sh->cu_qp_delta_subdiv) {
lc->parse.is_cu_qp_delta_coded = 0;
@@ -2109,7 +2110,11 @@ static int hls_coding_tree(VVCLocalContext *lc,
can_split(lc, x0, y0, cb_width, cb_height, mtt_depth, depth_offset, part_idx,
last_split_mode, tree_type_curr, mode_type_curr, &allowed);
-if (ff_vvc_split_cu_flag(lc, x0, y0, cb_width, cb_height, ch_type,
&allowed)) {
+if (allowed.btv || allowed.bth || allowed.ttv || allowed.tth || allowed.qt)
+split_cu_flag = ff_vvc_split_cu_flag(lc, x0, y0, cb_width, cb_height,
ch_type, &allowed);
+else
+split_cu_flag = 0;
+if (split_cu_flag) {
VVCSplitMode split = ff_vvc_split_mode(lc, x0, y0, cb_width,
cb_height, cqt_depth, mtt_depth, ch_type, &allowed);
VVCModeType mode_type = mode_type_decode(lc, x0, y0, cb_width,
cb_height, split, ch_type, mode_type_curr);
Retracting this patch as I missed that this logic is in fact
implemented, just elsewhere. There is still a bug here, but it seems
the condition to trigger it is more complex that I thought. Should have
an alternative patch soon.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Only read split_cu_flag if a split is allowed
Add a check to ensure some split is possible before reading the
split_cu_flag. This is present in the spec, in VVCv3 section 7.3.11.4.
Its omission could lead to infinite loops and ultimately crashing due to
stack overflow.
---
libavcodec/vvc/vvc_ctu.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/libavcodec/vvc/vvc_ctu.c b/libavcodec/vvc/vvc_ctu.c
index 8ba12c8d9f..32d8bc8f5c 100644
--- a/libavcodec/vvc/vvc_ctu.c
+++ b/libavcodec/vvc/vvc_ctu.c
@@ -2095,6 +2095,7 @@ static int hls_coding_tree(VVCLocalContext *lc,
const int ch_type = tree_type_curr == DUAL_TREE_CHROMA;
int ret;
VVCAllowedSplit allowed;
+int split_cu_flag;
if (pps->r->pps_cu_qp_delta_enabled_flag && qg_on_y && cb_sub_div <=
sh->cu_qp_delta_subdiv) {
lc->parse.is_cu_qp_delta_coded = 0;
@@ -2109,7 +2110,11 @@ static int hls_coding_tree(VVCLocalContext *lc,
can_split(lc, x0, y0, cb_width, cb_height, mtt_depth, depth_offset,
part_idx,
last_split_mode, tree_type_curr, mode_type_curr, &allowed);
-if (ff_vvc_split_cu_flag(lc, x0, y0, cb_width, cb_height, ch_type,
&allowed)) {
+if (allowed.btv || allowed.bth || allowed.ttv || allowed.tth || allowed.qt)
+split_cu_flag = ff_vvc_split_cu_flag(lc, x0, y0, cb_width, cb_height,
ch_type, &allowed);
+else
+split_cu_flag = 0;
+if (split_cu_flag) {
VVCSplitMode split = ff_vvc_split_mode(lc, x0, y0, cb_width,
cb_height, cqt_depth, mtt_depth, ch_type, &allowed);
VVCModeType mode_type = mode_type_decode(lc, x0, y0, cb_width,
cb_height, split, ch_type, mode_type_curr);
--
2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Fix check whether QG is in first tile col
The second part of this condition is intended to check whether the
current quantisation group is in the first CTU column of the current
tile. The issue is that ctb_to_col_bd gives the x-ordinate of the first
column of the current tile *in CTUs*, while xQg gives the x-ordinate of
the quantisation group *in samples*. Rectify this by shifting xQg by
ctb_log2_size to get xQg in CTUs before comparing.
Fixes FFVVC issues #201 and #203.
---
libavcodec/vvc/vvc_ctu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/vvc/vvc_ctu.c b/libavcodec/vvc/vvc_ctu.c
index 519bd1ba76..8ba12c8d9f 100644
--- a/libavcodec/vvc/vvc_ctu.c
+++ b/libavcodec/vvc/vvc_ctu.c
@@ -96,7 +96,7 @@ static int get_qp_y_pred(const VVCLocalContext *lc)
if (lc->na.cand_up) {
const int first_qg_in_ctu = !(xQg & ctb_size_mask) && !(yQg &
ctb_size_mask);
const int qPy_up = fc->tab.qp[LUMA][x_cb + (y_cb - 1) *
min_cb_width];
-if (first_qg_in_ctu && pps->ctb_to_col_bd[xQg >> ctb_log2_size] == xQg)
+if (first_qg_in_ctu && pps->ctb_to_col_bd[xQg >> ctb_log2_size] == xQg
>> ctb_log2_size)
return qPy_up;
}
--
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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 00/14] avcodec/vvcdec: support subpicture
On 18/03/2024 14:16, Nuo Mi wrote: > see introductions here: https://dashif.org/docs/VVC%20HLS%20overview%20.pdf > > Frank Plowman (1): > avcodec/vvcdec: support rectangular single-slice subpics > > Nuo Mi (13): > avcodec/vvcdec: NoBackwardPredFlag, only check active pictures > avcodec/cbs_h266: fix sh_collocated_from_l0_flag and > sh_collocated_ref_idx infer > avcodec/vvcdec: derive subpic postion for PPS > avcodec/vvcdec: ff_vvc_decode_neighbour, support subpicture > avcodec/vvcdec: misc, rename x_ctb, y_ctb, ctu_x, ctu_y to rx, ry to > avoid misleading > avcodec/vvcdec: refact out deblock_is_boundary > avcodec/vvcdec: deblock, support subpicture > avcodec/vvcdec: refact, movie the lc->sc assignment to task_run_stage > to simplify the code > avcodec/vvcdec: sao, refact out tile_edge arrays > avcodec/vvcdec: sao, support subpicture > avcodec/vvcdec: alf, support subpicture > avcodec/vvcdec: mvs, support subpicture > avcodec/vvcdec: inter prediction, support subpicture > > libavcodec/cbs_h266_syntax_template.c | 37 ++-- > libavcodec/vvc/vvc_ctu.c | 12 +- > libavcodec/vvc/vvc_ctu.h | 6 +- > libavcodec/vvc/vvc_filter.c | 233 +- > libavcodec/vvc/vvc_filter.h | 6 +- > libavcodec/vvc/vvc_inter.c| 79 ++--- > libavcodec/vvc/vvc_mvs.c | 35 ++-- > libavcodec/vvc/vvc_ps.c | 150 ++--- > libavcodec/vvc/vvc_ps.h | 4 + > libavcodec/vvc/vvc_thread.c | 66 ++-- > 10 files changed, 384 insertions(+), 244 deletions(-) > > -- > 2.25.1 Thank you. When compiled without a sanitizer, all subpicture conformance bitstreams decode correctly, however when compiled with UBSAN/ASAN this set causes a lot of errors for me. With UBSAN, I get errors similar to: libavcodec/vvc/vvc_ctu.c:1553:13: runtime error: member access within misaligned address 0xb52dbd6c for type 'av_alias64', which requires 8 byte alignment 0xb52dbd6c: note: pointer points here f0 b3 9a 99 ff ff 00 00 00 be 2d b5 ff ff 00 00 c4 a6 fb d5 aa aa 00 00 c0 f8 2d b5 ff ff 00 00 on the following sequences: * SUBPIC_B_HUAWEI * SUBPIC_C_ERICSSON * SUBPIC_E_MediaTek * LMCS_B_Dolby * CodingTools_E_Tencent With ASAN, I get errors similar to: = ==67289==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6030067a at pc 0xd00d21a0 bp 0xfba438f0 sp 0xfba43908 READ of size 2 at 0x6030067a thread T0 #0 0xd00d219c in subpic_tiles /home/frank/FFmpeg/libavcodec/vvc/vvc_ps.c:374:12 #1 0xd00d1048 in pps_subpic_slice /home/frank/FFmpeg/libavcodec/vvc/vvc_ps.c:406:5 #2 0xd00cf2cc in pps_single_slice_per_subpic /home/frank/FFmpeg/libavcodec/vvc/vvc_ps.c:419:13 #3 0xd00ce8e0 in pps_rect_slice /home/frank/FFmpeg/libavcodec/vvc/vvc_ps.c:464:9 #4 0xd00cd4f8 in pps_slice_map /home/frank/FFmpeg/libavcodec/vvc/vvc_ps.c:499:9 #5 0xd00caf64 in pps_derive /home/frank/FFmpeg/libavcodec/vvc/vvc_ps.c:543:11 #6 0xd00cac00 in pps_alloc /home/frank/FFmpeg/libavcodec/vvc/vvc_ps.c:576:11 #7 0xd00c64f8 in decode_pps /home/frank/FFmpeg/libavcodec/vvc/vvc_ps.c:598:11 #8 0xd00c in decode_ps /home/frank/FFmpeg/libavcodec/vvc/vvc_ps.c:630:11 #9 0xd00c3e70 in ff_vvc_decode_frame_ps /home/frank/FFmpeg/libavcodec/vvc/vvc_ps.c:866:11 ... 0x6030067a is located 0 bytes after 10-byte region [0x60300670,0x6030067a) on the following sequences: * SUBPIC_C_ERICSSON * SUBPIC_D_ERICSSON Note SUBPIC_D_ERICSSON has the ASAN error but no UBSAN error. Cheers, Frank ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] vvcdec: Mark as experimental
On 15/03/2024 10:22, Kieran Kunhya wrote: > On Thu, 14 Mar 2024, 22:54 Michael Niedermayer, > wrote: > >> On Wed, Feb 07, 2024 at 10:55:18PM +, Kieran Kunhya wrote: >>> On Wed, 7 Feb 2024 at 22:06, Paul B Mahol wrote: >>> On Wed, Feb 7, 2024 at 10:13 PM Kieran Kunhya wrote: > $subj > > As discussed at FOSDEM. > Author of this patch above is forced to FUZZ this decoder until experimental flag is removed. >>> >>> Sure, I will set some fuzzers up over the weekend. >> >> over a month later ... >> has this been done ? >> >> >> thx >> >> [...] >> > > Frank said there was no need as he was doing it himself. > > I do not appreciate your passive aggressive tone. > > Kieran > I have been fuzzing since the end of January. Various patches have made the decoder much more robust than it was then. Still getting a crash on occasion, but most are now false positives due to assertion failures rather than segmentation faults or other potential security issues. That being said, the hardware I am using is extremely modest compared to oss-fuzz's. -- Frank ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/vvc: AVERROR_PATCHWELCOME for subpictures
On 12/03/2024 03:33, Wang, Fei W wrote: > On Mon, 2024-03-11 at 21:57 -0300, James Almer wrote: >> On 3/11/2024 9:49 PM, Michael Niedermayer wrote: >>> On Mon, Mar 11, 2024 at 06:53:31PM +0000, Frank Plowman wrote: >>>> VVC's subpictures feature is not yet implemented in the native >>>> decoder. >>>> Throw an AVERROR_PATCHWELCOME when trying to decode a bitstream >>>> using >>>> the feature. Fixes crashes when trying to decode bitstreams >>>> which >>>> use the feature. >>>> >>>> Signed-off-by: Frank Plowman >>>> --- >>>> libavcodec/vvc/vvc_ps.c | 15 +++ >>>> 1 file changed, 15 insertions(+) >>> >>> This breaks fate-vvc-conformance-SUBPIC_A_3 >>> >>> make fate-vvc-conformance-SUBPIC_A_3 >>> TESTvvc-conformance-SUBPIC_A_3 >>> --- ./tests/ref/fate/vvc-conformance-SUBPIC_A_3 2024-03-05 >>> 02:37:36.235300141 +0100 >>> +++ tests/data/fate/vvc-conformance-SUBPIC_A_3 2024-03-12 >>> 01:47:27.301593567 +0100 >>> @@ -1,9 +0,0 @@ >>> -#tb 0: 1/25 >>> -#media_type 0: video >>> -#codec_id 0: rawvideo >>> -#dimensions 0: 1920x1080 >>> -#sar 0: 0/1 >>> -0, 0, 0,1, 6220800, 0xa419cfb6 >>> -0, 1, 1,1, 6220800, 0xa419cfb6 >>> -0, 2, 2,1, 6220800, 0xa419cfb6 >>> -0, 3, 3,1, 6220800, 0xa419cfb6 >>> Test vvc-conformance-SUBPIC_A_3 failed. Look at >>> tests/data/fate/vvc-conformance-SUBPIC_A_3.err for details. >>> tests/Makefile:318: recipe for target 'fate-vvc-conformance- >>> SUBPIC_A_3' failed >>> make: *** [fate-vvc-conformance-SUBPIC_A_3] Error 69 >>> >>> thx >> >> The sample appears to decode fine and doesn't crash, although all >> four >> frames are exactly the same (I don't know is that's intended). > > The result is correct. Assume native decode can handle subpic, at least > a part of the feature. > >> Maybe the crashes can be fixed in some other form? And abort only if >> FF_COMPLIANCE_STRICT is requested. > > Previous I made a patch to fix the crash in setup ps, but still get > error in decoding slice, it's better to return AVERROR_PATCHWELCOME > only in case of pps_single_slice_per_subpic_flag. > > https://github.com/intel-media-ci/ffmpeg/pull/723 > > Thanks > Fei Hi, Thanks all for your reviews. Yes, the feature is most problematic if pps_single_slice_per_subpic_flag is 1. This is because there is some unimplemented logic in the parameter set parser in this case which leads to some out-of-bounds accesses. Subpictures will also fail to decode bitexact if sps_independent_subpics is 1, although in practice the distortion this introduces is fairly subtle. I think we are able to decode other subpicture bitstreams bitexact. SUBPIC_A_HUAWEI_3 falls into this last category. I posted a patch, like yours Fei, which errors only the most problematic pps_single_slice_per_subpic_flag bitstreams some time ago: https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240201140055.63805-1-p...@frankplowman.com/ and the feedback I received there was to make this patch. I have also implemented the logic needed for pps_single_slice_per_subpic_flag here: https://github.com/ffvvc/FFmpeg/pull/191. I would be happy to make a v2 of the pps_single_slice_per_subpic_flag patch which moves where the error is being returned, or to put the GitHub PR on the ML if people would rather one of those two alternatives. Thanks again, -- Frank ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: AVERROR_PATCHWELCOME for subpictures
VVC's subpictures feature is not yet implemented in the native decoder.
Throw an AVERROR_PATCHWELCOME when trying to decode a bitstream using
the feature. Fixes crashes when trying to decode bitstreams which
use the feature.
Signed-off-by: Frank Plowman
---
libavcodec/vvc/vvc_ps.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/libavcodec/vvc/vvc_ps.c b/libavcodec/vvc/vvc_ps.c
index e6e46d2039..62515a6343 100644
--- a/libavcodec/vvc/vvc_ps.c
+++ b/libavcodec/vvc/vvc_ps.c
@@ -72,6 +72,18 @@ static int sps_map_pixel_format(VVCSPS *sps, void *log_ctx)
return 0;
}
+static int sps_subpic_info(VVCSPS *sps, void *log_ctx)
+{
+const H266RawSPS *r = sps->r;
+
+if (r->sps_num_subpics_minus1 > 0) {
+avpriv_report_missing_feature(log_ctx, "Subpictures");
+return AVERROR_PATCHWELCOME;
+}
+
+return 0;
+}
+
static int sps_bit_depth(VVCSPS *sps, void *log_ctx)
{
const H266RawSPS *r = sps->r;
@@ -177,6 +189,9 @@ static int sps_derive(VVCSPS *sps, void *log_ctx)
int ret;
const H266RawSPS *r = sps->r;
+ret = sps_subpic_info(sps, log_ctx);
+if (ret < 0)
+return ret;
ret = sps_bit_depth(sps, log_ctx);
if (ret < 0)
return ret;
--
2.44.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] FFmpeg 7.0 blocking issues
On 08/03/2024 14:04, James Almer wrote: > On 3/8/2024 11:02 AM, Kieran Kunhya wrote: >> On Fri, 8 Mar 2024 at 14:00, James Almer wrote: >> >>> On 3/3/2024 4:35 AM, Jean-Baptiste Kempf wrote: n Sat, 2 Mar 2024, at 23:55, Michael Niedermayer wrote: > On Tue, Jan 23, 2024 at 08:22:41PM +0100, Michael Niedermayer wrote: >> Hi all >> >> As it was a little difficult for me to not loose track of what is >> blocking a release. I suggest that for all release blocking issues >> open a ticket and set Blocking to 7.0 >> that way this: >> https://trac.ffmpeg.org/query?blocking=~7.0 >> >> or for the ones not closed: >> >>> https://trac.ffmpeg.org/query?status=new&status=open&status=reopened&blocking=~7.0 >> >> will list all blocking issues >> >> Ive added one, for testing that, i intend to add more if i see >>> something >> >> What is blocking? (IMHO) >> * regressions (unless its non possible to fix before release) >> * crashes >> * security issues >> * data loss >> * privacy issues >> * anything the commuity agrees should be in the release > > We still have 3 blocking issues on trac > > do people want me to wait or ignore them and branch ? I think you can branch soon. However, those 3 bugs are quite important, tbh. >>> >>> The bump and the AVOption changes were already applied, so IMO we can >>> branch. >>> The two remaining issues should not be blocking as they can be >>> backported to 7.0 in a point release. >>> >> >> VVC experimental flag is blocking. >> >> Kieran > > Is there a patch for that? There is this: https://ffmpeg.org//pipermail/ffmpeg-devel/2024-February/321060.html (missing from patchwork for some reason), but it looks like it causes FATE to fail as is. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/vvcdec: check pred flag to fix undefined beavhiours
On 03/03/2024 03:31, Nuo Mi wrote:
> libavcodec/vvc/vvc_inter.c:823:18: runtime error: signed integer overflow:
> 1426128896 + 1426128896 cannot be represented in type 'int'
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> libavcodec/vvc/vvc_inter.c:823:18
> ---
> libavcodec/vvc/vvc_inter.c | 15 +--
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/libavcodec/vvc/vvc_inter.c b/libavcodec/vvc/vvc_inter.c
> index d5be32aa14..48c566b097 100644
> --- a/libavcodec/vvc/vvc_inter.c
> +++ b/libavcodec/vvc/vvc_inter.c
> @@ -816,13 +816,16 @@ static void derive_affine_mvc(MvField *mvc, const
> VVCFrameContext *fc, const MvF
> const int hs = fc->ps.sps->hshift[1];
> const int vs = fc->ps.sps->vshift[1];
> const MvField* mv2 = ff_vvc_get_mvf(fc, x0 + hs * sbw, y0 + vs * sbh);
> +
> *mvc = *mv;
> -mvc->mv[0].x += mv2->mv[0].x;
> -mvc->mv[0].y += mv2->mv[0].y;
> -mvc->mv[1].x += mv2->mv[1].x;
> -mvc->mv[1].y += mv2->mv[1].y;
> -ff_vvc_round_mv(mvc->mv + 0, 0, 1);
> -ff_vvc_round_mv(mvc->mv + 1, 0, 1);
> +for (int i = L0; i <= L1; i++) {
> +PredFlag mask = 1 << i;
> +if (mv2->pred_flag & mask) {
> +mvc->mv[i].x += mv2->mv[i].x;
> +mvc->mv[i].y += mv2->mv[i].y;
> +ff_vvc_round_mv(mvc->mv + i, 0, 1);
> +}
> +}
> }
>
> static void pred_affine_blk(VVCLocalContext *lc)
LGTM. I happened to have a minimal case which reproduces this handy,
and the patch removes the error.
--
Frank
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Read subpic ID when only one subpicture is present
Previously, the following syntax elements were not read in the case
sps_num_subpics_minus is 0:
* sps_subpic_id_len_minus1
* sps_subpic_id_mapping_explicitly_signalled_flag
* sps_subpic_id_mapping_present_flag
* sps_subpic_id[i]
This was causing failures to decode bitstreams, for example the DVB's
"VVC HDR UHDTV1 OpenGOP 3840x2160 50fps HLG10 PiP" V&V bitstream.
Patch fixes this by moving the reads for these syntax elements out a
scope.
Signed-off-by: Frank Plowman
---
Sorry, should probably have put this in with
53ab7ff67e7ee9e7cae5cb0449203a7951cbe029.
libavcodec/cbs_h266_syntax_template.c | 36 +--
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/libavcodec/cbs_h266_syntax_template.c
b/libavcodec/cbs_h266_syntax_template.c
index 1d4d0c796f..623417fce4 100644
--- a/libavcodec/cbs_h266_syntax_template.c
+++ b/libavcodec/cbs_h266_syntax_template.c
@@ -1213,30 +1213,30 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
RWContext *rw,
infer(sps_loop_filter_across_subpic_enabled_flag[i], 0);
}
}
-ue(sps_subpic_id_len_minus1, 0, 15);
-if ((1 << (current->sps_subpic_id_len_minus1 + 1)) <
-current->sps_num_subpics_minus1 + 1) {
-av_log(ctx->log_ctx, AV_LOG_ERROR,
- "sps_subpic_id_len_minus1(%d) is too small\n",
- current->sps_subpic_id_len_minus1);
-return AVERROR_INVALIDDATA;
-}
-flag(sps_subpic_id_mapping_explicitly_signalled_flag);
-if (current->sps_subpic_id_mapping_explicitly_signalled_flag) {
-flag(sps_subpic_id_mapping_present_flag);
-if (current->sps_subpic_id_mapping_present_flag) {
-for (i = 0; i <= current->sps_num_subpics_minus1; i++) {
-ubs(current->sps_subpic_id_len_minus1 + 1,
-sps_subpic_id[i], 1, i);
-}
-}
-}
} else {
infer(sps_subpic_ctu_top_left_x[0], 0);
infer(sps_subpic_ctu_top_left_y[0], 0);
infer(sps_subpic_width_minus1[0], tmp_width_val - 1);
infer(sps_subpic_height_minus1[0], tmp_height_val - 1);
}
+ue(sps_subpic_id_len_minus1, 0, 15);
+if ((1 << (current->sps_subpic_id_len_minus1 + 1)) <
+current->sps_num_subpics_minus1 + 1) {
+av_log(ctx->log_ctx, AV_LOG_ERROR,
+ "sps_subpic_id_len_minus1(%d) is too small\n",
+ current->sps_subpic_id_len_minus1);
+return AVERROR_INVALIDDATA;
+}
+flag(sps_subpic_id_mapping_explicitly_signalled_flag);
+if (current->sps_subpic_id_mapping_explicitly_signalled_flag) {
+flag(sps_subpic_id_mapping_present_flag);
+if (current->sps_subpic_id_mapping_present_flag) {
+for (i = 0; i <= current->sps_num_subpics_minus1; i++) {
+ubs(current->sps_subpic_id_len_minus1 + 1,
+sps_subpic_id[i], 1, i);
+}
+}
+}
} else {
infer(sps_num_subpics_minus1, 0);
infer(sps_independent_subpics_flag, 1);
--
2.43.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/vvc: Correct sps_num_subpics_minus1 minimum
On 25/02/2024 19:50, James Almer wrote:
> On 2/25/2024 2:51 PM, Frank Plowman wrote:
>> The spec says "the value of sps_num_subpics_minus1 shall be in the
>> range of 0 to MaxSlicesPerAu − 1, inclusive."
>>
>> Signed-off-by: Frank Plowman
>> ---
>> libavcodec/cbs_h266_syntax_template.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/cbs_h266_syntax_template.c
>> b/libavcodec/cbs_h266_syntax_template.c
>> index 26ee7a420b..1d4d0c796f 100644
>> --- a/libavcodec/cbs_h266_syntax_template.c
>> +++ b/libavcodec/cbs_h266_syntax_template.c
>> @@ -1130,7 +1130,7 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
>> RWContext *rw,
>> flag(sps_subpic_info_present_flag);
>> if (current->sps_subpic_info_present_flag) {
>> - ue(sps_num_subpics_minus1, 1, VVC_MAX_SLICES - 1);
>> + ue(sps_num_subpics_minus1, 0, VVC_MAX_SLICES - 1);
>
> Strictly speaking, MaxSlicesPerAu varies depending on the stream's
> general_level_idc value, listed in table A.2 from Annex-A (in ITU-T
> H.266 V3).
>
> Right now, VVC_MAX_SLICES is defined as 600 in vvc.h, but the max value
> in the spec is 1000, from level 6.3. I assume it was added in V3?
Good catch. It looks like level 6.3 has been in the spec since the
first version, but all the VVC_MAX_* definitions derived from Annex A
did not account for it. Here is a patch which corrects this:
https://ffmpeg.org//pipermail/ffmpeg-devel/2024-February/322140.html
>> if (current->sps_num_subpics_minus1 > 0) {
>> flag(sps_independent_subpics_flag);
>> flag(sps_subpic_same_size_flag);
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Increase VVC_MAX_* definitions for level 6.3
Reported-by: James Almer
Signed-off-by: Frank Plowman
---
libavcodec/vvc.h | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/libavcodec/vvc.h b/libavcodec/vvc.h
index 7d165cdb86..c4cec1eb8f 100644
--- a/libavcodec/vvc.h
+++ b/libavcodec/vvc.h
@@ -123,25 +123,25 @@ enum {
// 7.4.6.1: hrd_cpb_cnt_minus1 is in [0, 31].
VVC_MAX_CPB_CNT = 32,
-// A.4.1: the highest level allows a MaxLumaPs of 35 651 584.
-VVC_MAX_LUMA_PS = 35651584,
+// A.4.1: the highest level allows a MaxLumaPs of 80,216,064.
+VVC_MAX_LUMA_PS = 80216064,
// A.4.1: pic_width_in_luma_samples and pic_height_in_luma_samples are
// constrained to be not greater than sqrt(MaxLumaPs * 8). Hence height/
-// width are bounded above by sqrt(8 * 35651584) = 16888.2 samples.
-VVC_MAX_WIDTH = 16888,
-VVC_MAX_HEIGHT = 16888,
-
-// A.4.1: table A.1 allows at most 440 tiles per au for any level.
-VVC_MAX_TILES_PER_AU = 440,
-// A.4.1: table A.1 did not define max tile rows.
-// in worest a case, we can have 1x440 tiles picture.
+// width are bounded above by sqrt(8 * 80216064) = 25332.4 samples.
+VVC_MAX_WIDTH = 25332,
+VVC_MAX_HEIGHT = 25332,
+
+// A.4.1: table A.2 allows at most 990 tiles per AU for any level.
+VVC_MAX_TILES_PER_AU = 990,
+// A.4.1: table A.2 did not define max tile rows.
+// in worest a case, we can have 1x990 tiles picture.
VVC_MAX_TILE_ROWS= VVC_MAX_TILES_PER_AU,
-// A.4.1: table A.1 allows at most 20 tile columns for any level.
-VVC_MAX_TILE_COLUMNS = 20,
+// A.4.1: table A.2 allows at most 30 tile columns for any level.
+VVC_MAX_TILE_COLUMNS = 30,
-// A.4.1 table A.1 allows at most 600 slice for any level.
-VVC_MAX_SLICES = 600,
+// A.4.1 table A.2 allows at most 1000 slices for any level.
+VVC_MAX_SLICES = 1000,
// 7.4.8: in the worst case (!pps_no_pic_partition_flag and
// sps_entropy_coding_sync_enabled_flag are both true), entry points can be
--
2.43.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Correct sps_num_subpics_minus1 minimum
The spec says "the value of sps_num_subpics_minus1 shall be in the
range of 0 to MaxSlicesPerAu − 1, inclusive."
Signed-off-by: Frank Plowman
---
libavcodec/cbs_h266_syntax_template.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/cbs_h266_syntax_template.c
b/libavcodec/cbs_h266_syntax_template.c
index 26ee7a420b..1d4d0c796f 100644
--- a/libavcodec/cbs_h266_syntax_template.c
+++ b/libavcodec/cbs_h266_syntax_template.c
@@ -1130,7 +1130,7 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
RWContext *rw,
flag(sps_subpic_info_present_flag);
if (current->sps_subpic_info_present_flag) {
-ue(sps_num_subpics_minus1, 1, VVC_MAX_SLICES - 1);
+ue(sps_num_subpics_minus1, 0, VVC_MAX_SLICES - 1);
if (current->sps_num_subpics_minus1 > 0) {
flag(sps_independent_subpics_flag);
flag(sps_subpic_same_size_flag);
--
2.43.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Fail inter prediction if using IBC
IBC is not yet implemented. Fail the inter prediction process with
AVERROR_PATCHWELCOME if the bitstream uses IBC. Fixes crashes due to
out-of-bounds reads when attempting to decode IBC bitstreams.
Signed-off-by: Frank Plowman
---
libavcodec/vvc/vvc_inter.c | 30 +++---
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/libavcodec/vvc/vvc_inter.c b/libavcodec/vvc/vvc_inter.c
index e05f3db93e..cb5e8d4ef6 100644
--- a/libavcodec/vvc/vvc_inter.c
+++ b/libavcodec/vvc/vvc_inter.c
@@ -779,7 +779,7 @@ static void derive_sb_mv(VVCLocalContext *lc, MvField *mv,
MvField *orig_mv, int
}
}
-static void pred_regular_blk(VVCLocalContext *lc, const int skip_ciip)
+static int pred_regular_blk(VVCLocalContext *lc, const int skip_ciip)
{
const VVCFrameContext *fc = lc->fc;
const CodingUnit *cu= lc->cu;
@@ -789,7 +789,7 @@ static void pred_regular_blk(VVCLocalContext *lc, const int
skip_ciip)
int sbw, sbh, sb_bdof_flag = 0;
if (cu->ciip_flag && skip_ciip)
-return;
+return 0;
sbw = cu->cb_width / mi->num_sb_x;
sbh = cu->cb_height / mi->num_sb_y;
@@ -803,11 +803,17 @@ static void pred_regular_blk(VVCLocalContext *lc, const
int skip_ciip)
ff_vvc_set_neighbour_available(lc, x0, y0, sbw, sbh);
derive_sb_mv(lc, &mv, &orig_mv, &sb_bdof_flag, x0, y0, sbw, sbh);
+if (mv.pred_flag == PF_INTRA) {
+avpriv_report_missing_feature(fc->log_ctx, "Intra Block Copy");
+return AVERROR_PATCHWELCOME;
+}
pred_regular_luma(lc, mi->hpel_if_idx, mi->hpel_if_idx, &mv, x0,
y0, sbw, sbh, &orig_mv, sb_bdof_flag);
if (fc->ps.sps->r->sps_chroma_format_idc)
pred_regular_chroma(lc, &mv, x0, y0, sbw, sbh, &orig_mv,
pu->dmvr_flag);
}
}
+
+return 0;
}
static void derive_affine_mvc(MvField *mvc, const VVCFrameContext *fc, const
MvField *mv,
@@ -872,23 +878,29 @@ static void pred_affine_blk(VVCLocalContext *lc)
}
}
-static void predict_inter(VVCLocalContext *lc)
+static int predict_inter(VVCLocalContext *lc)
{
const VVCFrameContext *fc = lc->fc;
const CodingUnit *cu= lc->cu;
const PredictionUnit *pu= &cu->pu;
+int ret;
if (pu->merge_gpm_flag)
pred_gpm_blk(lc);
else if (pu->inter_affine_flag)
pred_affine_blk(lc);
-else
-pred_regular_blk(lc, 1);//intra block is not ready yet, skip ciip
+else {
+ret = pred_regular_blk(lc, 1);//intra block is not ready yet, skip
ciip
+if (ret < 0)
+return ret;
+}
if (lc->sc->sh.r->sh_lmcs_used_flag && !cu->ciip_flag) {
uint8_t* dst0 = POS(0, cu->x0, cu->y0);
fc->vvcdsp.lmcs.filter(dst0, fc->frame->linesize[LUMA], cu->cb_width,
cu->cb_height, fc->ps.lmcs.fwd_lut);
}
+
+return 0;
}
static int has_inter_luma(const CodingUnit *cu)
@@ -901,11 +913,15 @@ int ff_vvc_predict_inter(VVCLocalContext *lc, const int
rs)
const VVCFrameContext *fc = lc->fc;
const CTU *ctu = fc->tab.ctus + rs;
CodingUnit *cu = ctu->cus;
+int ret;
while (cu) {
lc->cu = cu;
-if (has_inter_luma(cu))
-predict_inter(lc);
+if (has_inter_luma(cu)) {
+ret = predict_inter(lc);
+if (ret < 0)
+return ret;
+}
cu = cu->next;
}
--
2.43.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/vvcdec: frame_context_setup, set fc->ref to NULL
On 13/02/2024 02:30, Nuo Mi wrote:
> fc->ref points to an old VVCFrame, which cannot be used after
> frame_context_setup.
> This prevents crashes in decode_nal_units-->ff_vvc_report_frame_finished.
>
> Signed-off-by: Frank Plowman
> ---
> libavcodec/vvc/vvcdec.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/libavcodec/vvc/vvcdec.c b/libavcodec/vvc/vvcdec.c
> index 8163b5ecb6..e88e746de4 100644
> --- a/libavcodec/vvc/vvcdec.c
> +++ b/libavcodec/vvc/vvcdec.c
> @@ -594,6 +594,8 @@ static int frame_context_setup(VVCFrameContext *fc,
> VVCContext *s)
> {
> int ret;
>
> +fc->ref = NULL;
> +
> // copy refs from the last frame
> if (s->nb_frames && s->nb_fcs > 1) {
> VVCFrameContext *prev = get_frame_context(s, fc, -1);
LGTM. Fixes the crash on all the fuzz data I have which produce it.
FATE runners are failing at the time of writing, but I manually ran this
against the VVC tests as well as the suite from the FFVVC GitHub and all
tests passed.
Btw, I don't think you should add Signed-off-by tags for other people.
Their exact meaning varies by project and I am not sure of their meaning
in FFmpeg (if there is one), but generally they indicate that person
claims some sort of responsibility for the patch in the case of e.g. a
license violation. That being said, I am happy to sign this off.
--
Frank
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/vvc: Check fc->ref contains valid reference
On 08/02/2024 21:50, Lynne wrote: > Feb 8, 2024, 22:16 by p...@frankplowman.com: > >> From: Frank Plowman >> >> Depending on where exactly decode_nal_unit failed, it is possible that >> fc->ref holds a VVCFrame which has had ff_vvc_unref_frame called on it >> and not yet had ref_frame called on it. In this case, fc->ref most of >> the fields of fc->ref are NULL and attempting to call >> ff_vvc_report_frame_finished on it will result in a null dereference. >> >> Patch fixes the error described above by checking fc->ref has not only >> been allocated, but also references a valid AVFrame before attempting to >> call ff_vvc_report_frame_finished on it. >> >> Signed-off-by: Frank Plowman >> --- >> libavcodec/vvc/vvcdec.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavcodec/vvc/vvcdec.c b/libavcodec/vvc/vvcdec.c >> index 8163b5ecb6..246ee79299 100644 >> --- a/libavcodec/vvc/vvcdec.c >> +++ b/libavcodec/vvc/vvcdec.c >> @@ -820,7 +820,7 @@ static int decode_nal_units(VVCContext *s, >> VVCFrameContext *fc, AVPacket *avpkt) >> return 0; >> >> fail: >> -if (fc->ref) >> +if (fc->ref && fc->ref->frame->buf[0]) >> ff_vvc_report_frame_finished(fc->ref); >> return ret; >> } >> > > In general, for other codecs, if a reference does not exist, > we simply allocate it and pretend it existed and was correctly decoded. > This has better resilience against corrupt bitstreams or just bad muxing, > and yields an (maybe corrupt) output, which is better than nothing. > > Is this not possible for VVC? I think the naming is confusing here. fc->ref is a pointer to the VVCFrame currently being decoded. -- Frank ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [REFUND-REQUEST] FOSDEM '24 Expenses
Hello, I am requesting reimbursement for the following expenses made attending FOSDEM 2024, where I delivered a talk about the FFmpeg VVC decoder and met with FFmpeg developers. Expense | Amount (GBP) --+- Return flight MAN <-> CRL |45.98 Coach LDS -> MAN | 7.80 Coach MAN -> LDS | 9.00 2 nights Airbnb | 110.07 Train CRL -> BRU |14.90 Coach BRU -> CRL |16.91 Taxi LDS -> Home |10.92 --+- Total | 215.58 I took the taxi from the coach station to my house as it was in the early hours and public transport was not running. Thanks, Frank ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu/thread: Check HAVE_PTHREAD_SET_?NAME_NP is defined
On 06/02/2024 11:23, Andreas Rheinhardt wrote: > p...@frankplowman.com: >> From: Frank Plowman >> >> Check HAVE_PTHREAD_SETNAME_NP and HAVE_PTHREAD_SET_NAME_NP are defined >> before using them in macro conditions. Gets rid of lots of -Wundef >> warnings present when building on MacOS since >> fd16d8c68cd7b820eda76c407b0645b7cf470efd. >> >> Signed-off-by: Frank Plowman >> --- >> libavutil/thread.h | 8 +--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/libavutil/thread.h b/libavutil/thread.h >> index 2c00c7cc35..0200b7b511 100644 >> --- a/libavutil/thread.h >> +++ b/libavutil/thread.h >> @@ -26,7 +26,9 @@ >> >> #if HAVE_PRCTL >> #include >> -#elif (HAVE_PTHREAD_SETNAME_NP || HAVE_PTHREAD_SET_NAME_NP) && >> HAVE_PTHREAD_NP_H >> +#elif ((defined(HAVE_PTHREAD_SETNAME_NP) && HAVE_PTHREAD_SETNAME_NP) \ >> +|| (defined(HAVE_PTHREAD_SET_NAME_NP) && HAVE_PTHREAD_SET_NAME_NP)) \ >> +&& HAVE_PTHREAD_NP_H >> #include >> #endif >> >> @@ -219,7 +221,7 @@ static inline int ff_thread_setname(const char *name) >> >> #if HAVE_PRCTL >> ret = AVERROR(prctl(PR_SET_NAME, name)); >> -#elif HAVE_PTHREAD_SETNAME_NP >> +#elif defined(HAVE_PTHREAD_SETNAME_NP) && HAVE_PTHREAD_SETNAME_NP >> #if defined(__APPLE__) >> ret = AVERROR(pthread_setname_np(name)); >> #elif defined(__NetBSD__) >> @@ -227,7 +229,7 @@ static inline int ff_thread_setname(const char *name) >> #else >> ret = AVERROR(pthread_setname_np(pthread_self(), name)); >> #endif >> -#elif HAVE_PTHREAD_SET_NAME_NP >> +#elif defined(HAVE_PTHREAD_SET_NAME_NP) && HAVE_PTHREAD_SET_NAME_NP >> pthread_set_name_np(pthread_self(), name); >> #else >> ret = AVERROR(ENOSYS); > > You need to rerun configure. Since > fd16d8c68cd7b820eda76c407b0645b7cf470efd running configure will add > HAVE_PTHREAD_SET_NAME_NP and HAVE_PTHREAD_SETNAME_NP to your config.h file. Ah, I see thanks. Sorry for the noise! ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/vvc: Error pps_single_slice_per_subpic_flag
On 03/02/2024 16:56, Nuo Mi wrote:
On Sat, Feb 3, 2024 at 11:51 PM Frank Plowman wrote:
On 03/02/2024 15:46, Nuo Mi wrote:
On Sat, Feb 3, 2024 at 9:54 PM Frank Plowman
wrote:
On 02/02/2024 14:39, Nuo Mi wrote:
On Thu, Feb 1, 2024 at 10:01 PM wrote:
From: Frank Plowman
pps_single_slice_per_subpic_flag is not yet supported. Support is
WIP,
but in the meantime throw an error when trying to decode a bitstream
with it set, avoiding an out-of-bounds array access.
Fixes: out-of-bounds array access for conformance bitstreams
SUBPIC_C_ERICSSON_1, SUBPIC_D_ERICSSON_1, MNUT_A_Nokia_4 and
MNUT_B_Nokia_3.
Signed-off-by: Frank Plowman
---
libavcodec/vvc/vvc_ps.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/libavcodec/vvc/vvc_ps.c b/libavcodec/vvc/vvc_ps.c
index 2cf156b323..bd81d70e71 100644
--- a/libavcodec/vvc/vvc_ps.c
+++ b/libavcodec/vvc/vvc_ps.c
@@ -381,11 +381,16 @@ static void pps_multi_tiles_slice(VVCPPS *pps,
const
int tile_idx, const int i,
}
}
-static void pps_rect_slice(VVCPPS* pps)
+static int pps_rect_slice(VVCPPS* pps)
{
const H266RawPPS* r = pps->r;
int tile_idx = 0, off = 0;
+if (r->pps_single_slice_per_subpic_flag) {
+avpriv_report_missing_feature(NULL,
"pps_single_slice_per_subpic_flag");
+return AVERROR_PATCHWELCOME;
+}
+
for (int i = 0; i < r->pps_num_slices_in_pic_minus1 + 1; i++) {
if (!r->pps_slice_width_in_tiles_minus1[i] &&
!r->pps_slice_height_in_tiles_minus1[i]) {
@@ -396,9 +401,11 @@ static void pps_rect_slice(VVCPPS* pps)
}
tile_idx = next_tile_idx(tile_idx, i, r);
}
+
+return 0;
}
-static void pps_no_rect_slice(VVCPPS* pps)
+static int pps_no_rect_slice(VVCPPS* pps)
{
const H266RawPPS* r = pps->r;
int ctu_x, ctu_y, off = 0;
@@ -409,20 +416,24 @@ static void pps_no_rect_slice(VVCPPS* pps)
pps_add_ctus(pps, &off, ctu_x, ctu_y,
r->col_width_val[tile_x], r->row_height_val[tile_y]);
}
}
+
+return 0;
}
static int pps_slice_map(VVCPPS *pps)
{
+int ret;
+
pps->ctb_addr_in_slice = av_calloc(pps->ctb_count,
sizeof(*pps->ctb_addr_in_slice));
if (!pps->ctb_addr_in_slice)
return AVERROR(ENOMEM);
if (pps->r->pps_rect_slice_flag)
-pps_rect_slice(pps);
+ret = pps_rect_slice(pps);
else
-pps_no_rect_slice(pps);
+ret = pps_no_rect_slice(pps);
-return 0;
+return ret;
}
Thank you Frank. This changed too much code.
How about we only check the sps_num_subpics_minus1 in decode_sps.
I wrote it like this so that the avpriv_report_missing_feature is where
the feature would need to be, helping readability and searchability.
We need to make changes to both the cbs and the decoder for subpic
support.
pps_slice_map is not the first place.
There is nothing strictly missing in the CBS, only the derivation of
NumSlicesInSub needs to be moved which is quite subtle; I think the
putting the error in the parameter set parser is clearer.
How is the patch below as an alternative?
This fixes the single_slice_per_subpic_flag.
But fuzzer may find another subpic-related issue. Highly possible they will
crash too. :)
check sub picture number is a safer way
This issue can cause a crash even with the minimum
{s,p}ps_num_subpics_minus1 = 0 I believe, so this check is needed
regardless. We can add a PATCHWELCOME error if
{s,p}ps_num_subpics_minus1 > 0, but this should be a separate commit.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/vvc: Validate alf_list indexes
On 05/02/2024 13:45, Nuo Mi wrote:
On Sun, Feb 4, 2024 at 1:22 AM wrote:
From: Frank Plowman
Fixes crashes when decoding illegal bitstreams found by fuzzing.
Signed-off-by: Frank Plowman
---
libavcodec/vvc/vvc_ctu.c | 17 +++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/libavcodec/vvc/vvc_ctu.c b/libavcodec/vvc/vvc_ctu.c
index d166b16a19..f601c6c95e 100644
--- a/libavcodec/vvc/vvc_ctu.c
+++ b/libavcodec/vvc/vvc_ctu.c
@@ -2207,7 +2207,7 @@ static void hls_sao(VVCLocalContext *lc, const int
rx, const int ry)
}
}
-static void alf_params(VVCLocalContext *lc, const int rx, const int ry)
+static int alf_params(VVCLocalContext *lc, const int rx, const int ry)
{
const VVCFrameContext *fc = lc->fc;
const H266RawSliceHeader *sh = lc->sc->sh.r;
@@ -2233,6 +2233,11 @@ static void alf_params(VVCLocalContext *lc, const
int rx, const int ry)
c_idx == CB ? sh->sh_alf_cb_enabled_flag :
sh->sh_alf_cr_enabled_flag;
if (alf_enabled_flag) {
const VVCALF *aps =
fc->ps.alf_list[sh->sh_alf_aps_id_chroma];
+if (!aps) {
+// Slice header refers to an APS which does not exist
+return AVERROR_INVALIDDATA;
+}
+
Thank you Frank.
Since all "if conditions" are related to slices, could we perform the check
at sh_derive()?
Yes, I had another look at this yesterday and came to the same
conclusion. Do you think it's better to derive pointers to the relevant
APSs or just to validate the indices into fps->aps_list?
alf->ctb_flag[c_idx] = ff_vvc_alf_ctb_flag(lc, rx, ry,
c_idx);
alf->alf_ctb_filter_alt_idx[c_idx - 1] = 0;
if (alf->ctb_flag[c_idx] && aps->num_chroma_filters > 1)
@@ -2247,10 +2252,16 @@ static void alf_params(VVCLocalContext *lc, const
int rx, const int ry)
alf->ctb_cc_idc[i] = 0;
if (cc_enabled[i]) {
const VVCALF *aps = fc->ps.alf_list[cc_aps_id[i]];
+if (!aps) {
+// Slice header refers to an APS which does not exist
+return AVERROR_INVALIDDATA;
+}
alf->ctb_cc_idc[i] = ff_vvc_alf_ctb_cc_idc(lc, rx, ry, i,
aps->num_cc_filters[i]);
}
}
}
+
+return 0;
}
static void deblock_params(VVCLocalContext *lc, const int rx, const int
ry)
@@ -2274,7 +2285,9 @@ static int hls_coding_tree_unit(VVCLocalContext *lc,
memset(lc->parse.chroma_qp_offset, 0,
sizeof(lc->parse.chroma_qp_offset));
hls_sao(lc, x0 >> sps->ctb_log2_size_y, y0 >> sps->ctb_log2_size_y);
-alf_params(lc, x0 >> sps->ctb_log2_size_y, y0 >>
sps->ctb_log2_size_y);
+ret = alf_params(lc, x0 >> sps->ctb_log2_size_y, y0 >>
sps->ctb_log2_size_y);
+if (ret < 0)
+return ret;
deblock_params(lc, x0 >> sps->ctb_log2_size_y, y0 >>
sps->ctb_log2_size_y);
if (IS_I(rsh) && sps->r->sps_qtbtt_dual_tree_intra_flag)
--
2.43.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-requ...@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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/vvc: Error pps_single_slice_per_subpic_flag
On 03/02/2024 15:46, Nuo Mi wrote:
On Sat, Feb 3, 2024 at 9:54 PM Frank Plowman wrote:
On 02/02/2024 14:39, Nuo Mi wrote:
On Thu, Feb 1, 2024 at 10:01 PM wrote:
From: Frank Plowman
pps_single_slice_per_subpic_flag is not yet supported. Support is WIP,
but in the meantime throw an error when trying to decode a bitstream
with it set, avoiding an out-of-bounds array access.
Fixes: out-of-bounds array access for conformance bitstreams
SUBPIC_C_ERICSSON_1, SUBPIC_D_ERICSSON_1, MNUT_A_Nokia_4 and
MNUT_B_Nokia_3.
Signed-off-by: Frank Plowman
---
libavcodec/vvc/vvc_ps.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/libavcodec/vvc/vvc_ps.c b/libavcodec/vvc/vvc_ps.c
index 2cf156b323..bd81d70e71 100644
--- a/libavcodec/vvc/vvc_ps.c
+++ b/libavcodec/vvc/vvc_ps.c
@@ -381,11 +381,16 @@ static void pps_multi_tiles_slice(VVCPPS *pps,
const
int tile_idx, const int i,
}
}
-static void pps_rect_slice(VVCPPS* pps)
+static int pps_rect_slice(VVCPPS* pps)
{
const H266RawPPS* r = pps->r;
int tile_idx = 0, off = 0;
+if (r->pps_single_slice_per_subpic_flag) {
+avpriv_report_missing_feature(NULL,
"pps_single_slice_per_subpic_flag");
+return AVERROR_PATCHWELCOME;
+}
+
for (int i = 0; i < r->pps_num_slices_in_pic_minus1 + 1; i++) {
if (!r->pps_slice_width_in_tiles_minus1[i] &&
!r->pps_slice_height_in_tiles_minus1[i]) {
@@ -396,9 +401,11 @@ static void pps_rect_slice(VVCPPS* pps)
}
tile_idx = next_tile_idx(tile_idx, i, r);
}
+
+return 0;
}
-static void pps_no_rect_slice(VVCPPS* pps)
+static int pps_no_rect_slice(VVCPPS* pps)
{
const H266RawPPS* r = pps->r;
int ctu_x, ctu_y, off = 0;
@@ -409,20 +416,24 @@ static void pps_no_rect_slice(VVCPPS* pps)
pps_add_ctus(pps, &off, ctu_x, ctu_y,
r->col_width_val[tile_x], r->row_height_val[tile_y]);
}
}
+
+return 0;
}
static int pps_slice_map(VVCPPS *pps)
{
+int ret;
+
pps->ctb_addr_in_slice = av_calloc(pps->ctb_count,
sizeof(*pps->ctb_addr_in_slice));
if (!pps->ctb_addr_in_slice)
return AVERROR(ENOMEM);
if (pps->r->pps_rect_slice_flag)
-pps_rect_slice(pps);
+ret = pps_rect_slice(pps);
else
-pps_no_rect_slice(pps);
+ret = pps_no_rect_slice(pps);
-return 0;
+return ret;
}
Thank you Frank. This changed too much code.
How about we only check the sps_num_subpics_minus1 in decode_sps.
I wrote it like this so that the avpriv_report_missing_feature is where
the feature would need to be, helping readability and searchability.
We need to make changes to both the cbs and the decoder for subpic support.
pps_slice_map is not the first place.
There is nothing strictly missing in the CBS, only the derivation of
NumSlicesInSub needs to be moved which is quite subtle; I think the
putting the error in the parameter set parser is clearer.
How is the patch below as an alternative?
diff --git a/libavcodec/vvc/vvc_ps.c b/libavcodec/vvc/vvc_ps.c
index 2cf156b323..4ef8f9f9b9 100644
--- a/libavcodec/vvc/vvc_ps.c
+++ b/libavcodec/vvc/vvc_ps.c
@@ -413,13 +413,20 @@ static void pps_no_rect_slice(VVCPPS* pps)
static int pps_slice_map(VVCPPS *pps)
{
+const H266RawPPS* r = pps->r;
+
pps->ctb_addr_in_slice = av_calloc(pps->ctb_count,
sizeof(*pps->ctb_addr_in_slice));
if (!pps->ctb_addr_in_slice)
return AVERROR(ENOMEM);
-if (pps->r->pps_rect_slice_flag)
+if (pps->r->pps_rect_slice_flag) {
+if (r->pps_single_slice_per_subpic_flag) {
+avpriv_report_missing_feature(NULL,
"pps_single_slice_per_subpic_flag");
+return AVERROR_PATCHWELCOME;
+}
+
pps_rect_slice(pps);
-else
+} else
pps_no_rect_slice(pps);
return 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/vvc: Error pps_single_slice_per_subpic_flag
On 02/02/2024 14:39, Nuo Mi wrote:
On Thu, Feb 1, 2024 at 10:01 PM wrote:
From: Frank Plowman
pps_single_slice_per_subpic_flag is not yet supported. Support is WIP,
but in the meantime throw an error when trying to decode a bitstream
with it set, avoiding an out-of-bounds array access.
Fixes: out-of-bounds array access for conformance bitstreams
SUBPIC_C_ERICSSON_1, SUBPIC_D_ERICSSON_1, MNUT_A_Nokia_4 and
MNUT_B_Nokia_3.
Signed-off-by: Frank Plowman
---
libavcodec/vvc/vvc_ps.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/libavcodec/vvc/vvc_ps.c b/libavcodec/vvc/vvc_ps.c
index 2cf156b323..bd81d70e71 100644
--- a/libavcodec/vvc/vvc_ps.c
+++ b/libavcodec/vvc/vvc_ps.c
@@ -381,11 +381,16 @@ static void pps_multi_tiles_slice(VVCPPS *pps, const
int tile_idx, const int i,
}
}
-static void pps_rect_slice(VVCPPS* pps)
+static int pps_rect_slice(VVCPPS* pps)
{
const H266RawPPS* r = pps->r;
int tile_idx = 0, off = 0;
+if (r->pps_single_slice_per_subpic_flag) {
+avpriv_report_missing_feature(NULL,
"pps_single_slice_per_subpic_flag");
+return AVERROR_PATCHWELCOME;
+}
+
for (int i = 0; i < r->pps_num_slices_in_pic_minus1 + 1; i++) {
if (!r->pps_slice_width_in_tiles_minus1[i] &&
!r->pps_slice_height_in_tiles_minus1[i]) {
@@ -396,9 +401,11 @@ static void pps_rect_slice(VVCPPS* pps)
}
tile_idx = next_tile_idx(tile_idx, i, r);
}
+
+return 0;
}
-static void pps_no_rect_slice(VVCPPS* pps)
+static int pps_no_rect_slice(VVCPPS* pps)
{
const H266RawPPS* r = pps->r;
int ctu_x, ctu_y, off = 0;
@@ -409,20 +416,24 @@ static void pps_no_rect_slice(VVCPPS* pps)
pps_add_ctus(pps, &off, ctu_x, ctu_y,
r->col_width_val[tile_x], r->row_height_val[tile_y]);
}
}
+
+return 0;
}
static int pps_slice_map(VVCPPS *pps)
{
+int ret;
+
pps->ctb_addr_in_slice = av_calloc(pps->ctb_count,
sizeof(*pps->ctb_addr_in_slice));
if (!pps->ctb_addr_in_slice)
return AVERROR(ENOMEM);
if (pps->r->pps_rect_slice_flag)
-pps_rect_slice(pps);
+ret = pps_rect_slice(pps);
else
-pps_no_rect_slice(pps);
+ret = pps_no_rect_slice(pps);
-return 0;
+return ret;
}
Thank you Frank. This changed too much code.
How about we only check the sps_num_subpics_minus1 in decode_sps.
I wrote it like this so that the avpriv_report_missing_feature is where
the feature would need to be, helping readability and searchability. I
could remove the return from pps_rect_slice and pps_no_rect_slice which
would get rid of a handful of changed lines but the changes are trivial
so it is not a big deal imo.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/vvc: Add check to num_multi_layer_olss
On 30/01/2024 12:55, Frank Plowman wrote: On 30/01/2024 12:31, Nuo Mi wrote: On Tue, Jan 30, 2024 at 5:41 PM wrote: From: Frank Plowman Check that vps_each_layer_is_an_ols_flag, which indicates that "at least one OLS specified by the VPS contains more than one layer," is set if num_multi_layer_olss is non-zero. Fixes: 65160/clusterfuzz-testcase-minimized-ffmpeg_BSF_VVC_METADATA_fuzzer-4665241535119360 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by <https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by>: Frank Plowman --- libavcodec/cbs_h266_syntax_template.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libavcodec/cbs_h266_syntax_template.c b/libavcodec/cbs_h266_syntax_template.c index 2f3478e5e1..37dc3acba0 100644 --- a/libavcodec/cbs_h266_syntax_template.c +++ b/libavcodec/cbs_h266_syntax_template.c @@ -911,6 +911,8 @@ static int FUNC(vps) (CodedBitstreamContext *ctx, RWContext *rw, num_multi_layer_olss++; } } + if (!current->vps_each_layer_is_an_ols_flag && num_multi_layer_olss == 0) + return AVERROR_INVALIDDATA; } The specification does not provide information on how to obtain TotalNumOlss (total_num_olss) when ols_mode_idc is set to 3. Therefore, the earlier line "u(8, vps_num_ptls_minus1, 0, total_num_olss - 1)" is undefined. We'd better return a patch welcome error instead of printing a warning before vps_num_ptls_minus1 line This is the same behaviour James suggested in an earlier patch. The spec says "decoders conforming to this version of this Specification shall ignore the OLSs with vps_ols_mode_idc equal to 3." I don't think this should be an error as the spec is unambiguous here. Perhaps we can instead skip the remainder of the VPS if vps_ols_mode_idc is 3? Or is there some better way to ignore these OLSs? For reference, VTM's behaviour is the same as the current behaviour: TotalNumOlss is assumed to be 0 when ols_mode_idc, hence most of the remaining syntax elements in the VPS are not read as they are within for (i = 0; i < total_num_olss; i++) loops or other loops with bounds derived from total_num_olss. On the other hand, VVdeC's behaviour is the same as you suggest: it throws an error if total_num_olss is 3. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/vvc: Add check to num_multi_layer_olss
On 30/01/2024 12:31, Nuo Mi wrote: On Tue, Jan 30, 2024 at 5:41 PM wrote: From: Frank Plowman Check that vps_each_layer_is_an_ols_flag, which indicates that "at least one OLS specified by the VPS contains more than one layer," is set if num_multi_layer_olss is non-zero. Fixes: 65160/clusterfuzz-testcase-minimized-ffmpeg_BSF_VVC_METADATA_fuzzer-4665241535119360 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by <https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by>: Frank Plowman --- libavcodec/cbs_h266_syntax_template.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libavcodec/cbs_h266_syntax_template.c b/libavcodec/cbs_h266_syntax_template.c index 2f3478e5e1..37dc3acba0 100644 --- a/libavcodec/cbs_h266_syntax_template.c +++ b/libavcodec/cbs_h266_syntax_template.c @@ -911,6 +911,8 @@ static int FUNC(vps) (CodedBitstreamContext *ctx, RWContext *rw, num_multi_layer_olss++; } } +if (!current->vps_each_layer_is_an_ols_flag && num_multi_layer_olss == 0) +return AVERROR_INVALIDDATA; } The specification does not provide information on how to obtain TotalNumOlss (total_num_olss) when ols_mode_idc is set to 3. Therefore, the earlier line "u(8, vps_num_ptls_minus1, 0, total_num_olss - 1)" is undefined. We'd better return a patch welcome error instead of printing a warning before vps_num_ptls_minus1 line This is the same behaviour James suggested in an earlier patch. The spec says "decoders conforming to this version of this Specification shall ignore the OLSs with vps_ols_mode_idc equal to 3." I don't think this should be an error as the spec is unambiguous here. Perhaps we can instead skip the remainder of the VPS if vps_ols_mode_idc is 3? Or is there some better way to ignore these OLSs? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavc/vvc: Increase IntraEdgeParams buffer size
The reference line buffers are used with indices in the range
-MAX_TB_SIZE - 3 to refw + FFMAX(1, w/h) * ref_idx + 1, which is
at most 5*MAX_TB_SIZE + 1.
Fixes buffer overflows.
http://fate.ffmpeg.org/report.cgi?slot=armv7-linux-gcc-9&time=20240124051736
---
libavcodec/vvc/vvcdsp.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/libavcodec/vvc/vvcdsp.c b/libavcodec/vvc/vvcdsp.c
index c82ea7be30..56e71d5163 100644
--- a/libavcodec/vvc/vvcdsp.c
+++ b/libavcodec/vvc/vvcdsp.c
@@ -87,10 +87,10 @@ typedef struct IntraEdgeParams {
uint8_t* left;
int filter_flag;
-uint16_t left_array[3 * MAX_TB_SIZE + 3];
-uint16_t filtered_left_array[3 * MAX_TB_SIZE + 3];
-uint16_t top_array[3 * MAX_TB_SIZE + 3];
-uint16_t filtered_top_array[3 * MAX_TB_SIZE + 3];
+uint16_t left_array[6 * MAX_TB_SIZE + 5];
+uint16_t filtered_left_array[6 * MAX_TB_SIZE + 5];
+uint16_t top_array[6 * MAX_TB_SIZE + 5];
+uint16_t filtered_top_array[6 * MAX_TB_SIZE + 5];
} IntraEdgeParams;
#define PROF_BORDER_EXT 1
--
2.43.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/3] avcodec/cbs_h266_syntax_template: sanity check num_multi_layer_olss
On 29/01/2024 21:13, James Almer wrote:
On 1/29/2024 5:19 PM, Frank Plowman wrote:
Below is a patch which addresses the issue, an integer overflow when
calculating the bounds for
vps_num_ols_timing_hrd_params_minus1. There's also a similar fix for
vps_num_dpb_params_minus1.
diff --git a/libavcodec/cbs_h266_syntax_template.c
b/libavcodec/cbs_h266_syntax_template.c
index 549d021211..49bf2e45ac 100644
--- a/libavcodec/cbs_h266_syntax_template.c
+++ b/libavcodec/cbs_h266_syntax_template.c
@@ -946,7 +946,8 @@ static int FUNC(vps) (CodedBitstreamContext *ctx,
RWContext *rw,
if (!current->vps_each_layer_is_an_ols_flag) {
uint16_t vps_num_dpb_params;
- ue(vps_num_dpb_params_minus1, 0, num_multi_layer_olss - 1);
+ ue(vps_num_dpb_params_minus1, 0,
+ num_multi_layer_olss > 0 ? num_multi_layer_olss - 1 : 0);
FFMAX(0, num_multi_layer_olss - 1); looks better.
If the spec explicitly states num_multi_layer_olss - 1 should be used
here, wouldn't that mean that it doesn't expect num_multi_layer_olss
to be 0 at this point for vps_ols_mode_idc >= 0 && vps_ols_mode_idc < 3?
When vps_each_layer_is_an_ols_flag is true, num_multi_layer_olss is
inferred as 1
num_multi_layer_olss is 0 if vps_each_layer_is_an_ols_flag is true, it
is num_layers_in_ols which must be 1.
so I'd expect it to also be at least 1 for
vps_each_layer_is_an_ols_flag == false.
Yes I think you're right. The spec isn't especially clear, but the
description of the vps_each_layer_is_an_ols_flag leads me to believe we
are safe to assert that. Patch which does so below.
diff --git a/libavcodec/cbs_h266_syntax_template.c
b/libavcodec/cbs_h266_syntax_template.c
index 549d021211..5ae3fb9f08 100644
--- a/libavcodec/cbs_h266_syntax_template.c
+++ b/libavcodec/cbs_h266_syntax_template.c
@@ -912,6 +912,8 @@ static int FUNC(vps) (CodedBitstreamContext *ctx,
RWContext *rw,
num_multi_layer_olss++;
}
}
+ if (!current->vps_each_layer_is_an_ols_flag &&
num_multi_layer_olss == 0)
+ return AVERROR_INVALIDDATA;
}
for (i = 0; i <= current->vps_num_ptls_minus1; i++) {
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/3] avcodec/cbs_h266_syntax_template: sanity check num_multi_layer_olss
On 29/01/2024 19:04, James Almer wrote:
Well, turns out the current code is fine and my suggested change above
is wrong. Fun how that goes.
Can you test the following instead?
diff --git a/libavcodec/cbs_h266_syntax_template.c
b/libavcodec/cbs_h266_syntax_template.c
index 549d021211..30b4ae3bc0 100644
--- a/libavcodec/cbs_h266_syntax_template.c
+++ b/libavcodec/cbs_h266_syntax_template.c
@@ -764,7 +764,7 @@ static int FUNC(vps) (CodedBitstreamContext *ctx,
RWContext *rw,
infer(vps_each_layer_is_an_ols_flag, 0);
if (!current->vps_each_layer_is_an_ols_flag) {
if (!current->vps_all_independent_layers_flag)
- ub(2, vps_ols_mode_idc);
+ u(2, vps_ols_mode_idc, 0, 2);
else
infer(vps_ols_mode_idc, 2);
if (current->vps_ols_mode_idc == 2) {
The spec reads "Decoders conforming to this version of this
Specification shall *ignore* the OLSs with
vps_ols_mode_idc equal to 3." This change throws an error for these
OLSs, which I don't think is correct.
There is already some logic just below this to warn the user if
vps_ols_mode_idc is 3.
@@ -902,11 +902,10 @@ static int FUNC(vps) (CodedBitstreamContext
*ctx, RWContext *rw,
current->vps_ols_mode_idc == 1) {
num_layers_in_ols = i + 1;
} else if (current->vps_ols_mode_idc == 2) {
- for (k = 0, j = 0; k <=
current->vps_max_layers_minus1; k++) {
+ for (k = 0, j = 0; k <=
current->vps_max_layers_minus1; k++)
if (layer_included_in_ols_flag[i][k])
j++;
- num_layers_in_ols = j;
- }
+ num_layers_in_ols = j;
}
if (num_layers_in_ols > 1) {
num_multi_layer_olss++;
This looks good to me, the old behaviour was wrong. I don't think this
is what was causing this
particular crash however.
Below is a patch which addresses the issue, an integer overflow when
calculating the bounds for
vps_num_ols_timing_hrd_params_minus1. There's also a similar fix for
vps_num_dpb_params_minus1.
diff --git a/libavcodec/cbs_h266_syntax_template.c
b/libavcodec/cbs_h266_syntax_template.c
index 549d021211..49bf2e45ac 100644
--- a/libavcodec/cbs_h266_syntax_template.c
+++ b/libavcodec/cbs_h266_syntax_template.c
@@ -946,7 +946,8 @@ static int FUNC(vps) (CodedBitstreamContext *ctx,
RWContext *rw,
if (!current->vps_each_layer_is_an_ols_flag) {
uint16_t vps_num_dpb_params;
- ue(vps_num_dpb_params_minus1, 0, num_multi_layer_olss - 1);
+ ue(vps_num_dpb_params_minus1, 0,
+ num_multi_layer_olss > 0 ? num_multi_layer_olss - 1 : 0);
if (current->vps_each_layer_is_an_ols_flag)
vps_num_dpb_params = 0;
else
@@ -991,7 +992,7 @@ static int FUNC(vps) (CodedBitstreamContext *ctx,
RWContext *rw,
else
infer(vps_sublayer_cpb_params_present_flag, 0);
ue(vps_num_ols_timing_hrd_params_minus1, 0,
- num_multi_layer_olss - 1);
+ num_multi_layer_olss > 0 ? num_multi_layer_olss - 1 : 0);
for (i = 0; i <=
current->vps_num_ols_timing_hrd_params_minus1; i++) {
uint8_t first_sublayer;
if (!current->vps_default_ptl_dpb_hrd_max_tid_flag)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/ffrtmpcrypt: Fix int-conversion warning
Hi Martin, Thanks for the review. On 22/12/2023 12:15, Martin Storsjö wrote: The change LGTM, but the wording here is slightly confusing IMO. The problem isn't with using per se AVERROR, that's just a macro for generating suitable integers, the issue is more about the fact that we're returning from a macro, without knowing the actual context where the macro is invoked. WDYT about this wording? The gcrypt definition of `bn_new` used to use the return statement on errors, with an AVERROR return value, regardless of the signature of the function where the macro is used - it is called in `dh_generate_key` and `ff_dh_init` which return pointers. As a result, compiling with gcrypt and the ffrtmpcrypt protocol resulted in an int-conversion warning. GCC 14 may upgrade these to errors [1]. Yeah this is better, I agree. Cheers, Frank -- https://www.frankplowman.com/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avformat/ffrtmpcrypt: Fix int-conversion warning
The gcrypt definition of `bn_new` used to use `AVERROR`, however it is
called in `dh_generate_key` and `ff_dh_init` which return pointers. As a
result, compiling with gcrypt and the ffrtmpcrypt protocol resulted in an
int-conversion warning. GCC 14 may upgrade these to errors [1].
This patch fixes the problem by changing the macro to remove `AVERROR`
and instead set `bn` to null if the allocation fails. This is the
behaviour of all the other `bn_new` implementations and so the result is
already checked at all the callsites. AFAICT, this should be the only
change needed to get ffmpeg off Fedora's naughty list of projects with
warnings which may be upgraded to errors in GCC 14 [2].
[1]: https://gcc.gnu.org/pipermail/gcc/2023-May/241264.html
[2]: https://www.mail-archive.com/devel@lists.fedoraproject.org/msg196024.html
Signed-off-by: Frank Plowman
---
libavformat/rtmpdh.c | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/libavformat/rtmpdh.c b/libavformat/rtmpdh.c
index 5ddae537a1..6a6c2ccd87 100644
--- a/libavformat/rtmpdh.c
+++ b/libavformat/rtmpdh.c
@@ -113,15 +113,18 @@ static int bn_modexp(FFBigNum bn, FFBigNum y, FFBigNum q,
FFBigNum p)
return 0;
}
#elif CONFIG_GCRYPT
-#define bn_new(bn) \
-do {\
-if (!gcry_control(GCRYCTL_INITIALIZATION_FINISHED_P)) { \
-if (!gcry_check_version("1.5.4")) \
-return AVERROR(EINVAL); \
-gcry_control(GCRYCTL_DISABLE_SECMEM, 0);\
-gcry_control(GCRYCTL_INITIALIZATION_FINISHED, 0); \
-} \
-bn = gcry_mpi_new(1); \
+#define bn_new(bn)\
+do { \
+if (!gcry_control(GCRYCTL_INITIALIZATION_FINISHED_P)) { \
+if (gcry_check_version("1.5.4")) {\
+gcry_control(GCRYCTL_DISABLE_SECMEM, 0); \
+gcry_control(GCRYCTL_INITIALIZATION_FINISHED, 0); \
+} \
+} \
+if (gcry_control(GCRYCTL_INITIALIZATION_FINISHED_P)) \
+bn = gcry_mpi_new(1); \
+else \
+bn = NULL;\
} while (0)
#define bn_free(bn) gcry_mpi_release(bn)
#define bn_set_word(bn, w) gcry_mpi_set_ui(bn, w)
--
2.43.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] trac spam
On 03/12/2023 08:49, Michael Koch wrote: Does anybody know why the last regex seach pattern in the list doesn't work? I thought it should match if any of the keywords is followed by "http" in the same line, with any number of any characters before, between and after the keywords. (?i)^.*(appreciate|amazing|great|regards|thank).*http.*$ Perhaps the regex search is performed on the rendered text as opposed to the source text? Taking the spam comment on #2104 as an example, the spammer appears to be using the `[ ]` or `[[|]]` markup, therefore `http` does not appear in the rendered text. -- https://www.frankplowman.com/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] doc/t2h: Support texinfo 7.0
On 07/11/2023 20:38, epira...@gmail.com wrote: On 7 Nov 2023, at 16:57, Frank Plowman wrote: Resolves #10636 (http://trac.ffmpeg.org/ticket/10636) Texinfo 7.0, released in November 2022, changed the names of various functions. Compiling docs with Texinfo 7.0 results in warnings and improperly formatted documentation. More old names appear to have been removed in Texinfo 7.1, released October 2023, which causes docs compilation to fail. This PR addresses the issue by adding logic to switch between the old and new function names depending on the Texinfo version. Texinfo 6.8 produces identical documentation before and after the patch. The change to the CSS makes the documentation generated by Texinfo >= 7.0 appear more similar to that generated by Texinfo <= 6.8. CC https://www.mail-archive.com/debian-bugs-dist@lists.debian.org/msg1938238.html https://bugs.gentoo.org/916104 Signed-off-by: Frank Plowman Thanks! Just had a quick look and was wondering why your patch touches the bootstrap.min.css? Texinfo 7.0 produces quite different HTML to Texinfo 6.8. Without that change to the CSS, enumerated option flags (i.e. Possible values of x are...) render as white text on a white background with Texinfo 7.0 and are unreadable. The change removes a style for the selector `.table .table` which causes the background to turn white. As far as I can tell, it is not actually used anywhere in files generated by Texinfo 6.8. -- https://www.frankplowman.com/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] doc/t2h: Support texinfo 7.0
On 05/11/2023 21:01, Stefano Sabatini wrote:
@@ -159,7 +192,18 @@ sub ffmpeg_begin_file($$$)
my ($title, $description, $encoding, $date, $css_lines,
$doctype, $bodytext, $copying_comment, $after_body_open,
$extra_head, $program_and_version, $program_homepage,
-$program, $generator) = $self->_file_header_informations($command);
+$program, $generator);
+if ($program_version_num >= 7.00) {
+($title, $description, $encoding, $date, $css_lines,
+ $doctype, $bodytext, $copying_comment, $after_body_open,
+ $extra_head, $program_and_version, $program_homepage,
+ $program, $generator) = $self->_file_header_information($command);
+} else {
+($title, $description, $encoding, $date, $css_lines,
+ $doctype, $bodytext, $copying_comment, $after_body_open,
+ $extra_head, $program_and_version, $program_homepage,
+ $program, $generator) = $self->_file_header_informations($command);
+}
nit: maybe can be refactored a bit to avoid the duplication (but my
perl is rusty and I cannot test with texinfo 7.0):
my $get_header_information_fn = $program_version_num >= 7.00 ?
$self->_file_header_information : $self->_file_header_informations;
my (...) = $get_header_information_fn($command);
[...]
I've just had a little fiddle to try get this working, and unfortunately
it looks like while you can create references to normal subroutines, you
can't easily bind to object methods in Perl. There are some workarounds
(see
https://stackoverflow.com/questions/47077879/perl-pass-object-method-reference-as-parameter-to-function),
but imo they are less readable.
This is the first Perl I've ever written so if any wizards out there
know a better way please let me know and I'd be happy to put together a v2.
Looks good otherwise, thanks.
___
Cheers,
Frank
--
https://www.frankplowman.com/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] FFmpeg 6.0.1 and 5.1.4
On 05/11/2023 14:47, Sebastian Ramacher wrote: On 2023-11-04 00:00:38 +0100, Michael Niedermayer wrote: Hi i intend to do releases from thr release/6.0 and 5.1 branches very soon if theres something you want backported, please do it now. If you want me to wait for something, just say so. I'd appreciate a proper fix forhttps://trac.ffmpeg.org/ticket/10636 for at least 6.0.1. Cheers This issue is fixed by the patch I posted earlier today: http://ffmpeg.org/pipermail/ffmpeg-devel/2023-November/316605.html All the best, Frank -- https://www.frankplowman.com/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] x86inc: Add REPX macro to repeat instructions/operations
From: Henrik Gramner
When operating on large blocks of data it's common to repeatedly use
an instruction on multiple registers. Using the REPX macro makes it
easy to quickly write dense code to achieve this without having to
explicitly duplicate the same instruction over and over.
For example,
REPX {paddw x, m4}, m0, m1, m2, m3
REPX {mova [r0+16*x], m5}, 0, 1, 2, 3
will expand to
paddw m0, m4
paddw m1, m4
paddw m2, m4
paddw m3, m4
mova [r0+16*0], m5
mova [r0+16*1], m5
mova [r0+16*2], m5
mova [r0+16*3], m5
Commit taken from x264:
https://code.videolan.org/videolan/x264/-/commit/6d10612ab0007f8f60dd2399182efd696da3ffe4
Signed-off-by: Frank Plowman
---
libavutil/x86/x86inc.asm | 10 ++
1 file changed, 10 insertions(+)
diff --git a/libavutil/x86/x86inc.asm b/libavutil/x86/x86inc.asm
index 251ee797de..e099ee4b10 100644
--- a/libavutil/x86/x86inc.asm
+++ b/libavutil/x86/x86inc.asm
@@ -232,6 +232,16 @@ DECLARE_REG_TMP_SIZE 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14
%define gprsize 4
%endif
+; Repeats an instruction/operation for multiple arguments.
+; Example usage: "REPX {psrlw x, 8}, m0, m1, m2, m3"
+%macro REPX 2-* ; operation, args
+%xdefine %%f(x) %1
+%rep %0 - 1
+%rotate 1
+%%f(%1)
+%endrep
+%endmacro
+
%macro PUSH 1
push %1
%ifidn rstk, rsp
--
2.41.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-requ...@ffmpeg.org with subject "unsubscribe".
