* [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