| Message ID | 20231123191524.11296-4-anton@khirnov.net |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,01/13] lavfi/buffersink: avoid leaking peeked_frame on uninit | 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 |
Anton Khirnov (12023-11-23): > Avoid making decisions based on current graph input state, which makes > the output dependent on the order in which the frames from different > inputs are interleaved. > > Makes the output of fate-filter-overlay-dvdsub-2397 more correct - the > subtitle appears two frames later, which is closer to its PTS as stored > in the file. > --- > fftools/ffmpeg_filter.c | 3 +-- > tests/ref/fate/filter-overlay-dvdsub-2397 | 4 ++-- > tests/ref/fate/sub2video | 8 +++++--- > 3 files changed, 8 insertions(+), 7 deletions(-) Just as I warned you, it breaks the test case I suggested: ./ffmpeg_g -xerror -i /tmp/dummy_with_sub.mkv -preset ultrafast -lavfi '[0:s]setpts=PTS+60/TB[s] ; [0:v][s]overlay' -y /tmp/dummy_with_hardsub.mkv (/tmp/dummy_with_sub.mkv is created like I told a few days ago) thousands of frame queued, eventually failing on OOM. Regards,
Nicolas George (12023-11-27): > Just as I warned you, it breaks the test case I suggested: > > ./ffmpeg_g -xerror -i /tmp/dummy_with_sub.mkv -preset ultrafast -lavfi '[0:s]setpts=PTS+60/TB[s] ; [0:v][s]overlay' -y /tmp/dummy_with_hardsub.mkv > > (/tmp/dummy_with_sub.mkv is created like I told a few days ago) > thousands of frame queued, eventually failing on OOM. … And it fails with just this patch but also with the whole patch series.
On Mon, Nov 27, 2023 at 10:43 AM Nicolas George <george@nsup.org> wrote: > Nicolas George (12023-11-27): > > Just as I warned you, it breaks the test case I suggested: > > > > ./ffmpeg_g -xerror -i /tmp/dummy_with_sub.mkv -preset ultrafast -lavfi > '[0:s]setpts=PTS+60/TB[s] ; [0:v][s]overlay' -y /tmp/dummy_with_hardsub.mkv > > > > (/tmp/dummy_with_sub.mkv is created like I told a few days ago) > > thousands of frame queued, eventually failing on OOM. > > … And it fails with just this patch but also with the whole patch > series. > Looks unrelated issue I just fixed, and sent patch to ML. > > -- > Nicolas George > _______________________________________________ > 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". >
Paul B Mahol (12023-11-27):
> Looks unrelated issue I just fixed, and sent patch to ML.
No, it does not change anything, still “queued = 1081”. You could have
tested.
On Mon, Nov 27, 2023 at 2:49 PM Nicolas George <george@nsup.org> wrote: > Paul B Mahol (12023-11-27): > > Looks unrelated issue I just fixed, and sent patch to ML. > > No, it does not change anything, still “queued = 1081”. You could have > tested. > I tested it, it passed: -bash: mkvmerge: command not found > > -- > Nicolas George > _______________________________________________ > 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". >
Quoting Nicolas George (2023-11-27 10:40:18) > Anton Khirnov (12023-11-23): > > Avoid making decisions based on current graph input state, which makes > > the output dependent on the order in which the frames from different > > inputs are interleaved. > > > > Makes the output of fate-filter-overlay-dvdsub-2397 more correct - the > > subtitle appears two frames later, which is closer to its PTS as stored > > in the file. > > --- > > fftools/ffmpeg_filter.c | 3 +-- > > tests/ref/fate/filter-overlay-dvdsub-2397 | 4 ++-- > > tests/ref/fate/sub2video | 8 +++++--- > > 3 files changed, 8 insertions(+), 7 deletions(-) > > Just as I warned you, it breaks the test case I suggested: > > ./ffmpeg_g -xerror -i /tmp/dummy_with_sub.mkv -preset ultrafast -lavfi '[0:s]setpts=PTS+60/TB[s] ; [0:v][s]overlay' -y /tmp/dummy_with_hardsub.mkv > > (/tmp/dummy_with_sub.mkv is created like I told a few days ago) > thousands of frame queued, eventually failing on OOM. You're offsetting two streams from the same file by 60 seconds, so you should expect about 60s of buffering - that is 1500 frames at 25fps. The maximum amount of frames I see buffered is 1602, which roughly corresponds to the expected number. So I don't think this demonstrates there actually is a problem. Also note that the output timestamps look better after the patch than before.
diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c index b7da105141..b6fbc5b195 100644 --- a/fftools/ffmpeg_filter.c +++ b/fftools/ffmpeg_filter.c @@ -2274,8 +2274,7 @@ void ifilter_sub2video_heartbeat(InputFilter *ifilter, int64_t pts, AVRational t or if we need to initialize the system, update the overlayed subpicture and its start/end times */ sub2video_update(ifp, pts2 + 1, NULL); - - if (av_buffersrc_get_nb_failed_requests(ifp->filter)) + else sub2video_push_ref(ifp, pts2); } diff --git a/tests/ref/fate/filter-overlay-dvdsub-2397 b/tests/ref/fate/filter-overlay-dvdsub-2397 index 7df4f50776..45c026f540 100644 --- a/tests/ref/fate/filter-overlay-dvdsub-2397 +++ b/tests/ref/fate/filter-overlay-dvdsub-2397 @@ -489,12 +489,12 @@ 1, 3877, 3877, 10, 2013, 0x95a39f9c 1, 3887, 3887, 10, 2013, 0x4f7ea123 1, 3897, 3897, 10, 2013, 0x9efb9ba1 -0, 117, 117, 1, 518400, 0xbf8523da +0, 117, 117, 1, 518400, 0x61e0f688 1, 3907, 3907, 10, 2013, 0xf395b2cd 1, 3917, 3917, 10, 2013, 0x261a881e 1, 3927, 3927, 10, 2013, 0x7f2d9f72 1, 3937, 3937, 10, 2013, 0x0105b38d -0, 118, 118, 1, 518400, 0x41890ed6 +0, 118, 118, 1, 518400, 0xa47de755 1, 3952, 3952, 10, 2013, 0x0e5db67e 1, 3962, 3962, 10, 2013, 0xfc9baf97 0, 119, 119, 1, 518400, 0x588534fc diff --git a/tests/ref/fate/sub2video b/tests/ref/fate/sub2video index 80abe9c905..76347322f3 100644 --- a/tests/ref/fate/sub2video +++ b/tests/ref/fate/sub2video @@ -68,7 +68,8 @@ 0, 258, 258, 1, 518400, 0x34cdddee 0, 269, 269, 1, 518400, 0xbab197ea 1, 53910000, 53910000, 2696000, 2095, 0x61bb15ed -0, 270, 270, 1, 518400, 0x4db4ce51 +0, 270, 270, 1, 518400, 0xbab197ea +0, 271, 271, 1, 518400, 0x4db4ce51 0, 283, 283, 1, 518400, 0xbab197ea 1, 56663000, 56663000, 1262000, 1013, 0xc9ae89b7 0, 284, 284, 1, 518400, 0xe6bc0ea9 @@ -137,7 +138,7 @@ 1, 168049000, 168049000, 1900000, 1312, 0x0bf20e8d 0, 850, 850, 1, 518400, 0xbab197ea 1, 170035000, 170035000, 1524000, 1279, 0xb6c2dafe -0, 851, 851, 1, 518400, 0x8780239e +0, 851, 851, 1, 518400, 0xbab197ea 0, 858, 858, 1, 518400, 0xbab197ea 0, 861, 861, 1, 518400, 0x6eb72347 1, 172203000, 172203000, 1695000, 1826, 0x9a1ac769 @@ -161,7 +162,8 @@ 0, 976, 976, 1, 518400, 0x923d1ce7 0, 981, 981, 1, 518400, 0xbab197ea 1, 196361000, 196361000, 1524000, 1715, 0x695ca41e -0, 982, 982, 1, 518400, 0x6e652cd2 +0, 982, 982, 1, 518400, 0xbab197ea +0, 983, 983, 1, 518400, 0x6e652cd2 0, 989, 989, 1, 518400, 0xbab197ea 1, 197946000, 197946000, 1160000, 789, 0xc63a189e 0, 990, 990, 1, 518400, 0x25113966