| Message ID | 20240320233525.29361-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/hevc_ps: allocate only the required HEVCHdrParams within a VPS | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Wed, Mar 20, 2024 at 08:35:25PM -0300, James Almer wrote: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/hevc_ps.c | 17 +++++++++++++++-- > libavcodec/hevc_ps.h | 2 +- > libavcodec/vulkan_hevc.c | 2 +- > 3 files changed, 17 insertions(+), 4 deletions(-) the 3 patches fix the timeout from (didnt test only the 3rd as it doesnt apply automatcially alone) 64033/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-5332101272305664 feel free to apply thx [...]
James Almer: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/hevc_ps.c | 17 +++++++++++++++-- > libavcodec/hevc_ps.h | 2 +- > libavcodec/vulkan_hevc.c | 2 +- > 3 files changed, 17 insertions(+), 4 deletions(-) > > diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c > index 20ceb09829..d3edc0810d 100644 > --- a/libavcodec/hevc_ps.c > +++ b/libavcodec/hevc_ps.c > @@ -442,13 +442,21 @@ static int decode_hrd(GetBitContext *gb, int common_inf_present, > return 0; > } > > +static void uninit_vps(FFRefStructOpaque opaque, void *obj) > +{ > + HEVCVPS *vps = obj; > + > + for (int i = 0; i < vps->vps_num_hrd_parameters; i++) > + ff_refstruct_unref(&vps->hdr[i]); > +} > + > int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, > HEVCParamSets *ps) > { > int i,j; > int vps_id = 0; > ptrdiff_t nal_size; > - HEVCVPS *vps = ff_refstruct_allocz(sizeof(*vps)); > + HEVCVPS *vps = ff_refstruct_alloc_ext(sizeof(*vps), 0, NULL, uninit_vps); > > if (!vps) > return AVERROR(ENOMEM); > @@ -538,12 +546,17 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, > goto err; > } > for (i = 0; i < vps->vps_num_hrd_parameters; i++) { > + HEVCHdrParams *hdr = ff_refstruct_allocz(sizeof(*hdr)); > int common_inf_present = 1; > > + if (!hdr) > + return AVERROR(ENOMEM); > + > get_ue_golomb_long(gb); // hrd_layer_set_idx > if (i) > common_inf_present = get_bits1(gb); > - decode_hrd(gb, common_inf_present, &vps->hdr[i], > + > + decode_hrd(gb, common_inf_present, hdr, > vps->vps_max_sub_layers); > } Why do you allocate the HEVCHdrParams separately when you know vps_num_hrd_parameters before allocating the first one? > } > diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h > index 9cdec9b6c1..ff94f90a5e 100644 > --- a/libavcodec/hevc_ps.h > +++ b/libavcodec/hevc_ps.h > @@ -150,7 +150,7 @@ typedef struct PTL { > > typedef struct HEVCVPS { > unsigned int vps_id; > - HEVCHdrParams hdr[HEVC_MAX_LAYER_SETS]; > + HEVCHdrParams *hdr[HEVC_MAX_LAYER_SETS]; > > uint8_t vps_temporal_id_nesting_flag : 1; > uint8_t vps_sub_layer_ordering_info_present_flag : 1; > diff --git a/libavcodec/vulkan_hevc.c b/libavcodec/vulkan_hevc.c > index 239bff75e5..a89d09a372 100644 > --- a/libavcodec/vulkan_hevc.c > +++ b/libavcodec/vulkan_hevc.c > @@ -563,7 +563,7 @@ static void set_vps(const HEVCVPS *vps, > HEVCHeaderVPSSet sls[]) > { > for (int i = 0; i < vps->vps_num_hrd_parameters; i++) { > - const HEVCHdrParams *src = &vps->hdr[i]; > + const HEVCHdrParams *src = vps->hdr[i]; > > sls_hdr[i] = (StdVideoH265HrdParameters) { > .flags = (StdVideoH265HrdFlags) {
On 3/21/2024 8:15 AM, Andreas Rheinhardt wrote: > James Almer: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/hevc_ps.c | 17 +++++++++++++++-- >> libavcodec/hevc_ps.h | 2 +- >> libavcodec/vulkan_hevc.c | 2 +- >> 3 files changed, 17 insertions(+), 4 deletions(-) >> >> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c >> index 20ceb09829..d3edc0810d 100644 >> --- a/libavcodec/hevc_ps.c >> +++ b/libavcodec/hevc_ps.c >> @@ -442,13 +442,21 @@ static int decode_hrd(GetBitContext *gb, int common_inf_present, >> return 0; >> } >> >> +static void uninit_vps(FFRefStructOpaque opaque, void *obj) >> +{ >> + HEVCVPS *vps = obj; >> + >> + for (int i = 0; i < vps->vps_num_hrd_parameters; i++) >> + ff_refstruct_unref(&vps->hdr[i]); >> +} >> + >> int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, >> HEVCParamSets *ps) >> { >> int i,j; >> int vps_id = 0; >> ptrdiff_t nal_size; >> - HEVCVPS *vps = ff_refstruct_allocz(sizeof(*vps)); >> + HEVCVPS *vps = ff_refstruct_alloc_ext(sizeof(*vps), 0, NULL, uninit_vps); >> >> if (!vps) >> return AVERROR(ENOMEM); >> @@ -538,12 +546,17 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, >> goto err; >> } >> for (i = 0; i < vps->vps_num_hrd_parameters; i++) { >> + HEVCHdrParams *hdr = ff_refstruct_allocz(sizeof(*hdr)); >> int common_inf_present = 1; >> >> + if (!hdr) >> + return AVERROR(ENOMEM); >> + >> get_ue_golomb_long(gb); // hrd_layer_set_idx >> if (i) >> common_inf_present = get_bits1(gb); >> - decode_hrd(gb, common_inf_present, &vps->hdr[i], >> + >> + decode_hrd(gb, common_inf_present, hdr, >> vps->vps_max_sub_layers); >> } > > Why do you allocate the HEVCHdrParams separately when you know > vps_num_hrd_parameters before allocating the first one? You want me to allocate all of them with a single ff_refstruct_allocz() and store individual pointers for each? > >> } >> diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h >> index 9cdec9b6c1..ff94f90a5e 100644 >> --- a/libavcodec/hevc_ps.h >> +++ b/libavcodec/hevc_ps.h >> @@ -150,7 +150,7 @@ typedef struct PTL { >> >> typedef struct HEVCVPS { >> unsigned int vps_id; >> - HEVCHdrParams hdr[HEVC_MAX_LAYER_SETS]; >> + HEVCHdrParams *hdr[HEVC_MAX_LAYER_SETS]; >> >> uint8_t vps_temporal_id_nesting_flag : 1; >> uint8_t vps_sub_layer_ordering_info_present_flag : 1; >> diff --git a/libavcodec/vulkan_hevc.c b/libavcodec/vulkan_hevc.c >> index 239bff75e5..a89d09a372 100644 >> --- a/libavcodec/vulkan_hevc.c >> +++ b/libavcodec/vulkan_hevc.c >> @@ -563,7 +563,7 @@ static void set_vps(const HEVCVPS *vps, >> HEVCHeaderVPSSet sls[]) >> { >> for (int i = 0; i < vps->vps_num_hrd_parameters; i++) { >> - const HEVCHdrParams *src = &vps->hdr[i]; >> + const HEVCHdrParams *src = vps->hdr[i]; >> >> sls_hdr[i] = (StdVideoH265HrdParameters) { >> .flags = (StdVideoH265HrdFlags) { > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
James Almer: > On 3/21/2024 8:15 AM, Andreas Rheinhardt wrote: >> James Almer: >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> libavcodec/hevc_ps.c | 17 +++++++++++++++-- >>> libavcodec/hevc_ps.h | 2 +- >>> libavcodec/vulkan_hevc.c | 2 +- >>> 3 files changed, 17 insertions(+), 4 deletions(-) >>> >>> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c >>> index 20ceb09829..d3edc0810d 100644 >>> --- a/libavcodec/hevc_ps.c >>> +++ b/libavcodec/hevc_ps.c >>> @@ -442,13 +442,21 @@ static int decode_hrd(GetBitContext *gb, int >>> common_inf_present, >>> return 0; >>> } >>> +static void uninit_vps(FFRefStructOpaque opaque, void *obj) >>> +{ >>> + HEVCVPS *vps = obj; >>> + >>> + for (int i = 0; i < vps->vps_num_hrd_parameters; i++) >>> + ff_refstruct_unref(&vps->hdr[i]); >>> +} >>> + >>> int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, >>> HEVCParamSets *ps) >>> { >>> int i,j; >>> int vps_id = 0; >>> ptrdiff_t nal_size; >>> - HEVCVPS *vps = ff_refstruct_allocz(sizeof(*vps)); >>> + HEVCVPS *vps = ff_refstruct_alloc_ext(sizeof(*vps), 0, NULL, >>> uninit_vps); >>> if (!vps) >>> return AVERROR(ENOMEM); >>> @@ -538,12 +546,17 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, >>> AVCodecContext *avctx, >>> goto err; >>> } >>> for (i = 0; i < vps->vps_num_hrd_parameters; i++) { >>> + HEVCHdrParams *hdr = ff_refstruct_allocz(sizeof(*hdr)); >>> int common_inf_present = 1; >>> + if (!hdr) >>> + return AVERROR(ENOMEM); >>> + >>> get_ue_golomb_long(gb); // hrd_layer_set_idx >>> if (i) >>> common_inf_present = get_bits1(gb); >>> - decode_hrd(gb, common_inf_present, &vps->hdr[i], >>> + >>> + decode_hrd(gb, common_inf_present, hdr, >>> vps->vps_max_sub_layers); >>> } >> >> Why do you allocate the HEVCHdrParams separately when you know >> vps_num_hrd_parameters before allocating the first one? > > You want me to allocate all of them with a single ff_refstruct_allocz() > and store individual pointers for each? > No, I want you to allocate them jointly and access them via "const HEVCHdrParams *src = &vps->hdr[i]". (Removing the individual pointers is precisely the point.) Furthermore, there is no reason to allocate it via RefStruct. A simple av_calloc() is enough (they have one owner: The containing HEVCVPS; that said HEVCVPS can have multiple owners doesn't change this). >> >>> } >>> diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h >>> index 9cdec9b6c1..ff94f90a5e 100644 >>> --- a/libavcodec/hevc_ps.h >>> +++ b/libavcodec/hevc_ps.h >>> @@ -150,7 +150,7 @@ typedef struct PTL { >>> typedef struct HEVCVPS { >>> unsigned int vps_id; >>> - HEVCHdrParams hdr[HEVC_MAX_LAYER_SETS]; >>> + HEVCHdrParams *hdr[HEVC_MAX_LAYER_SETS]; >>> uint8_t vps_temporal_id_nesting_flag : 1; >>> uint8_t vps_sub_layer_ordering_info_present_flag : 1; >>> diff --git a/libavcodec/vulkan_hevc.c b/libavcodec/vulkan_hevc.c >>> index 239bff75e5..a89d09a372 100644 >>> --- a/libavcodec/vulkan_hevc.c >>> +++ b/libavcodec/vulkan_hevc.c >>> @@ -563,7 +563,7 @@ static void set_vps(const HEVCVPS *vps, >>> HEVCHeaderVPSSet sls[]) >>> { >>> for (int i = 0; i < vps->vps_num_hrd_parameters; i++) { >>> - const HEVCHdrParams *src = &vps->hdr[i]; >>> + const HEVCHdrParams *src = vps->hdr[i]; >>> sls_hdr[i] = (StdVideoH265HrdParameters) { >>> .flags = (StdVideoH265HrdFlags) { >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c index 20ceb09829..d3edc0810d 100644 --- a/libavcodec/hevc_ps.c +++ b/libavcodec/hevc_ps.c @@ -442,13 +442,21 @@ static int decode_hrd(GetBitContext *gb, int common_inf_present, return 0; } +static void uninit_vps(FFRefStructOpaque opaque, void *obj) +{ + HEVCVPS *vps = obj; + + for (int i = 0; i < vps->vps_num_hrd_parameters; i++) + ff_refstruct_unref(&vps->hdr[i]); +} + int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, HEVCParamSets *ps) { int i,j; int vps_id = 0; ptrdiff_t nal_size; - HEVCVPS *vps = ff_refstruct_allocz(sizeof(*vps)); + HEVCVPS *vps = ff_refstruct_alloc_ext(sizeof(*vps), 0, NULL, uninit_vps); if (!vps) return AVERROR(ENOMEM); @@ -538,12 +546,17 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, goto err; } for (i = 0; i < vps->vps_num_hrd_parameters; i++) { + HEVCHdrParams *hdr = ff_refstruct_allocz(sizeof(*hdr)); int common_inf_present = 1; + if (!hdr) + return AVERROR(ENOMEM); + get_ue_golomb_long(gb); // hrd_layer_set_idx if (i) common_inf_present = get_bits1(gb); - decode_hrd(gb, common_inf_present, &vps->hdr[i], + + decode_hrd(gb, common_inf_present, hdr, vps->vps_max_sub_layers); } } diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h index 9cdec9b6c1..ff94f90a5e 100644 --- a/libavcodec/hevc_ps.h +++ b/libavcodec/hevc_ps.h @@ -150,7 +150,7 @@ typedef struct PTL { typedef struct HEVCVPS { unsigned int vps_id; - HEVCHdrParams hdr[HEVC_MAX_LAYER_SETS]; + HEVCHdrParams *hdr[HEVC_MAX_LAYER_SETS]; uint8_t vps_temporal_id_nesting_flag : 1; uint8_t vps_sub_layer_ordering_info_present_flag : 1; diff --git a/libavcodec/vulkan_hevc.c b/libavcodec/vulkan_hevc.c index 239bff75e5..a89d09a372 100644 --- a/libavcodec/vulkan_hevc.c +++ b/libavcodec/vulkan_hevc.c @@ -563,7 +563,7 @@ static void set_vps(const HEVCVPS *vps, HEVCHeaderVPSSet sls[]) { for (int i = 0; i < vps->vps_num_hrd_parameters; i++) { - const HEVCHdrParams *src = &vps->hdr[i]; + const HEVCHdrParams *src = vps->hdr[i]; sls_hdr[i] = (StdVideoH265HrdParameters) { .flags = (StdVideoH265HrdFlags) {
Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/hevc_ps.c | 17 +++++++++++++++-- libavcodec/hevc_ps.h | 2 +- libavcodec/vulkan_hevc.c | 2 +- 3 files changed, 17 insertions(+), 4 deletions(-)