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
>Andreas Rheinhardt
>Sent: Friday, March 29, 2024 10:03 PM
>To: ffmpeg-devel@ffmpeg.org
>Subject: Re: [FFmpeg-devel] [PATCH] avcodec/hevc_ps: fix the problem of
>memcmp losing effectiveness
>
>Mark Thompson:
>> On 28/03/2024 13:15, 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(-)
>>
>> It doesn't seem like this method works at all, even before the recent
>> change with the pointer.
>>
>> Structs can contain arbitrary padding, and any write to the struct makes
>> the padding unspecified.  memcmp() is therefore never valid as a method
>> of comparing after writing some fields, as done here.  (It could only be
>> valid if the structs compared were made by memcpy() with no fields
>> written directly.)
>>
>> The problem is mostly harmless because the nondeterministic replacement
>> of structs which we were expecting to be equivalent doesn't actually
>> change anything, so why don't we just remove the comparison and always
>> replace?
>>
>
>remove_vps() also removes any SPS referencing this VPS (and remove_sps()
>does the same with PPS). Therefore if you simply repeat a VPS without
>also repeating the other parameter sets directly after the new VPS and
>before the first video NALU after the VPS, your extradata will have been
>discarded.
>This is not what the spec says.
>
>

Yes and I observed for hevc decoder with hwaccel, get_format() is called multiple times which initializes the hwaccel context multiple times, as s->ps.sps is unexpectedly removed because of that.

Hendrik also observed some playback glitches(see previous email) so it's not really harmless.


>_______________________________________________
>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".