[FFmpeg-devel] [PATCH] libavformat: Account for negative position differences in ff_configure_buffers_for_index

[FFmpeg-devel] [PATCH] libavformat: Account for negative position differences in ff_configure_buffers_for_index

[Martin Storsjö]

When scanning through the index, account for the fact that the
compared samples may be located in an unexpected order in the file;
this function is mainly interested in the absolute difference between
file locations.

Signed-off-by: Martin Storsjö <martin@martin.st>
---
 libavformat/seek.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/seek.c b/libavformat/seek.c
index 818549dfef..62a26d6b5f 100644
--- a/libavformat/seek.c
+++ b/libavformat/seek.c
@@ -210,7 +210,7 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance)
                     int64_t e2_pts = av_rescale_q(e2->timestamp, st2->time_base, AV_TIME_BASE_Q);
                     if (e2_pts < e1_pts || e2_pts - (uint64_t)e1_pts < time_tolerance)
                         continue;
-                    pos_delta = FFMAX(pos_delta, e1->pos - e2->pos);
+                    pos_delta = FFMAX(pos_delta, llabs(e1->pos - e2->pos));
                     break;
                 }
             }
-- 
2.37.1 (Apple Git-137.1)

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

Re: [FFmpeg-devel] [PATCH] libavformat: Account for negative position differences in ff_configure_buffers_for_index

[James Almer]

On 3/24/2023 6:09 PM, Martin Storsjö wrote:
> When scanning through the index, account for the fact that the
> compared samples may be located in an unexpected order in the file;
> this function is mainly interested in the absolute difference between
> file locations.
> 
> Signed-off-by: Martin Storsjö <martin@martin.st>
> ---
>   libavformat/seek.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/seek.c b/libavformat/seek.c
> index 818549dfef..62a26d6b5f 100644
> --- a/libavformat/seek.c
> +++ b/libavformat/seek.c
> @@ -210,7 +210,7 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance)
>                       int64_t e2_pts = av_rescale_q(e2->timestamp, st2->time_base, AV_TIME_BASE_Q);
>                       if (e2_pts < e1_pts || e2_pts - (uint64_t)e1_pts < time_tolerance)
>                           continue;
> -                    pos_delta = FFMAX(pos_delta, e1->pos - e2->pos);
> +                    pos_delta = FFMAX(pos_delta, llabs(e1->pos - e2->pos));

There's FFABS(), which gives a lot more hits with git grep than llabs.

>                       break;
>                   }
>               }
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] libavformat: Account for negative position differences in ff_configure_buffers_for_index

[Martin Storsjö]

On Fri, 24 Mar 2023, James Almer wrote:

> On 3/24/2023 6:09 PM, Martin Storsjö wrote:
>> When scanning through the index, account for the fact that the
>> compared samples may be located in an unexpected order in the file;
>> this function is mainly interested in the absolute difference between
>> file locations.
>> 
>> Signed-off-by: Martin Storsjö <martin@martin.st>
>> ---
>>   libavformat/seek.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/libavformat/seek.c b/libavformat/seek.c
>> index 818549dfef..62a26d6b5f 100644
>> --- a/libavformat/seek.c
>> +++ b/libavformat/seek.c
>> @@ -210,7 +210,7 @@ void ff_configure_buffers_for_index(AVFormatContext *s, 
> int64_t time_tolerance)
>>                       int64_t e2_pts = av_rescale_q(e2->timestamp, 
> st2->time_base, AV_TIME_BASE_Q);
>>                       if (e2_pts < e1_pts || e2_pts - (uint64_t)e1_pts < 
> time_tolerance)
>>                           continue;
>> -                    pos_delta = FFMAX(pos_delta, e1->pos - e2->pos);
>> +                    pos_delta = FFMAX(pos_delta, llabs(e1->pos - 
> e2->pos));
>
> There's FFABS(), which gives a lot more hits with git grep than llabs.

Fair enough, I can change it to that too.

Since FFMAX is a macro, it ends up expanding the arguments twice, so with 
a call to llabs, we'd have two function calls. I guess it'd be better with 
an intermediate variable,

     int64_t delta = FFABS(e1->pos - e2->pos);
     pos_delta = FFMAX(pos_delta, delta);

WDYT?

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

Re: [FFmpeg-devel] [PATCH] libavformat: Account for negative position differences in ff_configure_buffers_for_index

[James Almer]

On 3/24/2023 6:45 PM, Martin Storsjö wrote:
> On Fri, 24 Mar 2023, James Almer wrote:
> 
>> On 3/24/2023 6:09 PM, Martin Storsjö wrote:
>>> When scanning through the index, account for the fact that the
>>> compared samples may be located in an unexpected order in the file;
>>> this function is mainly interested in the absolute difference between
>>> file locations.
>>>
>>> Signed-off-by: Martin Storsjö <martin@martin.st>
>>> ---
>>>   libavformat/seek.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavformat/seek.c b/libavformat/seek.c
>>> index 818549dfef..62a26d6b5f 100644
>>> --- a/libavformat/seek.c
>>> +++ b/libavformat/seek.c
>>> @@ -210,7 +210,7 @@ void 
>>> ff_configure_buffers_for_index(AVFormatContext *s, 
>> int64_t time_tolerance)
>>>                       int64_t e2_pts = av_rescale_q(e2->timestamp, 
>> st2->time_base, AV_TIME_BASE_Q);
>>>                       if (e2_pts < e1_pts || e2_pts - (uint64_t)e1_pts < 
>> time_tolerance)
>>>                           continue;
>>> -                    pos_delta = FFMAX(pos_delta, e1->pos - e2->pos);
>>> +                    pos_delta = FFMAX(pos_delta, llabs(e1->pos - 
>> e2->pos));
>>
>> There's FFABS(), which gives a lot more hits with git grep than llabs.
> 
> Fair enough, I can change it to that too.
> 
> Since FFMAX is a macro, it ends up expanding the arguments twice, so 
> with a call to llabs, we'd have two function calls. I guess it'd be 
> better with an intermediate variable,
> 
>      int64_t delta = FFABS(e1->pos - e2->pos);
>      pos_delta = FFMAX(pos_delta, delta);
> 
> WDYT?

Yeah, that good.

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