| Message ID | 20230324210951.47779-1-martin@martin.st |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] libavformat: Account for negative position differences in ff_configure_buffers_for_index | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
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; > } > }
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
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".
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; } }
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(-)