* [FFmpeg-devel] [PATCH] avocdec/hevc_ps: don't use a fixed sized buffer for parameter set raw data @ 2024-03-29 15:56 James Almer 2024-03-29 16:10 ` Andreas Rheinhardt 0 siblings, 1 reply; 7+ messages in thread From: James Almer @ 2024-03-29 15:56 UTC (permalink / raw) To: ffmpeg-devel Allocate it instead, and use it to compare sets instead of the parsed struct. Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/hevc_ps.c | 84 ++++++++++++++++++++++---------------------- libavcodec/hevc_ps.h | 14 +++++--- 2 files changed, 51 insertions(+), 47 deletions(-) diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c index 6475d86d7d..e417039d76 100644 --- a/libavcodec/hevc_ps.c +++ b/libavcodec/hevc_ps.c @@ -442,20 +442,18 @@ static int decode_hrd(GetBitContext *gb, int common_inf_present, return 0; } -static void uninit_vps(FFRefStructOpaque opaque, void *obj) +static void hevc_vps_free(FFRefStructOpaque opaque, void *obj) { HEVCVPS *vps = obj; av_freep(&vps->hdr); + av_freep(&vps->data); } static int compare_vps(const HEVCVPS *vps1, const HEVCVPS *vps2) { - if (!memcmp(vps1, vps2, offsetof(HEVCVPS, hdr))) - return !vps1->vps_num_hrd_parameters || - !memcmp(vps1->hdr, vps2->hdr, vps1->vps_num_hrd_parameters * sizeof(*vps1->hdr)); - - return 0; + return vps1->data_size == vps2->data_size && + !memcmp(vps1->data, vps2->data, vps1->data_size); } int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, @@ -463,25 +461,20 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, { int i,j; int vps_id = 0; - ptrdiff_t nal_size; - HEVCVPS *vps = ff_refstruct_alloc_ext(sizeof(*vps), 0, NULL, uninit_vps); + int ret = AVERROR_INVALIDDATA; + HEVCVPS *vps = ff_refstruct_alloc_ext(sizeof(*vps), 0, NULL, hevc_vps_free); if (!vps) return AVERROR(ENOMEM); av_log(avctx, AV_LOG_DEBUG, "Decoding VPS\n"); - nal_size = gb->buffer_end - gb->buffer; - if (nal_size > sizeof(vps->data)) { - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized VPS " - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", - nal_size, sizeof(vps->data)); - vps->data_size = sizeof(vps->data); - } else { - vps->data_size = nal_size; + vps->data_size = gb->buffer_end - gb->buffer; + vps->data = av_memdup(gb->buffer, vps->data_size); + if (!vps->data) { + ret = AVERROR(ENOMEM); + goto err; } - memcpy(vps->data, gb->buffer, vps->data_size); - vps_id = vps->vps_id = get_bits(gb, 4); if (get_bits(gb, 2) != 3) { // vps_reserved_three_2bits @@ -591,7 +584,7 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, err: ff_refstruct_unref(&vps); - return AVERROR_INVALIDDATA; + return ret; } static void decode_vui(GetBitContext *gb, AVCodecContext *avctx, @@ -1294,36 +1287,43 @@ int ff_hevc_parse_sps(HEVCSPS *sps, GetBitContext *gb, unsigned int *sps_id, return 0; } +static void hevc_sps_free(FFRefStructOpaque opaque, void *obj) +{ + HEVCSPS *sps = obj; + + av_freep(&sps->data); +} + +static int compare_sps(const HEVCSPS *sps1, const HEVCSPS *sps2) +{ + return sps1->data_size == sps2->data_size && + !memcmp(sps1->data, sps2->data, sps1->data_size); +} + int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, HEVCParamSets *ps, int apply_defdispwin) { - HEVCSPS *sps = ff_refstruct_allocz(sizeof(*sps)); + HEVCSPS *sps = ff_refstruct_alloc_ext(sizeof(*sps), 0, NULL, hevc_sps_free); unsigned int sps_id; int ret; - ptrdiff_t nal_size; if (!sps) return AVERROR(ENOMEM); av_log(avctx, AV_LOG_DEBUG, "Decoding SPS\n"); - nal_size = gb->buffer_end - gb->buffer; - if (nal_size > sizeof(sps->data)) { - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized SPS " - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", - nal_size, sizeof(sps->data)); - sps->data_size = sizeof(sps->data); - } else { - sps->data_size = nal_size; + sps->data_size = gb->buffer_end - gb->buffer; + sps->data = av_memdup(gb->buffer, sps->data_size); + if (!sps->data) { + ret = AVERROR(ENOMEM); + goto err; } - memcpy(sps->data, gb->buffer, sps->data_size); ret = ff_hevc_parse_sps(sps, gb, &sps_id, apply_defdispwin, ps->vps_list, avctx); if (ret < 0) { - ff_refstruct_unref(&sps); - return ret; + goto err; } if (avctx->debug & FF_DEBUG_BITSTREAM) { @@ -1340,7 +1340,7 @@ int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, * original one. * otherwise drop all PPSes that depend on it */ if (ps->sps_list[sps_id] && - !memcmp(ps->sps_list[sps_id], sps, sizeof(*sps))) { + compare_sps(ps->sps_list[sps_id], sps)) { ff_refstruct_unref(&sps); } else { remove_sps(ps, sps_id); @@ -1348,6 +1348,9 @@ int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, } return 0; +err: + ff_refstruct_unref(&sps); + return ret; } static void hevc_pps_free(FFRefStructOpaque unused, void *obj) @@ -1364,6 +1367,7 @@ static void hevc_pps_free(FFRefStructOpaque unused, void *obj) av_freep(&pps->tile_pos_rs); av_freep(&pps->tile_id); av_freep(&pps->min_tb_addr_zs_tab); + av_freep(&pps->data); } static void colour_mapping_octants(GetBitContext *gb, HEVCPPS *pps, int inp_depth, @@ -1773,16 +1777,12 @@ int ff_hevc_decode_nal_pps(GetBitContext *gb, AVCodecContext *avctx, av_log(avctx, AV_LOG_DEBUG, "Decoding PPS\n"); - nal_size = gb->buffer_end - gb->buffer; - if (nal_size > sizeof(pps->data)) { - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized PPS " - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", - nal_size, sizeof(pps->data)); - pps->data_size = sizeof(pps->data); - } else { - pps->data_size = nal_size; + pps->data_size = gb->buffer_end - gb->buffer; + pps->data = av_memdup(gb->buffer, pps->data_size); + if (!pps->data) { + ret = AVERROR_INVALIDDATA; + goto err; } - memcpy(pps->data, gb->buffer, pps->data_size); // Default values pps->loop_filter_across_tiles_enabled_flag = 1; diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h index 0d8eaf2b3e..bed406770f 100644 --- a/libavcodec/hevc_ps.h +++ b/libavcodec/hevc_ps.h @@ -172,11 +172,11 @@ typedef struct HEVCVPS { int vps_num_ticks_poc_diff_one; ///< vps_num_ticks_poc_diff_one_minus1 + 1 int vps_num_hrd_parameters; - uint8_t data[4096]; - int data_size; - /* Put this at the end of the structure to make it easier to calculate the + /* Keep this at the end of the structure to make it easier to calculate the * size before this pointer, which is used for memcmp */ HEVCHdrParams *hdr; + uint8_t *data; + int data_size; } HEVCVPS; typedef struct ScalingList { @@ -299,7 +299,9 @@ typedef struct HEVCSPS { int qp_bd_offset; - uint8_t data[4096]; + /* Keep this at the end of the structure to make it easier to calculate the + * size before this pointer, which is used for memcmp */ + uint8_t *data; int data_size; } HEVCSPS; @@ -434,7 +436,9 @@ typedef struct HEVCPPS { int *min_tb_addr_zs; ///< MinTbAddrZS int *min_tb_addr_zs_tab;///< MinTbAddrZS - uint8_t data[4096]; + /* Keep this at the end of the structure to make it easier to calculate the + * size before this pointer, which is used for memcmp */ + uint8_t *data; int data_size; } HEVCPPS; -- 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-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avocdec/hevc_ps: don't use a fixed sized buffer for parameter set raw data 2024-03-29 15:56 [FFmpeg-devel] [PATCH] avocdec/hevc_ps: don't use a fixed sized buffer for parameter set raw data James Almer @ 2024-03-29 16:10 ` Andreas Rheinhardt 2024-03-29 16:17 ` James Almer 0 siblings, 1 reply; 7+ messages in thread From: Andreas Rheinhardt @ 2024-03-29 16:10 UTC (permalink / raw) To: ffmpeg-devel James Almer: > Allocate it instead, and use it to compare sets instead of the parsed struct. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/hevc_ps.c | 84 ++++++++++++++++++++++---------------------- > libavcodec/hevc_ps.h | 14 +++++--- > 2 files changed, 51 insertions(+), 47 deletions(-) > > diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c > index 6475d86d7d..e417039d76 100644 > --- a/libavcodec/hevc_ps.c > +++ b/libavcodec/hevc_ps.c > @@ -442,20 +442,18 @@ static int decode_hrd(GetBitContext *gb, int common_inf_present, > return 0; > } > > -static void uninit_vps(FFRefStructOpaque opaque, void *obj) > +static void hevc_vps_free(FFRefStructOpaque opaque, void *obj) > { > HEVCVPS *vps = obj; > > av_freep(&vps->hdr); > + av_freep(&vps->data); > } > > static int compare_vps(const HEVCVPS *vps1, const HEVCVPS *vps2) > { > - if (!memcmp(vps1, vps2, offsetof(HEVCVPS, hdr))) > - return !vps1->vps_num_hrd_parameters || > - !memcmp(vps1->hdr, vps2->hdr, vps1->vps_num_hrd_parameters * sizeof(*vps1->hdr)); > - > - return 0; > + return vps1->data_size == vps2->data_size && > + !memcmp(vps1->data, vps2->data, vps1->data_size); > } > > int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, > @@ -463,25 +461,20 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, > { > int i,j; > int vps_id = 0; > - ptrdiff_t nal_size; > - HEVCVPS *vps = ff_refstruct_alloc_ext(sizeof(*vps), 0, NULL, uninit_vps); > + int ret = AVERROR_INVALIDDATA; > + HEVCVPS *vps = ff_refstruct_alloc_ext(sizeof(*vps), 0, NULL, hevc_vps_free); > > if (!vps) > return AVERROR(ENOMEM); > > av_log(avctx, AV_LOG_DEBUG, "Decoding VPS\n"); > > - nal_size = gb->buffer_end - gb->buffer; > - if (nal_size > sizeof(vps->data)) { > - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized VPS " > - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", > - nal_size, sizeof(vps->data)); > - vps->data_size = sizeof(vps->data); > - } else { > - vps->data_size = nal_size; > + vps->data_size = gb->buffer_end - gb->buffer; > + vps->data = av_memdup(gb->buffer, vps->data_size); > + if (!vps->data) { > + ret = AVERROR(ENOMEM); > + goto err; > } > - memcpy(vps->data, gb->buffer, vps->data_size); > - > vps_id = vps->vps_id = get_bits(gb, 4); > > if (get_bits(gb, 2) != 3) { // vps_reserved_three_2bits > @@ -591,7 +584,7 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, > > err: > ff_refstruct_unref(&vps); > - return AVERROR_INVALIDDATA; > + return ret; > } > > static void decode_vui(GetBitContext *gb, AVCodecContext *avctx, > @@ -1294,36 +1287,43 @@ int ff_hevc_parse_sps(HEVCSPS *sps, GetBitContext *gb, unsigned int *sps_id, > return 0; > } > > +static void hevc_sps_free(FFRefStructOpaque opaque, void *obj) > +{ > + HEVCSPS *sps = obj; > + > + av_freep(&sps->data); > +} > + > +static int compare_sps(const HEVCSPS *sps1, const HEVCSPS *sps2) > +{ > + return sps1->data_size == sps2->data_size && > + !memcmp(sps1->data, sps2->data, sps1->data_size); > +} > + > int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, > HEVCParamSets *ps, int apply_defdispwin) > { > - HEVCSPS *sps = ff_refstruct_allocz(sizeof(*sps)); > + HEVCSPS *sps = ff_refstruct_alloc_ext(sizeof(*sps), 0, NULL, hevc_sps_free); > unsigned int sps_id; > int ret; > - ptrdiff_t nal_size; > > if (!sps) > return AVERROR(ENOMEM); > > av_log(avctx, AV_LOG_DEBUG, "Decoding SPS\n"); > > - nal_size = gb->buffer_end - gb->buffer; > - if (nal_size > sizeof(sps->data)) { > - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized SPS " > - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", > - nal_size, sizeof(sps->data)); > - sps->data_size = sizeof(sps->data); > - } else { > - sps->data_size = nal_size; > + sps->data_size = gb->buffer_end - gb->buffer; > + sps->data = av_memdup(gb->buffer, sps->data_size); > + if (!sps->data) { > + ret = AVERROR(ENOMEM); > + goto err; > } > - memcpy(sps->data, gb->buffer, sps->data_size); > > ret = ff_hevc_parse_sps(sps, gb, &sps_id, > apply_defdispwin, > ps->vps_list, avctx); > if (ret < 0) { > - ff_refstruct_unref(&sps); > - return ret; > + goto err; > } > > if (avctx->debug & FF_DEBUG_BITSTREAM) { > @@ -1340,7 +1340,7 @@ int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, > * original one. > * otherwise drop all PPSes that depend on it */ > if (ps->sps_list[sps_id] && > - !memcmp(ps->sps_list[sps_id], sps, sizeof(*sps))) { > + compare_sps(ps->sps_list[sps_id], sps)) { > ff_refstruct_unref(&sps); > } else { > remove_sps(ps, sps_id); > @@ -1348,6 +1348,9 @@ int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, > } > > return 0; > +err: > + ff_refstruct_unref(&sps); > + return ret; > } > > static void hevc_pps_free(FFRefStructOpaque unused, void *obj) > @@ -1364,6 +1367,7 @@ static void hevc_pps_free(FFRefStructOpaque unused, void *obj) > av_freep(&pps->tile_pos_rs); > av_freep(&pps->tile_id); > av_freep(&pps->min_tb_addr_zs_tab); > + av_freep(&pps->data); > } > > static void colour_mapping_octants(GetBitContext *gb, HEVCPPS *pps, int inp_depth, > @@ -1773,16 +1777,12 @@ int ff_hevc_decode_nal_pps(GetBitContext *gb, AVCodecContext *avctx, > > av_log(avctx, AV_LOG_DEBUG, "Decoding PPS\n"); > > - nal_size = gb->buffer_end - gb->buffer; > - if (nal_size > sizeof(pps->data)) { > - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized PPS " > - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", > - nal_size, sizeof(pps->data)); > - pps->data_size = sizeof(pps->data); > - } else { > - pps->data_size = nal_size; > + pps->data_size = gb->buffer_end - gb->buffer; > + pps->data = av_memdup(gb->buffer, pps->data_size); > + if (!pps->data) { > + ret = AVERROR_INVALIDDATA; > + goto err; > } > - memcpy(pps->data, gb->buffer, pps->data_size); > > // Default values > pps->loop_filter_across_tiles_enabled_flag = 1; > diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h > index 0d8eaf2b3e..bed406770f 100644 > --- a/libavcodec/hevc_ps.h > +++ b/libavcodec/hevc_ps.h > @@ -172,11 +172,11 @@ typedef struct HEVCVPS { > int vps_num_ticks_poc_diff_one; ///< vps_num_ticks_poc_diff_one_minus1 + 1 > int vps_num_hrd_parameters; > > - uint8_t data[4096]; > - int data_size; > - /* Put this at the end of the structure to make it easier to calculate the > + /* Keep this at the end of the structure to make it easier to calculate the > * size before this pointer, which is used for memcmp */ > HEVCHdrParams *hdr; > + uint8_t *data; > + int data_size; > } HEVCVPS; > > typedef struct ScalingList { > @@ -299,7 +299,9 @@ typedef struct HEVCSPS { > > int qp_bd_offset; > > - uint8_t data[4096]; > + /* Keep this at the end of the structure to make it easier to calculate the > + * size before this pointer, which is used for memcmp */ > + uint8_t *data; > int data_size; > } HEVCSPS; > > @@ -434,7 +436,9 @@ typedef struct HEVCPPS { > int *min_tb_addr_zs; ///< MinTbAddrZS > int *min_tb_addr_zs_tab;///< MinTbAddrZS > > - uint8_t data[4096]; > + /* Keep this at the end of the structure to make it easier to calculate the > + * size before this pointer, which is used for memcmp */ > + uint8_t *data; > int data_size; > } HEVCPPS; > This is vastly overcomplicated: If you already have the complete data of the earlier PS, all you need is comparing the data before you even parse the new parameter set. - Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avocdec/hevc_ps: don't use a fixed sized buffer for parameter set raw data 2024-03-29 16:10 ` Andreas Rheinhardt @ 2024-03-29 16:17 ` James Almer 2024-03-29 16:31 ` Andreas Rheinhardt 0 siblings, 1 reply; 7+ messages in thread From: James Almer @ 2024-03-29 16:17 UTC (permalink / raw) To: ffmpeg-devel On 3/29/2024 1:10 PM, Andreas Rheinhardt wrote: > James Almer: >> Allocate it instead, and use it to compare sets instead of the parsed struct. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/hevc_ps.c | 84 ++++++++++++++++++++++---------------------- >> libavcodec/hevc_ps.h | 14 +++++--- >> 2 files changed, 51 insertions(+), 47 deletions(-) >> >> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c >> index 6475d86d7d..e417039d76 100644 >> --- a/libavcodec/hevc_ps.c >> +++ b/libavcodec/hevc_ps.c >> @@ -442,20 +442,18 @@ static int decode_hrd(GetBitContext *gb, int common_inf_present, >> return 0; >> } >> >> -static void uninit_vps(FFRefStructOpaque opaque, void *obj) >> +static void hevc_vps_free(FFRefStructOpaque opaque, void *obj) >> { >> HEVCVPS *vps = obj; >> >> av_freep(&vps->hdr); >> + av_freep(&vps->data); >> } >> >> static int compare_vps(const HEVCVPS *vps1, const HEVCVPS *vps2) >> { >> - if (!memcmp(vps1, vps2, offsetof(HEVCVPS, hdr))) >> - return !vps1->vps_num_hrd_parameters || >> - !memcmp(vps1->hdr, vps2->hdr, vps1->vps_num_hrd_parameters * sizeof(*vps1->hdr)); >> - >> - return 0; >> + return vps1->data_size == vps2->data_size && >> + !memcmp(vps1->data, vps2->data, vps1->data_size); >> } >> >> int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, >> @@ -463,25 +461,20 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, >> { >> int i,j; >> int vps_id = 0; >> - ptrdiff_t nal_size; >> - HEVCVPS *vps = ff_refstruct_alloc_ext(sizeof(*vps), 0, NULL, uninit_vps); >> + int ret = AVERROR_INVALIDDATA; >> + HEVCVPS *vps = ff_refstruct_alloc_ext(sizeof(*vps), 0, NULL, hevc_vps_free); >> >> if (!vps) >> return AVERROR(ENOMEM); >> >> av_log(avctx, AV_LOG_DEBUG, "Decoding VPS\n"); >> >> - nal_size = gb->buffer_end - gb->buffer; >> - if (nal_size > sizeof(vps->data)) { >> - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized VPS " >> - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", >> - nal_size, sizeof(vps->data)); >> - vps->data_size = sizeof(vps->data); >> - } else { >> - vps->data_size = nal_size; >> + vps->data_size = gb->buffer_end - gb->buffer; >> + vps->data = av_memdup(gb->buffer, vps->data_size); >> + if (!vps->data) { >> + ret = AVERROR(ENOMEM); >> + goto err; >> } >> - memcpy(vps->data, gb->buffer, vps->data_size); >> - >> vps_id = vps->vps_id = get_bits(gb, 4); >> >> if (get_bits(gb, 2) != 3) { // vps_reserved_three_2bits >> @@ -591,7 +584,7 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, >> >> err: >> ff_refstruct_unref(&vps); >> - return AVERROR_INVALIDDATA; >> + return ret; >> } >> >> static void decode_vui(GetBitContext *gb, AVCodecContext *avctx, >> @@ -1294,36 +1287,43 @@ int ff_hevc_parse_sps(HEVCSPS *sps, GetBitContext *gb, unsigned int *sps_id, >> return 0; >> } >> >> +static void hevc_sps_free(FFRefStructOpaque opaque, void *obj) >> +{ >> + HEVCSPS *sps = obj; >> + >> + av_freep(&sps->data); >> +} >> + >> +static int compare_sps(const HEVCSPS *sps1, const HEVCSPS *sps2) >> +{ >> + return sps1->data_size == sps2->data_size && >> + !memcmp(sps1->data, sps2->data, sps1->data_size); >> +} >> + >> int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, >> HEVCParamSets *ps, int apply_defdispwin) >> { >> - HEVCSPS *sps = ff_refstruct_allocz(sizeof(*sps)); >> + HEVCSPS *sps = ff_refstruct_alloc_ext(sizeof(*sps), 0, NULL, hevc_sps_free); >> unsigned int sps_id; >> int ret; >> - ptrdiff_t nal_size; >> >> if (!sps) >> return AVERROR(ENOMEM); >> >> av_log(avctx, AV_LOG_DEBUG, "Decoding SPS\n"); >> >> - nal_size = gb->buffer_end - gb->buffer; >> - if (nal_size > sizeof(sps->data)) { >> - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized SPS " >> - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", >> - nal_size, sizeof(sps->data)); >> - sps->data_size = sizeof(sps->data); >> - } else { >> - sps->data_size = nal_size; >> + sps->data_size = gb->buffer_end - gb->buffer; >> + sps->data = av_memdup(gb->buffer, sps->data_size); >> + if (!sps->data) { >> + ret = AVERROR(ENOMEM); >> + goto err; >> } >> - memcpy(sps->data, gb->buffer, sps->data_size); >> >> ret = ff_hevc_parse_sps(sps, gb, &sps_id, >> apply_defdispwin, >> ps->vps_list, avctx); >> if (ret < 0) { >> - ff_refstruct_unref(&sps); >> - return ret; >> + goto err; >> } >> >> if (avctx->debug & FF_DEBUG_BITSTREAM) { >> @@ -1340,7 +1340,7 @@ int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, >> * original one. >> * otherwise drop all PPSes that depend on it */ >> if (ps->sps_list[sps_id] && >> - !memcmp(ps->sps_list[sps_id], sps, sizeof(*sps))) { >> + compare_sps(ps->sps_list[sps_id], sps)) { >> ff_refstruct_unref(&sps); >> } else { >> remove_sps(ps, sps_id); >> @@ -1348,6 +1348,9 @@ int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, >> } >> >> return 0; >> +err: >> + ff_refstruct_unref(&sps); >> + return ret; >> } >> >> static void hevc_pps_free(FFRefStructOpaque unused, void *obj) >> @@ -1364,6 +1367,7 @@ static void hevc_pps_free(FFRefStructOpaque unused, void *obj) >> av_freep(&pps->tile_pos_rs); >> av_freep(&pps->tile_id); >> av_freep(&pps->min_tb_addr_zs_tab); >> + av_freep(&pps->data); >> } >> >> static void colour_mapping_octants(GetBitContext *gb, HEVCPPS *pps, int inp_depth, >> @@ -1773,16 +1777,12 @@ int ff_hevc_decode_nal_pps(GetBitContext *gb, AVCodecContext *avctx, >> >> av_log(avctx, AV_LOG_DEBUG, "Decoding PPS\n"); >> >> - nal_size = gb->buffer_end - gb->buffer; >> - if (nal_size > sizeof(pps->data)) { >> - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized PPS " >> - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", >> - nal_size, sizeof(pps->data)); >> - pps->data_size = sizeof(pps->data); >> - } else { >> - pps->data_size = nal_size; >> + pps->data_size = gb->buffer_end - gb->buffer; >> + pps->data = av_memdup(gb->buffer, pps->data_size); >> + if (!pps->data) { >> + ret = AVERROR_INVALIDDATA; >> + goto err; >> } >> - memcpy(pps->data, gb->buffer, pps->data_size); >> >> // Default values >> pps->loop_filter_across_tiles_enabled_flag = 1; >> diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h >> index 0d8eaf2b3e..bed406770f 100644 >> --- a/libavcodec/hevc_ps.h >> +++ b/libavcodec/hevc_ps.h >> @@ -172,11 +172,11 @@ typedef struct HEVCVPS { >> int vps_num_ticks_poc_diff_one; ///< vps_num_ticks_poc_diff_one_minus1 + 1 >> int vps_num_hrd_parameters; >> >> - uint8_t data[4096]; >> - int data_size; >> - /* Put this at the end of the structure to make it easier to calculate the >> + /* Keep this at the end of the structure to make it easier to calculate the >> * size before this pointer, which is used for memcmp */ >> HEVCHdrParams *hdr; >> + uint8_t *data; >> + int data_size; >> } HEVCVPS; >> >> typedef struct ScalingList { >> @@ -299,7 +299,9 @@ typedef struct HEVCSPS { >> >> int qp_bd_offset; >> >> - uint8_t data[4096]; >> + /* Keep this at the end of the structure to make it easier to calculate the >> + * size before this pointer, which is used for memcmp */ >> + uint8_t *data; >> int data_size; >> } HEVCSPS; >> >> @@ -434,7 +436,9 @@ typedef struct HEVCPPS { >> int *min_tb_addr_zs; ///< MinTbAddrZS >> int *min_tb_addr_zs_tab;///< MinTbAddrZS >> >> - uint8_t data[4096]; >> + /* Keep this at the end of the structure to make it easier to calculate the >> + * size before this pointer, which is used for memcmp */ >> + uint8_t *data; >> int data_size; >> } HEVCPPS; >> > > This is vastly overcomplicated: If you already have the complete data of > the earlier PS, all you need is comparing the data before you even parse > the new parameter set. Need to get sps_id from the new sps buffer, which requires calling ff_hevc_parse_sps(). Same for pps to get pps_id. Only vps has its id as the very first element. _______________________________________________ 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". ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avocdec/hevc_ps: don't use a fixed sized buffer for parameter set raw data 2024-03-29 16:17 ` James Almer @ 2024-03-29 16:31 ` Andreas Rheinhardt 2024-03-29 16:59 ` [FFmpeg-devel] [PATCH v2] " James Almer 0 siblings, 1 reply; 7+ messages in thread From: Andreas Rheinhardt @ 2024-03-29 16:31 UTC (permalink / raw) To: ffmpeg-devel James Almer: > On 3/29/2024 1:10 PM, Andreas Rheinhardt wrote: >> James Almer: >>> Allocate it instead, and use it to compare sets instead of the parsed >>> struct. >>> >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> libavcodec/hevc_ps.c | 84 ++++++++++++++++++++++---------------------- >>> libavcodec/hevc_ps.h | 14 +++++--- >>> 2 files changed, 51 insertions(+), 47 deletions(-) >>> >>> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c >>> index 6475d86d7d..e417039d76 100644 >>> --- a/libavcodec/hevc_ps.c >>> +++ b/libavcodec/hevc_ps.c >>> @@ -442,20 +442,18 @@ static int decode_hrd(GetBitContext *gb, int >>> common_inf_present, >>> return 0; >>> } >>> -static void uninit_vps(FFRefStructOpaque opaque, void *obj) >>> +static void hevc_vps_free(FFRefStructOpaque opaque, void *obj) >>> { >>> HEVCVPS *vps = obj; >>> av_freep(&vps->hdr); >>> + av_freep(&vps->data); >>> } >>> static int compare_vps(const HEVCVPS *vps1, const HEVCVPS *vps2) >>> { >>> - if (!memcmp(vps1, vps2, offsetof(HEVCVPS, hdr))) >>> - return !vps1->vps_num_hrd_parameters || >>> - !memcmp(vps1->hdr, vps2->hdr, >>> vps1->vps_num_hrd_parameters * sizeof(*vps1->hdr)); >>> - >>> - return 0; >>> + return vps1->data_size == vps2->data_size && >>> + !memcmp(vps1->data, vps2->data, vps1->data_size); >>> } >>> int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, >>> @@ -463,25 +461,20 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, >>> AVCodecContext *avctx, >>> { >>> int i,j; >>> int vps_id = 0; >>> - ptrdiff_t nal_size; >>> - HEVCVPS *vps = ff_refstruct_alloc_ext(sizeof(*vps), 0, NULL, >>> uninit_vps); >>> + int ret = AVERROR_INVALIDDATA; >>> + HEVCVPS *vps = ff_refstruct_alloc_ext(sizeof(*vps), 0, NULL, >>> hevc_vps_free); >>> if (!vps) >>> return AVERROR(ENOMEM); >>> av_log(avctx, AV_LOG_DEBUG, "Decoding VPS\n"); >>> - nal_size = gb->buffer_end - gb->buffer; >>> - if (nal_size > sizeof(vps->data)) { >>> - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized >>> VPS " >>> - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", >>> - nal_size, sizeof(vps->data)); >>> - vps->data_size = sizeof(vps->data); >>> - } else { >>> - vps->data_size = nal_size; >>> + vps->data_size = gb->buffer_end - gb->buffer; >>> + vps->data = av_memdup(gb->buffer, vps->data_size); >>> + if (!vps->data) { >>> + ret = AVERROR(ENOMEM); >>> + goto err; >>> } >>> - memcpy(vps->data, gb->buffer, vps->data_size); >>> - >>> vps_id = vps->vps_id = get_bits(gb, 4); >>> if (get_bits(gb, 2) != 3) { // vps_reserved_three_2bits >>> @@ -591,7 +584,7 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, >>> AVCodecContext *avctx, >>> err: >>> ff_refstruct_unref(&vps); >>> - return AVERROR_INVALIDDATA; >>> + return ret; >>> } >>> static void decode_vui(GetBitContext *gb, AVCodecContext *avctx, >>> @@ -1294,36 +1287,43 @@ int ff_hevc_parse_sps(HEVCSPS *sps, >>> GetBitContext *gb, unsigned int *sps_id, >>> return 0; >>> } >>> +static void hevc_sps_free(FFRefStructOpaque opaque, void *obj) >>> +{ >>> + HEVCSPS *sps = obj; >>> + >>> + av_freep(&sps->data); >>> +} >>> + >>> +static int compare_sps(const HEVCSPS *sps1, const HEVCSPS *sps2) >>> +{ >>> + return sps1->data_size == sps2->data_size && >>> + !memcmp(sps1->data, sps2->data, sps1->data_size); >>> +} >>> + >>> int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, >>> HEVCParamSets *ps, int apply_defdispwin) >>> { >>> - HEVCSPS *sps = ff_refstruct_allocz(sizeof(*sps)); >>> + HEVCSPS *sps = ff_refstruct_alloc_ext(sizeof(*sps), 0, NULL, >>> hevc_sps_free); >>> unsigned int sps_id; >>> int ret; >>> - ptrdiff_t nal_size; >>> if (!sps) >>> return AVERROR(ENOMEM); >>> av_log(avctx, AV_LOG_DEBUG, "Decoding SPS\n"); >>> - nal_size = gb->buffer_end - gb->buffer; >>> - if (nal_size > sizeof(sps->data)) { >>> - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized >>> SPS " >>> - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", >>> - nal_size, sizeof(sps->data)); >>> - sps->data_size = sizeof(sps->data); >>> - } else { >>> - sps->data_size = nal_size; >>> + sps->data_size = gb->buffer_end - gb->buffer; >>> + sps->data = av_memdup(gb->buffer, sps->data_size); >>> + if (!sps->data) { >>> + ret = AVERROR(ENOMEM); >>> + goto err; >>> } >>> - memcpy(sps->data, gb->buffer, sps->data_size); >>> ret = ff_hevc_parse_sps(sps, gb, &sps_id, >>> apply_defdispwin, >>> ps->vps_list, avctx); >>> if (ret < 0) { >>> - ff_refstruct_unref(&sps); >>> - return ret; >>> + goto err; >>> } >>> if (avctx->debug & FF_DEBUG_BITSTREAM) { >>> @@ -1340,7 +1340,7 @@ int ff_hevc_decode_nal_sps(GetBitContext *gb, >>> AVCodecContext *avctx, >>> * original one. >>> * otherwise drop all PPSes that depend on it */ >>> if (ps->sps_list[sps_id] && >>> - !memcmp(ps->sps_list[sps_id], sps, sizeof(*sps))) { >>> + compare_sps(ps->sps_list[sps_id], sps)) { >>> ff_refstruct_unref(&sps); >>> } else { >>> remove_sps(ps, sps_id); >>> @@ -1348,6 +1348,9 @@ int ff_hevc_decode_nal_sps(GetBitContext *gb, >>> AVCodecContext *avctx, >>> } >>> return 0; >>> +err: >>> + ff_refstruct_unref(&sps); >>> + return ret; >>> } >>> static void hevc_pps_free(FFRefStructOpaque unused, void *obj) >>> @@ -1364,6 +1367,7 @@ static void hevc_pps_free(FFRefStructOpaque >>> unused, void *obj) >>> av_freep(&pps->tile_pos_rs); >>> av_freep(&pps->tile_id); >>> av_freep(&pps->min_tb_addr_zs_tab); >>> + av_freep(&pps->data); >>> } >>> static void colour_mapping_octants(GetBitContext *gb, HEVCPPS >>> *pps, int inp_depth, >>> @@ -1773,16 +1777,12 @@ int ff_hevc_decode_nal_pps(GetBitContext *gb, >>> AVCodecContext *avctx, >>> av_log(avctx, AV_LOG_DEBUG, "Decoding PPS\n"); >>> - nal_size = gb->buffer_end - gb->buffer; >>> - if (nal_size > sizeof(pps->data)) { >>> - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized >>> PPS " >>> - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", >>> - nal_size, sizeof(pps->data)); >>> - pps->data_size = sizeof(pps->data); >>> - } else { >>> - pps->data_size = nal_size; >>> + pps->data_size = gb->buffer_end - gb->buffer; >>> + pps->data = av_memdup(gb->buffer, pps->data_size); >>> + if (!pps->data) { >>> + ret = AVERROR_INVALIDDATA; >>> + goto err; >>> } >>> - memcpy(pps->data, gb->buffer, pps->data_size); >>> // Default values >>> pps->loop_filter_across_tiles_enabled_flag = 1; >>> diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h >>> index 0d8eaf2b3e..bed406770f 100644 >>> --- a/libavcodec/hevc_ps.h >>> +++ b/libavcodec/hevc_ps.h >>> @@ -172,11 +172,11 @@ typedef struct HEVCVPS { >>> int vps_num_ticks_poc_diff_one; ///< >>> vps_num_ticks_poc_diff_one_minus1 + 1 >>> int vps_num_hrd_parameters; >>> - uint8_t data[4096]; >>> - int data_size; >>> - /* Put this at the end of the structure to make it easier to >>> calculate the >>> + /* Keep this at the end of the structure to make it easier to >>> calculate the >>> * size before this pointer, which is used for memcmp */ >>> HEVCHdrParams *hdr; >>> + uint8_t *data; >>> + int data_size; >>> } HEVCVPS; >>> typedef struct ScalingList { >>> @@ -299,7 +299,9 @@ typedef struct HEVCSPS { >>> int qp_bd_offset; >>> - uint8_t data[4096]; >>> + /* Keep this at the end of the structure to make it easier to >>> calculate the >>> + * size before this pointer, which is used for memcmp */ >>> + uint8_t *data; >>> int data_size; >>> } HEVCSPS; >>> @@ -434,7 +436,9 @@ typedef struct HEVCPPS { >>> int *min_tb_addr_zs; ///< MinTbAddrZS >>> int *min_tb_addr_zs_tab;///< MinTbAddrZS >>> - uint8_t data[4096]; >>> + /* Keep this at the end of the structure to make it easier to >>> calculate the >>> + * size before this pointer, which is used for memcmp */ >>> + uint8_t *data; >>> int data_size; >>> } HEVCPPS; >>> >> >> This is vastly overcomplicated: If you already have the complete data of >> the earlier PS, all you need is comparing the data before you even parse >> the new parameter set. > > Need to get sps_id from the new sps buffer, which requires calling > ff_hevc_parse_sps(). Same for pps to get pps_id. Only vps has its id as > the very first element. No. SPS is the only exception. - Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 7+ messages in thread
* [FFmpeg-devel] [PATCH v2] avocdec/hevc_ps: don't use a fixed sized buffer for parameter set raw data 2024-03-29 16:31 ` Andreas Rheinhardt @ 2024-03-29 16:59 ` James Almer 2024-03-29 17:28 ` Andreas Rheinhardt 0 siblings, 1 reply; 7+ messages in thread From: James Almer @ 2024-03-29 16:59 UTC (permalink / raw) To: ffmpeg-devel Allocate it instead, and use it to compare sets instead of the parsed struct. Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/hevc_ps.c | 127 +++++++++++++++++++++++-------------------- libavcodec/hevc_ps.h | 14 +++-- 2 files changed, 77 insertions(+), 64 deletions(-) diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c index 6475d86d7d..7f74553fc1 100644 --- a/libavcodec/hevc_ps.c +++ b/libavcodec/hevc_ps.c @@ -442,47 +442,42 @@ static int decode_hrd(GetBitContext *gb, int common_inf_present, return 0; } -static void uninit_vps(FFRefStructOpaque opaque, void *obj) +static void hevc_vps_free(FFRefStructOpaque opaque, void *obj) { HEVCVPS *vps = obj; av_freep(&vps->hdr); -} - -static int compare_vps(const HEVCVPS *vps1, const HEVCVPS *vps2) -{ - if (!memcmp(vps1, vps2, offsetof(HEVCVPS, hdr))) - return !vps1->vps_num_hrd_parameters || - !memcmp(vps1->hdr, vps2->hdr, vps1->vps_num_hrd_parameters * sizeof(*vps1->hdr)); - - return 0; + av_freep(&vps->data); } 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_alloc_ext(sizeof(*vps), 0, NULL, uninit_vps); + int vps_id = get_bits(gb, 4); + int ret = AVERROR_INVALIDDATA; + HEVCVPS *vps; + if (ps->pps_list[vps_id]) { + const HEVCVPS *vps1 = ps->vps_list[vps_id]; + if (vps1->data_size == gb->buffer_end - gb->buffer && + !memcmp(vps1->data, gb->buffer, vps1->data_size)) + return 0; + } + + vps = ff_refstruct_alloc_ext(sizeof(*vps), 0, NULL, hevc_vps_free); if (!vps) return AVERROR(ENOMEM); av_log(avctx, AV_LOG_DEBUG, "Decoding VPS\n"); - nal_size = gb->buffer_end - gb->buffer; - if (nal_size > sizeof(vps->data)) { - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized VPS " - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", - nal_size, sizeof(vps->data)); - vps->data_size = sizeof(vps->data); - } else { - vps->data_size = nal_size; + vps->data_size = gb->buffer_end - gb->buffer; + vps->data = av_memdup(gb->buffer, vps->data_size); + if (!vps->data) { + ret = AVERROR(ENOMEM); + goto err; } - memcpy(vps->data, gb->buffer, vps->data_size); - - vps_id = vps->vps_id = get_bits(gb, 4); + vps->vps_id = vps_id; if (get_bits(gb, 2) != 3) { // vps_reserved_three_2bits av_log(avctx, AV_LOG_ERROR, "vps_reserved_three_2bits is not three\n"); @@ -579,19 +574,14 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, goto err; } - if (ps->vps_list[vps_id] && - compare_vps(ps->vps_list[vps_id], vps)) { - ff_refstruct_unref(&vps); - } else { remove_vps(ps, vps_id); ps->vps_list[vps_id] = vps; - } return 0; err: ff_refstruct_unref(&vps); - return AVERROR_INVALIDDATA; + return ret; } static void decode_vui(GetBitContext *gb, AVCodecContext *avctx, @@ -1294,36 +1284,43 @@ int ff_hevc_parse_sps(HEVCSPS *sps, GetBitContext *gb, unsigned int *sps_id, return 0; } +static void hevc_sps_free(FFRefStructOpaque opaque, void *obj) +{ + HEVCSPS *sps = obj; + + av_freep(&sps->data); +} + +static int compare_sps(const HEVCSPS *sps1, const HEVCSPS *sps2) +{ + return sps1->data_size == sps2->data_size && + !memcmp(sps1->data, sps2->data, sps1->data_size); +} + int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, HEVCParamSets *ps, int apply_defdispwin) { - HEVCSPS *sps = ff_refstruct_allocz(sizeof(*sps)); + HEVCSPS *sps = ff_refstruct_alloc_ext(sizeof(*sps), 0, NULL, hevc_sps_free); unsigned int sps_id; int ret; - ptrdiff_t nal_size; if (!sps) return AVERROR(ENOMEM); av_log(avctx, AV_LOG_DEBUG, "Decoding SPS\n"); - nal_size = gb->buffer_end - gb->buffer; - if (nal_size > sizeof(sps->data)) { - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized SPS " - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", - nal_size, sizeof(sps->data)); - sps->data_size = sizeof(sps->data); - } else { - sps->data_size = nal_size; + sps->data_size = gb->buffer_end - gb->buffer; + sps->data = av_memdup(gb->buffer, sps->data_size); + if (!sps->data) { + ret = AVERROR(ENOMEM); + goto err; } - memcpy(sps->data, gb->buffer, sps->data_size); ret = ff_hevc_parse_sps(sps, gb, &sps_id, apply_defdispwin, ps->vps_list, avctx); if (ret < 0) { - ff_refstruct_unref(&sps); - return ret; + goto err; } if (avctx->debug & FF_DEBUG_BITSTREAM) { @@ -1340,7 +1337,7 @@ int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, * original one. * otherwise drop all PPSes that depend on it */ if (ps->sps_list[sps_id] && - !memcmp(ps->sps_list[sps_id], sps, sizeof(*sps))) { + compare_sps(ps->sps_list[sps_id], sps)) { ff_refstruct_unref(&sps); } else { remove_sps(ps, sps_id); @@ -1348,6 +1345,9 @@ int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, } return 0; +err: + ff_refstruct_unref(&sps); + return ret; } static void hevc_pps_free(FFRefStructOpaque unused, void *obj) @@ -1364,6 +1364,7 @@ static void hevc_pps_free(FFRefStructOpaque unused, void *obj) av_freep(&pps->tile_pos_rs); av_freep(&pps->tile_id); av_freep(&pps->min_tb_addr_zs_tab); + av_freep(&pps->data); } static void colour_mapping_octants(GetBitContext *gb, HEVCPPS *pps, int inp_depth, @@ -1762,27 +1763,35 @@ int ff_hevc_decode_nal_pps(GetBitContext *gb, AVCodecContext *avctx, const HEVCSPS *sps = NULL; const HEVCVPS *vps = NULL; int i, ret = 0; - unsigned int pps_id = 0; - ptrdiff_t nal_size; + unsigned int pps_id = get_ue_golomb_long(gb); unsigned log2_parallel_merge_level_minus2; + HEVCPPS *pps; + + av_log(avctx, AV_LOG_DEBUG, "Decoding PPS\n"); + + if (pps_id >= HEVC_MAX_PPS_COUNT) { + av_log(avctx, AV_LOG_ERROR, "PPS id out of range: %d\n", pps_id); + ret = AVERROR_INVALIDDATA; + goto err; + } - HEVCPPS *pps = ff_refstruct_alloc_ext(sizeof(*pps), 0, NULL, hevc_pps_free); + if (ps->pps_list[pps_id]) { + const HEVCPPS *pps1 = ps->pps_list[pps_id]; + if (pps1->data_size == gb->buffer_end - gb->buffer && + !memcmp(pps1->data, gb->buffer, pps1->data_size)) + return 0; + } + pps = ff_refstruct_alloc_ext(sizeof(*pps), 0, NULL, hevc_pps_free); if (!pps) return AVERROR(ENOMEM); - av_log(avctx, AV_LOG_DEBUG, "Decoding PPS\n"); - - nal_size = gb->buffer_end - gb->buffer; - if (nal_size > sizeof(pps->data)) { - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized PPS " - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", - nal_size, sizeof(pps->data)); - pps->data_size = sizeof(pps->data); - } else { - pps->data_size = nal_size; + pps->data_size = gb->buffer_end - gb->buffer; + pps->data = av_memdup(gb->buffer, pps->data_size); + if (!pps->data) { + ret = AVERROR_INVALIDDATA; + goto err; } - memcpy(pps->data, gb->buffer, pps->data_size); // Default values pps->loop_filter_across_tiles_enabled_flag = 1; @@ -1795,7 +1804,7 @@ int ff_hevc_decode_nal_pps(GetBitContext *gb, AVCodecContext *avctx, pps->log2_max_transform_skip_block_size = 2; // Coded parameters - pps_id = pps->pps_id = get_ue_golomb_long(gb); + pps->pps_id = pps_id; if (pps_id >= HEVC_MAX_PPS_COUNT) { av_log(avctx, AV_LOG_ERROR, "PPS id out of range: %d\n", pps_id); ret = AVERROR_INVALIDDATA; diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h index 0d8eaf2b3e..bed406770f 100644 --- a/libavcodec/hevc_ps.h +++ b/libavcodec/hevc_ps.h @@ -172,11 +172,11 @@ typedef struct HEVCVPS { int vps_num_ticks_poc_diff_one; ///< vps_num_ticks_poc_diff_one_minus1 + 1 int vps_num_hrd_parameters; - uint8_t data[4096]; - int data_size; - /* Put this at the end of the structure to make it easier to calculate the + /* Keep this at the end of the structure to make it easier to calculate the * size before this pointer, which is used for memcmp */ HEVCHdrParams *hdr; + uint8_t *data; + int data_size; } HEVCVPS; typedef struct ScalingList { @@ -299,7 +299,9 @@ typedef struct HEVCSPS { int qp_bd_offset; - uint8_t data[4096]; + /* Keep this at the end of the structure to make it easier to calculate the + * size before this pointer, which is used for memcmp */ + uint8_t *data; int data_size; } HEVCSPS; @@ -434,7 +436,9 @@ typedef struct HEVCPPS { int *min_tb_addr_zs; ///< MinTbAddrZS int *min_tb_addr_zs_tab;///< MinTbAddrZS - uint8_t data[4096]; + /* Keep this at the end of the structure to make it easier to calculate the + * size before this pointer, which is used for memcmp */ + uint8_t *data; int data_size; } HEVCPPS; -- 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-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avocdec/hevc_ps: don't use a fixed sized buffer for parameter set raw data 2024-03-29 16:59 ` [FFmpeg-devel] [PATCH v2] " James Almer @ 2024-03-29 17:28 ` Andreas Rheinhardt 2024-03-29 17:36 ` James Almer 0 siblings, 1 reply; 7+ messages in thread From: Andreas Rheinhardt @ 2024-03-29 17:28 UTC (permalink / raw) To: ffmpeg-devel James Almer: > Allocate it instead, and use it to compare sets instead of the parsed struct. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/hevc_ps.c | 127 +++++++++++++++++++++++-------------------- > libavcodec/hevc_ps.h | 14 +++-- > 2 files changed, 77 insertions(+), 64 deletions(-) > > diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c > index 6475d86d7d..7f74553fc1 100644 > --- a/libavcodec/hevc_ps.c > +++ b/libavcodec/hevc_ps.c > @@ -442,47 +442,42 @@ static int decode_hrd(GetBitContext *gb, int common_inf_present, > return 0; > } > > -static void uninit_vps(FFRefStructOpaque opaque, void *obj) > +static void hevc_vps_free(FFRefStructOpaque opaque, void *obj) > { > HEVCVPS *vps = obj; > > av_freep(&vps->hdr); > -} > - > -static int compare_vps(const HEVCVPS *vps1, const HEVCVPS *vps2) > -{ > - if (!memcmp(vps1, vps2, offsetof(HEVCVPS, hdr))) > - return !vps1->vps_num_hrd_parameters || > - !memcmp(vps1->hdr, vps2->hdr, vps1->vps_num_hrd_parameters * sizeof(*vps1->hdr)); > - > - return 0; > + av_freep(&vps->data); > } > > 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_alloc_ext(sizeof(*vps), 0, NULL, uninit_vps); > + int vps_id = get_bits(gb, 4); > + int ret = AVERROR_INVALIDDATA; > + HEVCVPS *vps; > > + if (ps->pps_list[vps_id]) { > + const HEVCVPS *vps1 = ps->vps_list[vps_id]; > + if (vps1->data_size == gb->buffer_end - gb->buffer && > + !memcmp(vps1->data, gb->buffer, vps1->data_size)) > + return 0; > + } > + > + vps = ff_refstruct_alloc_ext(sizeof(*vps), 0, NULL, hevc_vps_free); > if (!vps) > return AVERROR(ENOMEM); > > av_log(avctx, AV_LOG_DEBUG, "Decoding VPS\n"); > > - nal_size = gb->buffer_end - gb->buffer; > - if (nal_size > sizeof(vps->data)) { > - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized VPS " > - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", > - nal_size, sizeof(vps->data)); > - vps->data_size = sizeof(vps->data); > - } else { > - vps->data_size = nal_size; > + vps->data_size = gb->buffer_end - gb->buffer; > + vps->data = av_memdup(gb->buffer, vps->data_size); > + if (!vps->data) { > + ret = AVERROR(ENOMEM); > + goto err; > } > - memcpy(vps->data, gb->buffer, vps->data_size); > - > - vps_id = vps->vps_id = get_bits(gb, 4); > + vps->vps_id = vps_id; > > if (get_bits(gb, 2) != 3) { // vps_reserved_three_2bits > av_log(avctx, AV_LOG_ERROR, "vps_reserved_three_2bits is not three\n"); > @@ -579,19 +574,14 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, > goto err; > } > > - if (ps->vps_list[vps_id] && > - compare_vps(ps->vps_list[vps_id], vps)) { > - ff_refstruct_unref(&vps); > - } else { > remove_vps(ps, vps_id); > ps->vps_list[vps_id] = vps; > - } > > return 0; > > err: > ff_refstruct_unref(&vps); > - return AVERROR_INVALIDDATA; > + return ret; > } > > static void decode_vui(GetBitContext *gb, AVCodecContext *avctx, > @@ -1294,36 +1284,43 @@ int ff_hevc_parse_sps(HEVCSPS *sps, GetBitContext *gb, unsigned int *sps_id, > return 0; > } > > +static void hevc_sps_free(FFRefStructOpaque opaque, void *obj) > +{ > + HEVCSPS *sps = obj; > + > + av_freep(&sps->data); > +} > + > +static int compare_sps(const HEVCSPS *sps1, const HEVCSPS *sps2) > +{ > + return sps1->data_size == sps2->data_size && > + !memcmp(sps1->data, sps2->data, sps1->data_size); > +} > + > int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, > HEVCParamSets *ps, int apply_defdispwin) > { > - HEVCSPS *sps = ff_refstruct_allocz(sizeof(*sps)); > + HEVCSPS *sps = ff_refstruct_alloc_ext(sizeof(*sps), 0, NULL, hevc_sps_free); > unsigned int sps_id; > int ret; > - ptrdiff_t nal_size; > > if (!sps) > return AVERROR(ENOMEM); > > av_log(avctx, AV_LOG_DEBUG, "Decoding SPS\n"); > > - nal_size = gb->buffer_end - gb->buffer; > - if (nal_size > sizeof(sps->data)) { > - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized SPS " > - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", > - nal_size, sizeof(sps->data)); > - sps->data_size = sizeof(sps->data); > - } else { > - sps->data_size = nal_size; > + sps->data_size = gb->buffer_end - gb->buffer; > + sps->data = av_memdup(gb->buffer, sps->data_size); > + if (!sps->data) { > + ret = AVERROR(ENOMEM); > + goto err; > } > - memcpy(sps->data, gb->buffer, sps->data_size); > > ret = ff_hevc_parse_sps(sps, gb, &sps_id, > apply_defdispwin, > ps->vps_list, avctx); > if (ret < 0) { > - ff_refstruct_unref(&sps); > - return ret; > + goto err; > } > > if (avctx->debug & FF_DEBUG_BITSTREAM) { > @@ -1340,7 +1337,7 @@ int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, > * original one. > * otherwise drop all PPSes that depend on it */ > if (ps->sps_list[sps_id] && > - !memcmp(ps->sps_list[sps_id], sps, sizeof(*sps))) { > + compare_sps(ps->sps_list[sps_id], sps)) { > ff_refstruct_unref(&sps); > } else { > remove_sps(ps, sps_id); > @@ -1348,6 +1345,9 @@ int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, > } > > return 0; > +err: > + ff_refstruct_unref(&sps); > + return ret; > } > > static void hevc_pps_free(FFRefStructOpaque unused, void *obj) > @@ -1364,6 +1364,7 @@ static void hevc_pps_free(FFRefStructOpaque unused, void *obj) > av_freep(&pps->tile_pos_rs); > av_freep(&pps->tile_id); > av_freep(&pps->min_tb_addr_zs_tab); > + av_freep(&pps->data); > } > > static void colour_mapping_octants(GetBitContext *gb, HEVCPPS *pps, int inp_depth, > @@ -1762,27 +1763,35 @@ int ff_hevc_decode_nal_pps(GetBitContext *gb, AVCodecContext *avctx, > const HEVCSPS *sps = NULL; > const HEVCVPS *vps = NULL; > int i, ret = 0; > - unsigned int pps_id = 0; > - ptrdiff_t nal_size; > + unsigned int pps_id = get_ue_golomb_long(gb); > unsigned log2_parallel_merge_level_minus2; > + HEVCPPS *pps; > + > + av_log(avctx, AV_LOG_DEBUG, "Decoding PPS\n"); > + > + if (pps_id >= HEVC_MAX_PPS_COUNT) { > + av_log(avctx, AV_LOG_ERROR, "PPS id out of range: %d\n", pps_id); > + ret = AVERROR_INVALIDDATA; > + goto err; return AVERROR_INVALIDDATA; goto err would use pps which is uninitialized. > + } > > - HEVCPPS *pps = ff_refstruct_alloc_ext(sizeof(*pps), 0, NULL, hevc_pps_free); > + if (ps->pps_list[pps_id]) { > + const HEVCPPS *pps1 = ps->pps_list[pps_id]; > + if (pps1->data_size == gb->buffer_end - gb->buffer && > + !memcmp(pps1->data, gb->buffer, pps1->data_size)) > + return 0; > + } > > + pps = ff_refstruct_alloc_ext(sizeof(*pps), 0, NULL, hevc_pps_free); > if (!pps) > return AVERROR(ENOMEM); > > - av_log(avctx, AV_LOG_DEBUG, "Decoding PPS\n"); > - > - nal_size = gb->buffer_end - gb->buffer; > - if (nal_size > sizeof(pps->data)) { > - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized PPS " > - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", > - nal_size, sizeof(pps->data)); > - pps->data_size = sizeof(pps->data); > - } else { > - pps->data_size = nal_size; > + pps->data_size = gb->buffer_end - gb->buffer; > + pps->data = av_memdup(gb->buffer, pps->data_size); > + if (!pps->data) { > + ret = AVERROR_INVALIDDATA; > + goto err; > } > - memcpy(pps->data, gb->buffer, pps->data_size); > > // Default values > pps->loop_filter_across_tiles_enabled_flag = 1; > @@ -1795,7 +1804,7 @@ int ff_hevc_decode_nal_pps(GetBitContext *gb, AVCodecContext *avctx, > pps->log2_max_transform_skip_block_size = 2; > > // Coded parameters > - pps_id = pps->pps_id = get_ue_golomb_long(gb); > + pps->pps_id = pps_id; > if (pps_id >= HEVC_MAX_PPS_COUNT) { > av_log(avctx, AV_LOG_ERROR, "PPS id out of range: %d\n", pps_id); > ret = AVERROR_INVALIDDATA; You already check for this above. > diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h > index 0d8eaf2b3e..bed406770f 100644 > --- a/libavcodec/hevc_ps.h > +++ b/libavcodec/hevc_ps.h > @@ -172,11 +172,11 @@ typedef struct HEVCVPS { > int vps_num_ticks_poc_diff_one; ///< vps_num_ticks_poc_diff_one_minus1 + 1 > int vps_num_hrd_parameters; > > - uint8_t data[4096]; > - int data_size; > - /* Put this at the end of the structure to make it easier to calculate the > + /* Keep this at the end of the structure to make it easier to calculate the > * size before this pointer, which is used for memcmp */ > HEVCHdrParams *hdr; > + uint8_t *data; > + int data_size; > } HEVCVPS; > > typedef struct ScalingList { > @@ -299,7 +299,9 @@ typedef struct HEVCSPS { > > int qp_bd_offset; > > - uint8_t data[4096]; > + /* Keep this at the end of the structure to make it easier to calculate the > + * size before this pointer, which is used for memcmp */ > + uint8_t *data; > int data_size; > } HEVCSPS; > > @@ -434,7 +436,9 @@ typedef struct HEVCPPS { > int *min_tb_addr_zs; ///< MinTbAddrZS > int *min_tb_addr_zs_tab;///< MinTbAddrZS > > - uint8_t data[4096]; > + /* Keep this at the end of the structure to make it easier to calculate the > + * size before this pointer, which is used for memcmp */ You are adding comments that are already wrong at the time you add them. > + uint8_t *data; > int data_size; > } HEVCPPS; > _______________________________________________ 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". ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avocdec/hevc_ps: don't use a fixed sized buffer for parameter set raw data 2024-03-29 17:28 ` Andreas Rheinhardt @ 2024-03-29 17:36 ` James Almer 0 siblings, 0 replies; 7+ messages in thread From: James Almer @ 2024-03-29 17:36 UTC (permalink / raw) To: ffmpeg-devel On 3/29/2024 2:28 PM, Andreas Rheinhardt wrote: > James Almer: >> Allocate it instead, and use it to compare sets instead of the parsed struct. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/hevc_ps.c | 127 +++++++++++++++++++++++-------------------- >> libavcodec/hevc_ps.h | 14 +++-- >> 2 files changed, 77 insertions(+), 64 deletions(-) >> >> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c >> index 6475d86d7d..7f74553fc1 100644 >> --- a/libavcodec/hevc_ps.c >> +++ b/libavcodec/hevc_ps.c >> @@ -442,47 +442,42 @@ static int decode_hrd(GetBitContext *gb, int common_inf_present, >> return 0; >> } >> >> -static void uninit_vps(FFRefStructOpaque opaque, void *obj) >> +static void hevc_vps_free(FFRefStructOpaque opaque, void *obj) >> { >> HEVCVPS *vps = obj; >> >> av_freep(&vps->hdr); >> -} >> - >> -static int compare_vps(const HEVCVPS *vps1, const HEVCVPS *vps2) >> -{ >> - if (!memcmp(vps1, vps2, offsetof(HEVCVPS, hdr))) >> - return !vps1->vps_num_hrd_parameters || >> - !memcmp(vps1->hdr, vps2->hdr, vps1->vps_num_hrd_parameters * sizeof(*vps1->hdr)); >> - >> - return 0; >> + av_freep(&vps->data); >> } >> >> 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_alloc_ext(sizeof(*vps), 0, NULL, uninit_vps); >> + int vps_id = get_bits(gb, 4); >> + int ret = AVERROR_INVALIDDATA; >> + HEVCVPS *vps; >> >> + if (ps->pps_list[vps_id]) { >> + const HEVCVPS *vps1 = ps->vps_list[vps_id]; >> + if (vps1->data_size == gb->buffer_end - gb->buffer && >> + !memcmp(vps1->data, gb->buffer, vps1->data_size)) >> + return 0; >> + } >> + >> + vps = ff_refstruct_alloc_ext(sizeof(*vps), 0, NULL, hevc_vps_free); >> if (!vps) >> return AVERROR(ENOMEM); >> >> av_log(avctx, AV_LOG_DEBUG, "Decoding VPS\n"); >> >> - nal_size = gb->buffer_end - gb->buffer; >> - if (nal_size > sizeof(vps->data)) { >> - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized VPS " >> - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", >> - nal_size, sizeof(vps->data)); >> - vps->data_size = sizeof(vps->data); >> - } else { >> - vps->data_size = nal_size; >> + vps->data_size = gb->buffer_end - gb->buffer; >> + vps->data = av_memdup(gb->buffer, vps->data_size); >> + if (!vps->data) { >> + ret = AVERROR(ENOMEM); >> + goto err; >> } >> - memcpy(vps->data, gb->buffer, vps->data_size); >> - >> - vps_id = vps->vps_id = get_bits(gb, 4); >> + vps->vps_id = vps_id; >> >> if (get_bits(gb, 2) != 3) { // vps_reserved_three_2bits >> av_log(avctx, AV_LOG_ERROR, "vps_reserved_three_2bits is not three\n"); >> @@ -579,19 +574,14 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx, >> goto err; >> } >> >> - if (ps->vps_list[vps_id] && >> - compare_vps(ps->vps_list[vps_id], vps)) { >> - ff_refstruct_unref(&vps); >> - } else { >> remove_vps(ps, vps_id); >> ps->vps_list[vps_id] = vps; >> - } >> >> return 0; >> >> err: >> ff_refstruct_unref(&vps); >> - return AVERROR_INVALIDDATA; >> + return ret; >> } >> >> static void decode_vui(GetBitContext *gb, AVCodecContext *avctx, >> @@ -1294,36 +1284,43 @@ int ff_hevc_parse_sps(HEVCSPS *sps, GetBitContext *gb, unsigned int *sps_id, >> return 0; >> } >> >> +static void hevc_sps_free(FFRefStructOpaque opaque, void *obj) >> +{ >> + HEVCSPS *sps = obj; >> + >> + av_freep(&sps->data); >> +} >> + >> +static int compare_sps(const HEVCSPS *sps1, const HEVCSPS *sps2) >> +{ >> + return sps1->data_size == sps2->data_size && >> + !memcmp(sps1->data, sps2->data, sps1->data_size); >> +} >> + >> int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, >> HEVCParamSets *ps, int apply_defdispwin) >> { >> - HEVCSPS *sps = ff_refstruct_allocz(sizeof(*sps)); >> + HEVCSPS *sps = ff_refstruct_alloc_ext(sizeof(*sps), 0, NULL, hevc_sps_free); >> unsigned int sps_id; >> int ret; >> - ptrdiff_t nal_size; >> >> if (!sps) >> return AVERROR(ENOMEM); >> >> av_log(avctx, AV_LOG_DEBUG, "Decoding SPS\n"); >> >> - nal_size = gb->buffer_end - gb->buffer; >> - if (nal_size > sizeof(sps->data)) { >> - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized SPS " >> - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", >> - nal_size, sizeof(sps->data)); >> - sps->data_size = sizeof(sps->data); >> - } else { >> - sps->data_size = nal_size; >> + sps->data_size = gb->buffer_end - gb->buffer; >> + sps->data = av_memdup(gb->buffer, sps->data_size); >> + if (!sps->data) { >> + ret = AVERROR(ENOMEM); >> + goto err; >> } >> - memcpy(sps->data, gb->buffer, sps->data_size); >> >> ret = ff_hevc_parse_sps(sps, gb, &sps_id, >> apply_defdispwin, >> ps->vps_list, avctx); >> if (ret < 0) { >> - ff_refstruct_unref(&sps); >> - return ret; >> + goto err; >> } >> >> if (avctx->debug & FF_DEBUG_BITSTREAM) { >> @@ -1340,7 +1337,7 @@ int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, >> * original one. >> * otherwise drop all PPSes that depend on it */ >> if (ps->sps_list[sps_id] && >> - !memcmp(ps->sps_list[sps_id], sps, sizeof(*sps))) { >> + compare_sps(ps->sps_list[sps_id], sps)) { >> ff_refstruct_unref(&sps); >> } else { >> remove_sps(ps, sps_id); >> @@ -1348,6 +1345,9 @@ int ff_hevc_decode_nal_sps(GetBitContext *gb, AVCodecContext *avctx, >> } >> >> return 0; >> +err: >> + ff_refstruct_unref(&sps); >> + return ret; >> } >> >> static void hevc_pps_free(FFRefStructOpaque unused, void *obj) >> @@ -1364,6 +1364,7 @@ static void hevc_pps_free(FFRefStructOpaque unused, void *obj) >> av_freep(&pps->tile_pos_rs); >> av_freep(&pps->tile_id); >> av_freep(&pps->min_tb_addr_zs_tab); >> + av_freep(&pps->data); >> } >> >> static void colour_mapping_octants(GetBitContext *gb, HEVCPPS *pps, int inp_depth, >> @@ -1762,27 +1763,35 @@ int ff_hevc_decode_nal_pps(GetBitContext *gb, AVCodecContext *avctx, >> const HEVCSPS *sps = NULL; >> const HEVCVPS *vps = NULL; >> int i, ret = 0; >> - unsigned int pps_id = 0; >> - ptrdiff_t nal_size; >> + unsigned int pps_id = get_ue_golomb_long(gb); >> unsigned log2_parallel_merge_level_minus2; >> + HEVCPPS *pps; >> + >> + av_log(avctx, AV_LOG_DEBUG, "Decoding PPS\n"); >> + >> + if (pps_id >= HEVC_MAX_PPS_COUNT) { >> + av_log(avctx, AV_LOG_ERROR, "PPS id out of range: %d\n", pps_id); >> + ret = AVERROR_INVALIDDATA; >> + goto err; > > return AVERROR_INVALIDDATA; > goto err would use pps which is uninitialized. > >> + } >> >> - HEVCPPS *pps = ff_refstruct_alloc_ext(sizeof(*pps), 0, NULL, hevc_pps_free); >> + if (ps->pps_list[pps_id]) { >> + const HEVCPPS *pps1 = ps->pps_list[pps_id]; >> + if (pps1->data_size == gb->buffer_end - gb->buffer && >> + !memcmp(pps1->data, gb->buffer, pps1->data_size)) >> + return 0; >> + } >> >> + pps = ff_refstruct_alloc_ext(sizeof(*pps), 0, NULL, hevc_pps_free); >> if (!pps) >> return AVERROR(ENOMEM); >> >> - av_log(avctx, AV_LOG_DEBUG, "Decoding PPS\n"); >> - >> - nal_size = gb->buffer_end - gb->buffer; >> - if (nal_size > sizeof(pps->data)) { >> - av_log(avctx, AV_LOG_WARNING, "Truncating likely oversized PPS " >> - "(%"PTRDIFF_SPECIFIER" > %"SIZE_SPECIFIER")\n", >> - nal_size, sizeof(pps->data)); >> - pps->data_size = sizeof(pps->data); >> - } else { >> - pps->data_size = nal_size; >> + pps->data_size = gb->buffer_end - gb->buffer; >> + pps->data = av_memdup(gb->buffer, pps->data_size); >> + if (!pps->data) { >> + ret = AVERROR_INVALIDDATA; >> + goto err; >> } >> - memcpy(pps->data, gb->buffer, pps->data_size); >> >> // Default values >> pps->loop_filter_across_tiles_enabled_flag = 1; >> @@ -1795,7 +1804,7 @@ int ff_hevc_decode_nal_pps(GetBitContext *gb, AVCodecContext *avctx, >> pps->log2_max_transform_skip_block_size = 2; >> >> // Coded parameters >> - pps_id = pps->pps_id = get_ue_golomb_long(gb); >> + pps->pps_id = pps_id; >> if (pps_id >= HEVC_MAX_PPS_COUNT) { >> av_log(avctx, AV_LOG_ERROR, "PPS id out of range: %d\n", pps_id); >> ret = AVERROR_INVALIDDATA; > > You already check for this above. Will remove. > >> diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h >> index 0d8eaf2b3e..bed406770f 100644 >> --- a/libavcodec/hevc_ps.h >> +++ b/libavcodec/hevc_ps.h >> @@ -172,11 +172,11 @@ typedef struct HEVCVPS { >> int vps_num_ticks_poc_diff_one; ///< vps_num_ticks_poc_diff_one_minus1 + 1 >> int vps_num_hrd_parameters; >> >> - uint8_t data[4096]; >> - int data_size; >> - /* Put this at the end of the structure to make it easier to calculate the >> + /* Keep this at the end of the structure to make it easier to calculate the >> * size before this pointer, which is used for memcmp */ >> HEVCHdrParams *hdr; >> + uint8_t *data; >> + int data_size; >> } HEVCVPS; >> >> typedef struct ScalingList { >> @@ -299,7 +299,9 @@ typedef struct HEVCSPS { >> >> int qp_bd_offset; >> >> - uint8_t data[4096]; >> + /* Keep this at the end of the structure to make it easier to calculate the >> + * size before this pointer, which is used for memcmp */ >> + uint8_t *data; >> int data_size; >> } HEVCSPS; >> >> @@ -434,7 +436,9 @@ typedef struct HEVCPPS { >> int *min_tb_addr_zs; ///< MinTbAddrZS >> int *min_tb_addr_zs_tab;///< MinTbAddrZS >> >> - uint8_t data[4096]; >> + /* Keep this at the end of the structure to make it easier to calculate the >> + * size before this pointer, which is used for memcmp */ > > You are adding comments that are already wrong at the time you add them. Yeah, remnant of a previous version where i was doing first a data check then struct check. Will fix the above stuff and push later. Thanks. > >> + uint8_t *data; >> int data_size; >> } HEVCPPS; >> > > _______________________________________________ > 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". ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-03-29 17:36 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-03-29 15:56 [FFmpeg-devel] [PATCH] avocdec/hevc_ps: don't use a fixed sized buffer for parameter set raw data James Almer 2024-03-29 16:10 ` Andreas Rheinhardt 2024-03-29 16:17 ` James Almer 2024-03-29 16:31 ` Andreas Rheinhardt 2024-03-29 16:59 ` [FFmpeg-devel] [PATCH v2] " James Almer 2024-03-29 17:28 ` Andreas Rheinhardt 2024-03-29 17:36 ` James Almer
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git