| Message ID | 20240329155601.14190-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avocdec/hevc_ps: don't use a fixed sized buffer for parameter set raw data | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| yinshiyou/make_loongarch64 | warning | New warnings during build |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_x86 | warning | New warnings during build |
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
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.
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
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;
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(-)