| Message ID | tencent_E39F23AB404E2370BB2D5D9A3940FE9A2408@qq.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/utils: fix seek failed | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
On Thu, 31 Dec 2020, Zhao Zhili wrote: > Rounding min_ts towards +infinity and max_ts towards -infinity can > make ts out of the [min_ts, max_ts] range, and then leads to seek > failure. For example, I think this is intentional to adhere to the docs of the function: "Seeking will be done so that the point from which all active streams can be presented successfully will be closest to ts and within min/max_ts." If you round down min_ts, you might return a packet with a timestamp less than the original min_ts, which contradicts the documentation. Regards, Marton > > max_ts = ts = -25057 > time_base = (num = 1, den = 14112000) > > After rescale, ts = -353604, and max_ts = -353605. > --- > libavformat/utils.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavformat/utils.c b/libavformat/utils.c > index 503e583ad0..69a0f901b2 100644 > --- a/libavformat/utils.c > +++ b/libavformat/utils.c > @@ -2500,10 +2500,10 @@ int avformat_seek_file(AVFormatContext *s, int stream_index, int64_t min_ts, > ts = av_rescale_q(ts, AV_TIME_BASE_Q, time_base); > min_ts = av_rescale_rnd(min_ts, time_base.den, > time_base.num * (int64_t)AV_TIME_BASE, > - AV_ROUND_UP | AV_ROUND_PASS_MINMAX); > + AV_ROUND_DOWN | AV_ROUND_PASS_MINMAX); > max_ts = av_rescale_rnd(max_ts, time_base.den, > time_base.num * (int64_t)AV_TIME_BASE, > - AV_ROUND_DOWN | AV_ROUND_PASS_MINMAX); > + AV_ROUND_UP | AV_ROUND_PASS_MINMAX); > stream_index = 0; > } > > -- > 2.28.0 > > _______________________________________________ > 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".
> On Dec 31, 2020, at 6:51 PM, Marton Balint <cus@passwd.hu> wrote: > > > > On Thu, 31 Dec 2020, Zhao Zhili wrote: > >> Rounding min_ts towards +infinity and max_ts towards -infinity can >> make ts out of the [min_ts, max_ts] range, and then leads to seek >> failure. For example, > > I think this is intentional to adhere to the docs of the function: > > "Seeking will be done so that the point from which all active streams > can be presented successfully will be closest to ts and within min/max_ts." > > If you round down min_ts, you might return a packet with a timestamp less than the original min_ts, which contradicts the documentation. > Yes, it’s not comply with the documentation strictly. On the other side, does the documentation have strong guarantee about the rounding error? Rounding error is less of a surprise than seek failure in my opinion. ffmpeg.c seek_to_start() call avformat_seek_file() with max_ts equal to ts. If avformat_seek_file() should be kept as it is, then the caller should be fixed. > Regards, > Marton > > >> >> max_ts = ts = -25057 >> time_base = (num = 1, den = 14112000) >> >> After rescale, ts = -353604, and max_ts = -353605. >> --- >> libavformat/utils.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/libavformat/utils.c b/libavformat/utils.c >> index 503e583ad0..69a0f901b2 100644 >> --- a/libavformat/utils.c >> +++ b/libavformat/utils.c >> @@ -2500,10 +2500,10 @@ int avformat_seek_file(AVFormatContext *s, int stream_index, int64_t min_ts, >> ts = av_rescale_q(ts, AV_TIME_BASE_Q, time_base); >> min_ts = av_rescale_rnd(min_ts, time_base.den, >> time_base.num * (int64_t)AV_TIME_BASE, >> - AV_ROUND_UP | AV_ROUND_PASS_MINMAX); >> + AV_ROUND_DOWN | AV_ROUND_PASS_MINMAX); >> max_ts = av_rescale_rnd(max_ts, time_base.den, >> time_base.num * (int64_t)AV_TIME_BASE, >> - AV_ROUND_DOWN | AV_ROUND_PASS_MINMAX); >> + AV_ROUND_UP | AV_ROUND_PASS_MINMAX); >> stream_index = 0; >> } >> -- >> 2.28.0 >> >> _______________________________________________ >> 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".
On Thu, 31 Dec 2020, "zhilizhao(赵志立)" wrote: > > >> On Dec 31, 2020, at 6:51 PM, Marton Balint <cus@passwd.hu> wrote: >> >> >> >> On Thu, 31 Dec 2020, Zhao Zhili wrote: >> >>> Rounding min_ts towards +infinity and max_ts towards -infinity can >>> make ts out of the [min_ts, max_ts] range, and then leads to seek >>> failure. For example, >> >> I think this is intentional to adhere to the docs of the function: >> >> "Seeking will be done so that the point from which all active streams >> can be presented successfully will be closest to ts and within min/max_ts." >> >> If you round down min_ts, you might return a packet with a timestamp less than the original min_ts, which contradicts the documentation. >> > > Yes, it’s not comply with the documentation strictly. On the other side, does > the documentation have strong guarantee about the rounding error? Rounding > error is less of a surprise than seek failure in my opinion. If the question is what the user would expect, then the answer is probably simple rounding for both min_ts and max_ts. That should also solve your error of ts becoming out of the [min_ts,max_ts] interval. However in your example the time base of the stream is more precise than AV_TIME_BASE_Q, and that can cause unexpected behaviour regardless of the rouding you use. If start_time is not represented accurately, a seek attempt to start_time may well cause a seek after, or before the actual start of the stream, because the error is at most AV_TIME_BASE_Q, not AVStream->time_base. Regards, Marton > ffmpeg.c seek_to_start() call avformat_seek_file() with max_ts equal to ts. > If avformat_seek_file() should be kept as it is, then the caller should be fixed. > >> Regards, >> Marton >> >> >>> >>> max_ts = ts = -25057 >>> time_base = (num = 1, den = 14112000) >>> >>> After rescale, ts = -353604, and max_ts = -353605. >>> --- >>> libavformat/utils.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/libavformat/utils.c b/libavformat/utils.c >>> index 503e583ad0..69a0f901b2 100644 >>> --- a/libavformat/utils.c >>> +++ b/libavformat/utils.c >>> @@ -2500,10 +2500,10 @@ int avformat_seek_file(AVFormatContext *s, int stream_index, int64_t min_ts, >>> ts = av_rescale_q(ts, AV_TIME_BASE_Q, time_base); >>> min_ts = av_rescale_rnd(min_ts, time_base.den, >>> time_base.num * (int64_t)AV_TIME_BASE, >>> - AV_ROUND_UP | AV_ROUND_PASS_MINMAX); >>> + AV_ROUND_DOWN | AV_ROUND_PASS_MINMAX); >>> max_ts = av_rescale_rnd(max_ts, time_base.den, >>> time_base.num * (int64_t)AV_TIME_BASE, >>> - AV_ROUND_DOWN | AV_ROUND_PASS_MINMAX); >>> + AV_ROUND_UP | AV_ROUND_PASS_MINMAX); >>> stream_index = 0; >>> } >>> -- >>> 2.28.0 >>> >>> _______________________________________________ >>> 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". > > _______________________________________________ > 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/utils.c b/libavformat/utils.c index 503e583ad0..69a0f901b2 100644 --- a/libavformat/utils.c +++ b/libavformat/utils.c @@ -2500,10 +2500,10 @@ int avformat_seek_file(AVFormatContext *s, int stream_index, int64_t min_ts, ts = av_rescale_q(ts, AV_TIME_BASE_Q, time_base); min_ts = av_rescale_rnd(min_ts, time_base.den, time_base.num * (int64_t)AV_TIME_BASE, - AV_ROUND_UP | AV_ROUND_PASS_MINMAX); + AV_ROUND_DOWN | AV_ROUND_PASS_MINMAX); max_ts = av_rescale_rnd(max_ts, time_base.den, time_base.num * (int64_t)AV_TIME_BASE, - AV_ROUND_DOWN | AV_ROUND_PASS_MINMAX); + AV_ROUND_UP | AV_ROUND_PASS_MINMAX); stream_index = 0; }