From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 18A134A554 for ; Fri, 29 Mar 2024 16:17:06 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 105A568D758; Fri, 29 Mar 2024 18:17:04 +0200 (EET) Received: from mail-pj1-f41.google.com (mail-pj1-f41.google.com [209.85.216.41]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 66AA168D4BA for ; Fri, 29 Mar 2024 18:16:57 +0200 (EET) Received: by mail-pj1-f41.google.com with SMTP id 98e67ed59e1d1-29ddfada0d0so1604239a91.3 for ; Fri, 29 Mar 2024 09:16:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1711729015; x=1712333815; darn=ffmpeg.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id:from :to:cc:subject:date:message-id:reply-to; bh=1GycqFeBtu1g8KqKMC0ltN0V6Ap/0udWYstZE+n+5HQ=; b=Jl/IgcEK8I5AL17DjSFg0QFjMKqzjUgtMztx6cYYu8gh0ELGYGingGm3/kelONSTgO ipvUHi+lRiVHXTqcKKjv3jqdU+Jk4xO4T274vCe9pIPpkFGOqrPrpXfcHoCJ1JqOi5Ub nV5rQvIjbOSZgcHmFEc2E8hLqUyACQkafT5FSTyXVlLsZ8ZBxzyLvgq1z2iHjw0GzsZO J1tsnirgYYokkd+YWliCAAFP6vO4guBmfHm5lKuqi2TCzS/fvEFp62EhAINbHYcdM71t NvzLvE6I/mTfL5v3YebVwzU7biP6GvsiuaHatP9zmaqIghyVX4MdGI/HSagXfXRIQQds Z1nA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711729015; x=1712333815; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=1GycqFeBtu1g8KqKMC0ltN0V6Ap/0udWYstZE+n+5HQ=; b=WLZ7OjaWTJJR2wroMmIkFOQMh4C/FTmXDx5f9Yj5rOKoRnVfpOiwPpX52Y0084aujd 12pnRd3lG+1ZJWF8y3lu4LffDA9nC3lAyHo6SijXw+DqDVY8XzUVvxiun4k+nwC30fYT u40kWwba4ANZyrHDKbVVBPiMsuv9ipRereLt6wHlkv7BXDF72lykZEbcMfF/pavjhZmG MsHEZexFVwUU0V/7uwNCikgnxFSHVmNRL5YQFUWjlN8nG7HF3L6RyGHxg8Vy3fT8JFAd OClfBB/im0eKnwsQe9X/maIPIpckza9FSS9yY5rKM3IlcEEZQlcZXq5k8XKyV8XNJYNj ZoGQ== X-Gm-Message-State: AOJu0YzDi2M7LwBTiRNHIy/VTkfBLjuwiqALGefbpU6dp7ZU2OMMKLSk 6mMfwnK57LqdpN2H6gpI6/jxNdtiZxENSPjLNl7PuFqbuQMI0b2dW9sFmUVT X-Google-Smtp-Source: AGHT+IFpEHqLl/9nDPBau6Nsb5rpE6HsgsYFph0Q3W2mztp+31QaPlANktSQFAUAox0b2EEGiMQWZQ== X-Received: by 2002:a17:90a:7d08:b0:2a0:3c38:5e2e with SMTP id g8-20020a17090a7d0800b002a03c385e2emr2270864pjl.45.1711729015192; Fri, 29 Mar 2024 09:16:55 -0700 (PDT) Received: from [192.168.0.15] ([190.194.167.233]) by smtp.gmail.com with ESMTPSA id 13-20020a630b0d000000b005e840ad9aaesm3115218pgl.30.2024.03.29.09.16.54 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 29 Mar 2024 09:16:54 -0700 (PDT) Message-ID: <88b558f7-df39-4078-b256-ab94a5e20d7f@gmail.com> Date: Fri, 29 Mar 2024 13:17:00 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: ffmpeg-devel@ffmpeg.org References: <20240329155601.14190-1-jamrial@gmail.com> Content-Language: en-US From: James Almer In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH] avocdec/hevc_ps: don't use a fixed sized buffer for parameter set raw data X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: 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 >> --- >> 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".