Re: [FFmpeg-devel] [PATCH] avcodec/hevc_ps: fix the problem of memcmp losing effectiveness

["Wu, Tong1" <tong1.wu-at-intel.com@ffmpeg.org>]

>From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of James
>Almer
>Sent: Friday, March 29, 2024 8:46 PM
>To: ffmpeg-devel@ffmpeg.org
>Subject: Re: [FFmpeg-devel] [PATCH] avcodec/hevc_ps: fix the problem of
>memcmp losing effectiveness
>
>On 3/28/2024 10:15 AM, tong1.wu-at-intel.com@ffmpeg.org wrote:
>> From: Tong Wu <tong1.wu@intel.com>
>>
>> HEVCHdrParams* receives a pointer which points to a dynamically
>> allocated memory block. It causes the memcmp always returning 1.
>> Add a function to do the comparision. A condition is also added to
>> avoid malloc(0).
>>
>> Signed-off-by: Tong Wu <tong1.wu@intel.com>
>> ---
>>   libavcodec/hevc_ps.c | 20 ++++++++++++++++----
>>   libavcodec/hevc_ps.h |  4 +++-
>>   2 files changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
>> index cbef3ef4cd..8b3a27a17c 100644
>> --- a/libavcodec/hevc_ps.c
>> +++ b/libavcodec/hevc_ps.c
>> @@ -449,6 +449,16 @@ static void uninit_vps(FFRefStructOpaque opaque,
>void *obj)
>>       av_freep(&vps->hdr);
>>   }
>>
>> +static int compare_vps(const HEVCVPS *vps1, const HEVCVPS *vps2)
>> +{
>> +    if ((!vps1->hdr && !vps2->hdr) ||
>> +        (vps1->hdr && vps2->hdr && !memcmp(vps1->hdr, vps2->hdr,
>sizeof(*vps1->hdr)))) {
>
>I think this should be vps1->vps_num_hrd_parameters *
>sizeof(*vps1->hdr), and done after the memcmp below to ensure
>vps_num_hrd_parameters is the same value in both structs.

Updated as suggested. Thanks.

Tong

>
>> +        return !memcmp(vps1, vps2, offsetof(HEVCVPS, hdr));
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   int ff_hevc_decode_nal_vps(GetBitContext *gb, AVCodecContext *avctx,
>>                              HEVCParamSets *ps)
>>   {
>> @@ -545,9 +555,11 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb,
>AVCodecContext *avctx,
>>               goto err;
>>           }
>>
>> -        vps->hdr = av_calloc(vps->vps_num_hrd_parameters, sizeof(*vps-
>>hdr));
>> -        if (!vps->hdr)
>> -            goto err;
>> +        if (vps->vps_num_hrd_parameters) {
>> +            vps->hdr = av_calloc(vps->vps_num_hrd_parameters, sizeof(*vps-
>>hdr));
>> +            if (!vps->hdr)
>> +                goto err;
>> +        }
>>
>>           for (i = 0; i < vps->vps_num_hrd_parameters; i++) {
>>               int common_inf_present = 1;
>> @@ -569,7 +581,7 @@ int ff_hevc_decode_nal_vps(GetBitContext *gb,
>AVCodecContext *avctx,
>>       }
>>
>>       if (ps->vps_list[vps_id] &&
>> -        !memcmp(ps->vps_list[vps_id], vps, sizeof(*vps))) {
>> +        compare_vps(ps->vps_list[vps_id], vps)) {
>>           ff_refstruct_unref(&vps);
>>       } else {
>>           remove_vps(ps, vps_id);
>> diff --git a/libavcodec/hevc_ps.h b/libavcodec/hevc_ps.h
>> index cc75aeb8d3..0d8eaf2b3e 100644
>> --- a/libavcodec/hevc_ps.h
>> +++ b/libavcodec/hevc_ps.h
>> @@ -153,7 +153,6 @@ typedef struct PTL {
>>
>>   typedef struct HEVCVPS {
>>       unsigned int vps_id;
>> -    HEVCHdrParams *hdr;
>>
>>       uint8_t vps_temporal_id_nesting_flag;
>>       int vps_max_layers;
>> @@ -175,6 +174,9 @@ typedef struct HEVCVPS {
>>
>>       uint8_t data[4096];
>>       int data_size;
>> +    /* Put 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;
>>   } HEVCVPS;
>>
>>   typedef struct ScalingList {
>_______________________________________________
>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".