[FFmpeg-devel] [PATCH 3/5] startcode: Stop overreading

[Andreas Rheinhardt]

Michael Niedermayer:
> On Sun, Jun 02, 2019 at 12:47:17AM +0200, Andreas Rheinhardt wrote:
>> Up until now ff_startcode_find_candidate_c could overread; it relied on
>> zero-padding after the buffer in order to function correctly. This has
>> been changed: No overreads occur any more.
>> The ultimate goal behind all this is to create a high-performance
>> function for searching of startcodes that can be applied even in
>> scenarios where the buffer is not padded.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>> ---
>>  libavcodec/startcode.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/libavcodec/startcode.c b/libavcodec/startcode.c
>> index 373572365b..b027c191c0 100644
>> --- a/libavcodec/startcode.c
>> +++ b/libavcodec/startcode.c
>> @@ -41,10 +41,7 @@ int ff_startcode_find_candidate_c(const uint8_t *buf, int size)
>>  
>>  #define READ(bitness) AV_RN ## bitness ## A
>>  #define MAIN_LOOP(bitness, mask1, mask2) do {                              \
>> -        /* we check p < end instead of p + 3 / 7 because it is
>> -         * simpler and there must be AV_INPUT_BUFFER_PADDING_SIZE
>> -         * bytes at the end. */                                            \
> 
>> -        for (; buf < end; buf += bitness / 8)                              \
>> +        for (; buf <= end - bitness / 8; buf += bitness / 8)               \
> 
> is this faster than subtracting  bitness / 8 from end outside the loop ?
> if not then i would suggest to do it outside as that means 1 thing less
> the compiler has to optimize out
> 
> thx
In my tests the compiler calculates end - bitness / 8 only once and
puts it in a register of its own. But I can do it manually, of course.
In fact, I just noticed a potential undefined behaviour in this patch
(and only in this patch, the next one actually fixes it), so this
needs to be changed anyway: Nothing guarantees that end - bitness / 8
is actually part of the array any more and so this would be undefined
behaviour (even if no access occurs).

- Andreas