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