| Message ID | 20231215070046.771-1-ffmpeg@gyani.pro |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v2] swr/swresample: avoid reapplication of firstpts | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Fri, Dec 15, 2023 at 12:30:46PM +0530, Gyan Doshi wrote: > During a resampling operation where > > 1) user has specified first_pts > 2) SWR_FLAG_RESAMPLE is not set initially (directly or otherwise) > 3) first_pts has been fulfilled (always using hard compensation) > > then upon first encountering a delay where a soft compensation is > required, swr_set_compensation will lead to another init of swr which > will reset outpts to the specified firstpts thus leading to an output > frame having its pts = firstpts. When the next input frame is received, > swr will see a large delay and inject silence from firstpts to the > current frame's pts. This can lead to severe desync and in worst case, > loss of audio playback. > > Parameter firstpts initialized to AV_NOPTS_VALUE in swr_alloc and then > checked in swr_init to avoid resetting outpts, thus avoiding reapplication > of firstpts. > > Fixes #4131. > --- > libswresample/options.c | 1 + > libswresample/swresample.c | 5 +++-- > 2 files changed, 4 insertions(+), 2 deletions(-) LGTM also can a fate test be added for this ? thx [...]
On 2023-12-15 11:46 pm, Michael Niedermayer wrote: > On Fri, Dec 15, 2023 at 12:30:46PM +0530, Gyan Doshi wrote: >> During a resampling operation where >> >> 1) user has specified first_pts >> 2) SWR_FLAG_RESAMPLE is not set initially (directly or otherwise) >> 3) first_pts has been fulfilled (always using hard compensation) >> >> then upon first encountering a delay where a soft compensation is >> required, swr_set_compensation will lead to another init of swr which >> will reset outpts to the specified firstpts thus leading to an output >> frame having its pts = firstpts. When the next input frame is received, >> swr will see a large delay and inject silence from firstpts to the >> current frame's pts. This can lead to severe desync and in worst case, >> loss of audio playback. >> >> Parameter firstpts initialized to AV_NOPTS_VALUE in swr_alloc and then >> checked in swr_init to avoid resetting outpts, thus avoiding reapplication >> of firstpts. >> >> Fixes #4131. >> --- >> libswresample/options.c | 1 + >> libswresample/swresample.c | 5 +++-- >> 2 files changed, 4 insertions(+), 2 deletions(-) > LGTM > > also can a fate test be added for this ? v3 with fate test sent. Regards, Gyan
diff --git a/libswresample/options.c b/libswresample/options.c index fb109fdbab..d8cf85c053 100644 --- a/libswresample/options.c +++ b/libswresample/options.c @@ -171,6 +171,7 @@ av_cold struct SwrContext *swr_alloc(void){ if(s){ s->av_class= &av_class; av_opt_set_defaults(s); + s->firstpts = AV_NOPTS_VALUE; } return s; } diff --git a/libswresample/swresample.c b/libswresample/swresample.c index f2a9b40474..1cf83a803f 100644 --- a/libswresample/swresample.c +++ b/libswresample/swresample.c @@ -375,8 +375,9 @@ av_cold int swr_init(struct SwrContext *s){ if (s->firstpts_in_samples != AV_NOPTS_VALUE) { if (!s->async && s->min_compensation >= FLT_MAX/2) s->async = 1; - s->firstpts = - s->outpts = s->firstpts_in_samples * s->out_sample_rate; + if (s->firstpts == AV_NOPTS_VALUE) + s->firstpts = + s->outpts = s->firstpts_in_samples * s->out_sample_rate; } else s->firstpts = AV_NOPTS_VALUE;