| Message ID | 20161223204952.7804-1-george@nsup.org |
|---|---|
| State | Accepted |
| Commit | 0ff5567a30be6d7c804e95997ae282d6bacd76c3 |
| Headers | show |
On Fri, Dec 23, 2016 at 09:49:52PM +0100, Nicolas George wrote: > Reduce peak memory consumption with ffmpeg in certain cases. > > Signed-off-by: Nicolas George <george@nsup.org> > --- > libavfilter/buffersrc.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) deadlocks ./ffmpeg -i mpegts_with_dvbsubs.ts -filter_complex '[#0xf30]scale=320:240:flags=bilinear[sub];[#0xfa8]scale=320:240:flags=bilinear[video];[video][sub]overlay[v]' -map '[v]' -map '#0x1048' -t 1 a.mkv http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket4074/mpegts_with_dvbsubs.ts also this patch changes the output of ffmpeg in many cases [...]
On Sat, Dec 24, 2016 at 03:10:28AM +0100, Michael Niedermayer wrote: > On Fri, Dec 23, 2016 at 09:49:52PM +0100, Nicolas George wrote: > > Reduce peak memory consumption with ffmpeg in certain cases. > > > > Signed-off-by: Nicolas George <george@nsup.org> > > --- > > libavfilter/buffersrc.c | 25 +++++++++++++++++++++++++ > > 1 file changed, 25 insertions(+) > > deadlocks > > ./ffmpeg -i mpegts_with_dvbsubs.ts -filter_complex '[#0xf30]scale=320:240:flags=bilinear[sub];[#0xfa8]scale=320:240:flags=bilinear[video];[video][sub]overlay[v]' -map '[v]' -map '#0x1048' -t 1 a.mkv > > http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket4074/mpegts_with_dvbsubs.ts > > also this patch changes the output of ffmpeg in many cases heres one case that shows artifacts create a file with: (can be any ffmpeg version) ./ffmpeg -i matrixbench_mpeg2.mpg -an -vcodec utvideo -t 1 -pix_fmt yuv420p -pred median utv.avi if this is played with ffplay (with patch) it shows a brighter artifact on the left side of te screen [...]
On Sat, Dec 24, 2016 at 03:18:11AM +0100, Michael Niedermayer wrote: > On Sat, Dec 24, 2016 at 03:10:28AM +0100, Michael Niedermayer wrote: > > On Fri, Dec 23, 2016 at 09:49:52PM +0100, Nicolas George wrote: > > > Reduce peak memory consumption with ffmpeg in certain cases. > > > > > > Signed-off-by: Nicolas George <george@nsup.org> > > > --- > > > libavfilter/buffersrc.c | 25 +++++++++++++++++++++++++ > > > 1 file changed, 25 insertions(+) > > > > deadlocks > > > > ./ffmpeg -i mpegts_with_dvbsubs.ts -filter_complex '[#0xf30]scale=320:240:flags=bilinear[sub];[#0xfa8]scale=320:240:flags=bilinear[video];[video][sub]overlay[v]' -map '[v]' -map '#0x1048' -t 1 a.mkv > > > > http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket4074/mpegts_with_dvbsubs.ts > > > > also this patch changes the output of ffmpeg in many cases > > heres one case that shows artifacts > create a file with: (can be any ffmpeg version) > ./ffmpeg -i matrixbench_mpeg2.mpg -an -vcodec utvideo -t 1 -pix_fmt yuv420p -pred median utv.avi > > if this is played with ffplay (with patch) it shows a brighter > artifact on the left side of te screen the utvideo change is caused by ea93052db3594f93f2d10be085a770184da0513d. not by this patch. i had both locally and just now double checked [...]
Le quartidi 4 nivôse, an CCXXV, Michael Niedermayer a écrit : > deadlocks > > ./ffmpeg -i mpegts_with_dvbsubs.ts -filter_complex '[#0xf30]scale=320:240:flags=bilinear[sub];[#0xfa8]scale=320:240:flags=bilinear[video];[video][sub]overlay[v]' -map '[v]' -map '#0x1048' -t 1 a.mkv > > http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket4074/mpegts_with_dvbsubs.ts Thanks for noticing. See the two incoming patches. > also this patch changes the output of ffmpeg in many cases Were these all related to the utvideo change like the one you shared? If not, can you point one? FATE detects nothing. Regards,
On Sat, Dec 24, 2016 at 10:39:58AM +0100, Nicolas George wrote: > Le quartidi 4 nivôse, an CCXXV, Michael Niedermayer a écrit : > > deadlocks > > > > ./ffmpeg -i mpegts_with_dvbsubs.ts -filter_complex '[#0xf30]scale=320:240:flags=bilinear[sub];[#0xfa8]scale=320:240:flags=bilinear[video];[video][sub]overlay[v]' -map '[v]' -map '#0x1048' -t 1 a.mkv > > > > http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket4074/mpegts_with_dvbsubs.ts > > Thanks for noticing. See the two incoming patches. > > > also this patch changes the output of ffmpeg in many cases > > Were these all related to the utvideo change like the one you shared? If > not, can you point one? FATE detects nothing. theres a change with "-af apad=pad_len=10000 -shortest" (no -vframes) the file is not sharable, this change undoes the effect of the non-recursive patch use of vframes generally shows differences which seem to return to what things were before the "non recursive patch" [...]
Le quartidi 4 nivôse, an CCXXV, Michael Niedermayer a écrit : > theres a change with "-af apad=pad_len=10000 -shortest" (no -vframes) > the file is not sharable, this change undoes the effect of the > non-recursive patch > > use of vframes generally shows differences which seem to return to what > things were before the "non recursive patch" Thanks for checking. Then I shall push shortly. Regards,
On 12/24/16, Nicolas George <george@nsup.org> wrote: > Le quartidi 4 nivose, an CCXXV, Michael Niedermayer a ecrit : >> theres a change with "-af apad=pad_len=10000 -shortest" (no -vframes) >> the file is not sharable, this change undoes the effect of the >> non-recursive patch >> >> use of vframes generally shows differences which seem to return to what >> things were before the "non recursive patch" > > Thanks for checking. Then I shall push shortly. This breaks :shortest=1 in all framesync filters, so I'm for reverting this.
On Tue, Jun 20, 2017 at 11:29 PM, Paul B Mahol <onemda@gmail.com> wrote: > On 12/24/16, Nicolas George <george@nsup.org> wrote: > > Le quartidi 4 nivose, an CCXXV, Michael Niedermayer a ecrit : > >> theres a change with "-af apad=pad_len=10000 -shortest" (no -vframes) > >> the file is not sharable, this change undoes the effect of the > >> non-recursive patch > >> > >> use of vframes generally shows differences which seem to return to what > >> things were before the "non recursive patch" > > > > Thanks for checking. Then I shall push shortly. > > This breaks :shortest=1 in all framesync filters, so I'm for reverting > this. > Thanks. Please close ticket 6292 when you do.
On Tue, Jun 20, 2017 at 07:59:26PM +0200, Paul B Mahol wrote: > On 12/24/16, Nicolas George <george@nsup.org> wrote: > > Le quartidi 4 nivose, an CCXXV, Michael Niedermayer a ecrit : > >> theres a change with "-af apad=pad_len=10000 -shortest" (no -vframes) > >> the file is not sharable, this change undoes the effect of the > >> non-recursive patch > >> > >> use of vframes generally shows differences which seem to return to what > >> things were before the "non recursive patch" > > > > Thanks for checking. Then I shall push shortly. > > This breaks :shortest=1 in all framesync filters, so I'm for reverting this. The revert broke alot I would suggest to undo the revert and go through the normal review process examples of breakages: ./ffmpeg -i bgc.sub.dub.ogm -vframes 3 subs.webm (produces 1 instead of 3 frames in the output) ./ffmpeg -i 2014-10-17\ 11.31\ i95Dev\ -\ Carlo\ Pazolini\ _\ KWI\ -\ Meeting.g2m -max_muxing_queue_size 8000 -vframes 2 g2m5.avi fails: [libmp3lame @ 0x3a5aee0] Queue input is backward in time [avi @ 0x3a5e280] Too large number of skipped frames 194211 > 60000 54.1kbits/s speed= 129x av_interleaved_write_frame(): Invalid argument [avi @ 0x3a5e280] Too large number of skipped frames 194111 > 60000 frame= 2 fps=1.1 q=2.0 Lsize= 1857kB time=00:03:14.35 bitrate= 78.3kbits/s speed= 103x video:149kB audio:1519kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 11.380948% Conversion failed! (iam not sure i can share this one) ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -vframes 20 -vcodec libx265 -omit_video_pes_length 0 -pix_fmt yuv420p file2.ts the change results in the aditional warnings on playback of the resulting file: [mpegts @ 0x3d10100] start time for stream 1 is not set in estimate_timings_from_pts [mpegts @ 0x3d10100] Could not find codec parameters for stream 1 (Audio: mp3 ([3][0][0][0] / 0x0003), 0 channels, s16p): unspecified frame size Consider increasing the value for the 'analyzeduration' and 'probesize' options [...]
On Sat, Jun 24, 2017 at 04:54:38PM +0200, Michael Niedermayer wrote: > On Tue, Jun 20, 2017 at 07:59:26PM +0200, Paul B Mahol wrote: > > On 12/24/16, Nicolas George <george@nsup.org> wrote: > > > Le quartidi 4 nivose, an CCXXV, Michael Niedermayer a ecrit : > > >> theres a change with "-af apad=pad_len=10000 -shortest" (no -vframes) > > >> the file is not sharable, this change undoes the effect of the > > >> non-recursive patch > > >> > > >> use of vframes generally shows differences which seem to return to what > > >> things were before the "non recursive patch" > > > > > > Thanks for checking. Then I shall push shortly. > > > > This breaks :shortest=1 in all framesync filters, so I'm for reverting this. > > The revert broke alot > I would suggest to undo the revert and go through the normal review > process > > examples of breakages: > ./ffmpeg -i bgc.sub.dub.ogm -vframes 3 subs.webm > > (produces 1 instead of 3 frames in the output) > > > ./ffmpeg -i 2014-10-17\ 11.31\ i95Dev\ -\ Carlo\ Pazolini\ _\ KWI\ -\ Meeting.g2m -max_muxing_queue_size 8000 -vframes 2 g2m5.avi > > fails: > [libmp3lame @ 0x3a5aee0] Queue input is backward in time > [avi @ 0x3a5e280] Too large number of skipped frames 194211 > 60000 54.1kbits/s speed= 129x > av_interleaved_write_frame(): Invalid argument > [avi @ 0x3a5e280] Too large number of skipped frames 194111 > 60000 > frame= 2 fps=1.1 q=2.0 Lsize= 1857kB time=00:03:14.35 bitrate= 78.3kbits/s speed= 103x > video:149kB audio:1519kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 11.380948% > Conversion failed! > (iam not sure i can share this one) > > > ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -vframes 20 -vcodec libx265 -omit_video_pes_length 0 -pix_fmt yuv420p file2.ts > > the change results in the aditional warnings on playback of the resulting file: > [mpegts @ 0x3d10100] start time for stream 1 is not set in estimate_timings_from_pts > [mpegts @ 0x3d10100] Could not find codec parameters for stream 1 (Audio: mp3 ([3][0][0][0] / 0x0003), 0 channels, s16p): unspecified frame size > Consider increasing the value for the 'analyzeduration' and 'probesize' options For some files the code as in git works better for example: ffmpeg -i ~/tickets/3872/MJ._.Hold.My.Hand._with.Akon__HEVC.1080p.Aex_A__1__001.mkv -vframes 4 file.avi produces a better matching audio stream length now [...]
On 6/24/17, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Sat, Jun 24, 2017 at 04:54:38PM +0200, Michael Niedermayer wrote: >> On Tue, Jun 20, 2017 at 07:59:26PM +0200, Paul B Mahol wrote: >> > On 12/24/16, Nicolas George <george@nsup.org> wrote: >> > > Le quartidi 4 nivose, an CCXXV, Michael Niedermayer a ecrit : >> > >> theres a change with "-af apad=pad_len=10000 -shortest" (no >> > >> -vframes) >> > >> the file is not sharable, this change undoes the effect of the >> > >> non-recursive patch >> > >> >> > >> use of vframes generally shows differences which seem to return to >> > >> what >> > >> things were before the "non recursive patch" >> > > >> > > Thanks for checking. Then I shall push shortly. >> > >> > This breaks :shortest=1 in all framesync filters, so I'm for reverting >> > this. >> >> The revert broke alot >> I would suggest to undo the revert and go through the normal review >> process >> >> examples of breakages: >> ./ffmpeg -i bgc.sub.dub.ogm -vframes 3 subs.webm >> >> (produces 1 instead of 3 frames in the output) >> >> >> ./ffmpeg -i 2014-10-17\ 11.31\ i95Dev\ -\ Carlo\ Pazolini\ _\ KWI\ -\ >> Meeting.g2m -max_muxing_queue_size 8000 -vframes 2 g2m5.avi >> >> fails: >> [libmp3lame @ 0x3a5aee0] Queue input is backward in time >> [avi @ 0x3a5e280] Too large number of skipped frames 194211 > 60000 >> 54.1kbits/s speed= 129x >> av_interleaved_write_frame(): Invalid argument >> [avi @ 0x3a5e280] Too large number of skipped frames 194111 > 60000 >> frame= 2 fps=1.1 q=2.0 Lsize= 1857kB time=00:03:14.35 bitrate= >> 78.3kbits/s speed= 103x >> video:149kB audio:1519kB subtitle:0kB other streams:0kB global headers:0kB >> muxing overhead: 11.380948% >> Conversion failed! >> (iam not sure i can share this one) >> >> >> ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -vframes 20 -vcodec libx265 >> -omit_video_pes_length 0 -pix_fmt yuv420p file2.ts >> >> the change results in the aditional warnings on playback of the resulting >> file: >> [mpegts @ 0x3d10100] start time for stream 1 is not set in >> estimate_timings_from_pts >> [mpegts @ 0x3d10100] Could not find codec parameters for stream 1 (Audio: >> mp3 ([3][0][0][0] / 0x0003), 0 channels, s16p): unspecified frame size >> Consider increasing the value for the 'analyzeduration' and 'probesize' >> options > > For some files the code as in git works better > > for example: > ffmpeg -i > ~/tickets/3872/MJ._.Hold.My.Hand._with.Akon__HEVC.1080p.Aex_A__1__001.mkv > -vframes 4 file.avi > > produces a better matching audio stream length now The reverted commit breaks lot of filters and I failed to find fix, the only logical solution was to revert commmit. Also I'm not happy with current lavfi core state, its left in chaos.
On Sat, Jun 24, 2017 at 04:54:38PM +0200, Michael Niedermayer wrote: > On Tue, Jun 20, 2017 at 07:59:26PM +0200, Paul B Mahol wrote: > > On 12/24/16, Nicolas George <george@nsup.org> wrote: > > > Le quartidi 4 nivose, an CCXXV, Michael Niedermayer a ecrit : > > >> theres a change with "-af apad=pad_len=10000 -shortest" (no -vframes) > > >> the file is not sharable, this change undoes the effect of the > > >> non-recursive patch > > >> > > >> use of vframes generally shows differences which seem to return to what > > >> things were before the "non recursive patch" > > > > > > Thanks for checking. Then I shall push shortly. > > > > This breaks :shortest=1 in all framesync filters, so I'm for reverting this. > > The revert broke alot > I would suggest to undo the revert and go through the normal review > process > > examples of breakages: > ./ffmpeg -i bgc.sub.dub.ogm -vframes 3 subs.webm > > (produces 1 instead of 3 frames in the output) > > > ./ffmpeg -i 2014-10-17\ 11.31\ i95Dev\ -\ Carlo\ Pazolini\ _\ KWI\ -\ Meeting.g2m -max_muxing_queue_size 8000 -vframes 2 g2m5.avi > > fails: > [libmp3lame @ 0x3a5aee0] Queue input is backward in time > [avi @ 0x3a5e280] Too large number of skipped frames 194211 > 60000 54.1kbits/s speed= 129x > av_interleaved_write_frame(): Invalid argument > [avi @ 0x3a5e280] Too large number of skipped frames 194111 > 60000 > frame= 2 fps=1.1 q=2.0 Lsize= 1857kB time=00:03:14.35 bitrate= 78.3kbits/s speed= 103x > video:149kB audio:1519kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 11.380948% > Conversion failed! > (iam not sure i can share this one) > > > ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -vframes 20 -vcodec libx265 -omit_video_pes_length 0 -pix_fmt yuv420p file2.ts > > the change results in the aditional warnings on playback of the resulting file: > [mpegts @ 0x3d10100] start time for stream 1 is not set in estimate_timings_from_pts > [mpegts @ 0x3d10100] Could not find codec parameters for stream 1 (Audio: mp3 ([3][0][0][0] / 0x0003), 0 channels, s16p): unspecified frame size > Consider increasing the value for the 'analyzeduration' and 'probesize' options Heres an example without vframes: ./ffmpeg-good -v 0 -i ~/tickets/3539/crash.swf -map 0 -t 1 -f framecrc - vs. ./ffmpeg -v 0 -i ~/tickets/3539/crash.swf -map 0 -t 1 -f framecrc - new code returns fewer frames http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket3539/crash.swf [...]
On Sat, Jun 24, 2017 at 05:12:34PM +0200, Paul B Mahol wrote: > On 6/24/17, Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Sat, Jun 24, 2017 at 04:54:38PM +0200, Michael Niedermayer wrote: > >> On Tue, Jun 20, 2017 at 07:59:26PM +0200, Paul B Mahol wrote: > >> > On 12/24/16, Nicolas George <george@nsup.org> wrote: > >> > > Le quartidi 4 nivose, an CCXXV, Michael Niedermayer a ecrit : > >> > >> theres a change with "-af apad=pad_len=10000 -shortest" (no > >> > >> -vframes) > >> > >> the file is not sharable, this change undoes the effect of the > >> > >> non-recursive patch > >> > >> > >> > >> use of vframes generally shows differences which seem to return to > >> > >> what > >> > >> things were before the "non recursive patch" > >> > > > >> > > Thanks for checking. Then I shall push shortly. > >> > > >> > This breaks :shortest=1 in all framesync filters, so I'm for reverting > >> > this. > >> > >> The revert broke alot > >> I would suggest to undo the revert and go through the normal review > >> process > >> > >> examples of breakages: > >> ./ffmpeg -i bgc.sub.dub.ogm -vframes 3 subs.webm > >> > >> (produces 1 instead of 3 frames in the output) > >> > >> > >> ./ffmpeg -i 2014-10-17\ 11.31\ i95Dev\ -\ Carlo\ Pazolini\ _\ KWI\ -\ > >> Meeting.g2m -max_muxing_queue_size 8000 -vframes 2 g2m5.avi > >> > >> fails: > >> [libmp3lame @ 0x3a5aee0] Queue input is backward in time > >> [avi @ 0x3a5e280] Too large number of skipped frames 194211 > 60000 > >> 54.1kbits/s speed= 129x > >> av_interleaved_write_frame(): Invalid argument > >> [avi @ 0x3a5e280] Too large number of skipped frames 194111 > 60000 > >> frame= 2 fps=1.1 q=2.0 Lsize= 1857kB time=00:03:14.35 bitrate= > >> 78.3kbits/s speed= 103x > >> video:149kB audio:1519kB subtitle:0kB other streams:0kB global headers:0kB > >> muxing overhead: 11.380948% > >> Conversion failed! > >> (iam not sure i can share this one) > >> > >> > >> ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -vframes 20 -vcodec libx265 > >> -omit_video_pes_length 0 -pix_fmt yuv420p file2.ts > >> > >> the change results in the aditional warnings on playback of the resulting > >> file: > >> [mpegts @ 0x3d10100] start time for stream 1 is not set in > >> estimate_timings_from_pts > >> [mpegts @ 0x3d10100] Could not find codec parameters for stream 1 (Audio: > >> mp3 ([3][0][0][0] / 0x0003), 0 channels, s16p): unspecified frame size > >> Consider increasing the value for the 'analyzeduration' and 'probesize' > >> options > > > > For some files the code as in git works better > > > > for example: > > ffmpeg -i > > ~/tickets/3872/MJ._.Hold.My.Hand._with.Akon__HEVC.1080p.Aex_A__1__001.mkv > > -vframes 4 file.avi > > > > produces a better matching audio stream length now > > The reverted commit breaks lot of filters and I failed to find fix, > the only logical solution > was to revert commmit. I understand, but it kind of sucks as solution because either way some things are broken. It would be better if there was some change that would make all or most things work. Not fix some and break others > > Also I'm not happy with current lavfi core state, its left in chaos. I wish i had more time (and motivation) to work on lavfi, i must admit i did not follow lavfi core changes as close as i ideally should have so iam not entirely aware of how exactly things look ... [...]
On Sat, Jun 24, 2017 at 05:06:41PM +0200, Michael Niedermayer wrote: > On Sat, Jun 24, 2017 at 04:54:38PM +0200, Michael Niedermayer wrote: > > On Tue, Jun 20, 2017 at 07:59:26PM +0200, Paul B Mahol wrote: > > > On 12/24/16, Nicolas George <george@nsup.org> wrote: > > > > Le quartidi 4 nivose, an CCXXV, Michael Niedermayer a ecrit : > > > >> theres a change with "-af apad=pad_len=10000 -shortest" (no -vframes) > > > >> the file is not sharable, this change undoes the effect of the > > > >> non-recursive patch > > > >> > > > >> use of vframes generally shows differences which seem to return to what > > > >> things were before the "non recursive patch" > > > > > > > > Thanks for checking. Then I shall push shortly. > > > > > > This breaks :shortest=1 in all framesync filters, so I'm for reverting this. > > > > The revert broke alot > > I would suggest to undo the revert and go through the normal review > > process > > > > examples of breakages: > > ./ffmpeg -i bgc.sub.dub.ogm -vframes 3 subs.webm http://samples.ffmpeg.org/ogg/bgc.sub.dub.ogm [...]
Le sextidi 6 messidor, an CCXXV, Paul B Mahol a écrit : > The reverted commit breaks lot of filters and I failed to find fix, > the only logical solution It breaks a corner-case option of, indeed a lot of, filters. I have not yet been able to track down the exact cause of the problem, I still have all this in my work tree. Regards,
On Sat, 24 Jun 2017, Nicolas George wrote: > Le sextidi 6 messidor, an CCXXV, Paul B Mahol a écrit : >> The reverted commit breaks lot of filters and I failed to find fix, >> the only logical solution > > It breaks a corner-case option of, indeed a lot of, filters. I have not > yet been able to track down the exact cause of the problem, I still have > all this in my work tree. > The revert itself broke a rather simple buffersrc->buffersink case as well: ffmpeg -f lavfi -i "testsrc[out0];sine[out1]" -bsf:a noise=dropamount=1 out.avi ffmpeg is now consuming all memory, it seems video frames are stuck in the filter graph forever. (the dropamount parameter to the noise bitstream filter was just committed to be able to drop all packets of a stream) Regards, Marton
On 7/9/17, Marton Balint <cus@passwd.hu> wrote: > > On Sat, 24 Jun 2017, Nicolas George wrote: > >> Le sextidi 6 messidor, an CCXXV, Paul B Mahol a ecrit : >>> The reverted commit breaks lot of filters and I failed to find fix, >>> the only logical solution >> >> It breaks a corner-case option of, indeed a lot of, filters. I have not >> yet been able to track down the exact cause of the problem, I still have >> all this in my work tree. >> > > The revert itself broke a rather simple buffersrc->buffersink case as > well: > > ffmpeg -f lavfi -i "testsrc[out0];sine[out1]" -bsf:a noise=dropamount=1 > out.avi > > ffmpeg is now consuming all memory, it seems video frames are stuck in the > filter graph forever. > > (the dropamount parameter to the noise bitstream filter was just committed > to be able to drop all packets of a stream) I believe you are referring to another commit which also needs to be reverted.
On Sun, 9 Jul 2017, Paul B Mahol wrote: > On 7/9/17, Marton Balint <cus@passwd.hu> wrote: >> >> On Sat, 24 Jun 2017, Nicolas George wrote: >> >>> Le sextidi 6 messidor, an CCXXV, Paul B Mahol a ecrit : >>>> The reverted commit breaks lot of filters and I failed to find fix, >>>> the only logical solution >>> >>> It breaks a corner-case option of, indeed a lot of, filters. I have not >>> yet been able to track down the exact cause of the problem, I still have >>> all this in my work tree. >>> >> >> The revert itself broke a rather simple buffersrc->buffersink case as >> well: >> >> ffmpeg -f lavfi -i "testsrc[out0];sine[out1]" -bsf:a noise=dropamount=1 >> out.avi >> >> ffmpeg is now consuming all memory, it seems video frames are stuck in the >> filter graph forever. >> >> (the dropamount parameter to the noise bitstream filter was just committed >> to be able to drop all packets of a stream) > > I believe you are referring to another commit which also needs to be reverted. No, definitely this one (04aa09c4bcf2d5a634a35da3a3ae3fc1abe30ef8). The input frames are arriving just fine, so ffmpeg can initialize the filtergraph, but only one video frame comes out of the filtergraph, the others are stuck there forever. Regards, Marton
On 7/9/17, Marton Balint <cus@passwd.hu> wrote: > > On Sun, 9 Jul 2017, Paul B Mahol wrote: > >> On 7/9/17, Marton Balint <cus@passwd.hu> wrote: >>> >>> On Sat, 24 Jun 2017, Nicolas George wrote: >>> >>>> Le sextidi 6 messidor, an CCXXV, Paul B Mahol a ecrit : >>>>> The reverted commit breaks lot of filters and I failed to find fix, >>>>> the only logical solution >>>> >>>> It breaks a corner-case option of, indeed a lot of, filters. I have not >>>> yet been able to track down the exact cause of the problem, I still have >>>> all this in my work tree. >>>> >>> >>> The revert itself broke a rather simple buffersrc->buffersink case as >>> well: >>> >>> ffmpeg -f lavfi -i "testsrc[out0];sine[out1]" -bsf:a noise=dropamount=1 >>> out.avi >>> >>> ffmpeg is now consuming all memory, it seems video frames are stuck in >>> the >>> filter graph forever. >>> >>> (the dropamount parameter to the noise bitstream filter was just >>> committed >>> to be able to drop all packets of a stream) >> >> I believe you are referring to another commit which also needs to be >> reverted. > > No, definitely this one (04aa09c4bcf2d5a634a35da3a3ae3fc1abe30ef8). The > input frames are arriving just fine, so ffmpeg can initialize the > filtergraph, but only one video frame comes out of the filtergraph, the > others are stuck there forever. Nope. Without that revert bunch of filters are simply broken.
On Sun, 9 Jul 2017, Paul B Mahol wrote: > On 7/9/17, Marton Balint <cus@passwd.hu> wrote: >> >> On Sun, 9 Jul 2017, Paul B Mahol wrote: >> >>> On 7/9/17, Marton Balint <cus@passwd.hu> wrote: >>>> >>>> On Sat, 24 Jun 2017, Nicolas George wrote: >>>> >>>>> Le sextidi 6 messidor, an CCXXV, Paul B Mahol a ecrit : >>>>>> The reverted commit breaks lot of filters and I failed to find fix, >>>>>> the only logical solution >>>>> >>>>> It breaks a corner-case option of, indeed a lot of, filters. I have not >>>>> yet been able to track down the exact cause of the problem, I still have >>>>> all this in my work tree. >>>>> >>>> >>>> The revert itself broke a rather simple buffersrc->buffersink case as >>>> well: >>>> >>>> ffmpeg -f lavfi -i "testsrc[out0];sine[out1]" -bsf:a noise=dropamount=1 >>>> out.avi >>>> >>>> ffmpeg is now consuming all memory, it seems video frames are stuck in >>>> the >>>> filter graph forever. >>>> >>>> (the dropamount parameter to the noise bitstream filter was just >>>> committed >>>> to be able to drop all packets of a stream) >>> >>> I believe you are referring to another commit which also needs to be >>> reverted. >> >> No, definitely this one (04aa09c4bcf2d5a634a35da3a3ae3fc1abe30ef8). The >> input frames are arriving just fine, so ffmpeg can initialize the >> filtergraph, but only one video frame comes out of the filtergraph, the >> others are stuck there forever. > > Nope. Without that revert bunch of filters are simply broken. I am not saying they aren't, I'm just showing a case where your revert is breaking another case which was working before. Nicolas also stated there can be cases where the old code (before the revert) worked better. I just hope that I found a simple enough case, so somebody can take a look and find the root cause. Regards, Marton
Le primidi 21 messidor, an CCXXV, Marton Balint a écrit : > I am not saying they aren't, I'm just showing a case where your revert is > breaking another case which was working before. Nicolas also stated there > can be cases where the old code (before the revert) worked better. I just > hope that I found a simple enough case, so somebody can take a look and find > the root cause. Thanks for stepping in. I will try to dot the ‘ı’s in this issue. From the start, libavfilter has had subtle issues in corner cases. These issue reside primarily with filters with several inputs and the handling of EOF. We all tried to get rid of them, but soon realized they are like lamination bubbles: once you think you are rid of one, another pops up a few centimeters away, the only solution is to redo almost the whole thing. I have been doing just that for libavfilter. And since doing it all at once is not an option, I have been doing it step by step. The consequence is that there is a large stitch between the new and the old code, that may be as annoying as the bubbles, and even more so when the stitch approaches areas where there are bubbles. Dropping the metaphor: I have been reworking the scheduling of libavfilter to get filter with multiple inputs and EOF working, but look, I don't mean to be rude, but this is not as easy as it looks. Since changing all the filters is not an option, I had to include a compatibility wrapper to have the old implementations work in the new design. This compatibility wrapper is obviously not perfect. In fact, it probably cannot be perfect, because if it could, we would not need it in the first place. It took time, but the new design is there, and as far as I know, it works. Filters with several inputs are activated when needed, EOF is propagated with its timestamp (but ffmpeg does not inject it because the patch series implementing it was blocked by bikeshedding). The only thing that does not work is the existing filters. That makes quite a big “only”, I grant you. Now, I realize how frustrating to have a feature in a filter that used to work and no longer does, especially if it is a feature that you implemented yourself lovingly, and I sympathize. My goal, obviously, is to have everything working. But it takes time, especially when alone. Also, there have been unexpected stuff in my personal life that have left me with less time than before to work on ffmpeg. Now, if a feature has been broken and you want it back, you can have two options: you can revert, or you can help move forward. If you revert, you will possibly fix the feature, but you will also bring back all the issues that were fixed by these commits. Plus possibly other issues due to code changes that expect the new behaviour. And thus, you will have to revert and revert and revert. Also, you make more work for me, delaying by that much the time where the work is done. If you want to help move forward, on the other hand... IMO, it is the only sane option. Now, I also realize that helping move forward is easier said than done, even more so since I have not documented the new design. Documentation takes time, documenting in details something that does not work yet is a waste of time. Still, there is already quite a lot in the doxygen comments, and if somebody wants to help, I will give any information needed. And I will do so for the issue at hand. The problem is that with the "shortest" option, filters that use the "dualinput" system do not report EOF correctly. I have not yet been able to determine if the problem is in the dualinput system, not returning an error when it should, or in the wrapper code, not taking the return code into account properly. Still, I am not sure understanding that and fixing it would be a good use of the time, because it may not be possible, and because the filters with several inputs need to be rewritten with the new framework in mind: only then can they reap the benefits, especially getting rid of the "buffer queue overflow" problems. So, how to write a filter like that with the new framework? There is a single callback, called "activate". It must, in succession: - Examine the input links (using: http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170615/4490ac33/attachment.patch ): - If a link has no queued frames but a status, use ff_inlink_acknowledge_status(). - If the queued frames (and if relevant status) allow to produce a frame on output, do it and return. - If the input status change allow to deduce that one output or several are finished, report it with ff_avfilter_link_set_in_status() and return. - Examine the output links (using a wrapper, to plan for future evolutions such as mutexes) - If frame_wanted_out is set on an output, find out which inputs would require a frame to allow to produce one on that output and request them using ff_inlink_request_frame(). - If nothing could be done, just return FFERROR_NOT_READY (it will need to be moved into filters.h). I think doing that work for the dualinput / framesync filters is not a very complex work, but I am afraid it will require similar changes to all filters using it. (A side note about lamination: if what you want to laminate is water-resistant, adding a layer of soapy water will help: it will not damage the adhesive but slow it down, and the bubbles, filled with water instead of air, are much easier to lead to the edges. But do not do that to apply a screen protector on a capacitive touchscreen, they do not like to get wet!) Regards,
On 7/11/17, Nicolas George <george@nsup.org> wrote: > Le primidi 21 messidor, an CCXXV, Marton Balint a ecrit : >> I am not saying they aren't, I'm just showing a case where your revert is >> breaking another case which was working before. Nicolas also stated there >> can be cases where the old code (before the revert) worked better. I just >> hope that I found a simple enough case, so somebody can take a look and >> find >> the root cause. > > Thanks for stepping in. I will try to dot the `i's in this issue. > > From the start, libavfilter has had subtle issues in corner cases. These > issue reside primarily with filters with several inputs and the handling > of EOF. We all tried to get rid of them, but soon realized they are like > lamination bubbles: once you think you are rid of one, another pops up a > few centimeters away, the only solution is to redo almost the whole > thing. > > I have been doing just that for libavfilter. And since doing it all at > once is not an option, I have been doing it step by step. The > consequence is that there is a large stitch between the new and the old > code, that may be as annoying as the bubbles, and even more so when the > stitch approaches areas where there are bubbles. > > Dropping the metaphor: I have been reworking the scheduling of > libavfilter to get filter with multiple inputs and EOF working, but > look, I don't mean to be rude, but this is not as easy as it looks. > > Since changing all the filters is not an option, I had to include a > compatibility wrapper to have the old implementations work in the new > design. This compatibility wrapper is obviously not perfect. In fact, it > probably cannot be perfect, because if it could, we would not need it in > the first place. > > It took time, but the new design is there, and as far as I know, it > works. Filters with several inputs are activated when needed, EOF is > propagated with its timestamp (but ffmpeg does not inject it because the > patch series implementing it was blocked by bikeshedding). The only > thing that does not work is the existing filters. That makes quite a big > "only", I grant you. > > Now, I realize how frustrating to have a feature in a filter that used > to work and no longer does, especially if it is a feature that you > implemented yourself lovingly, and I sympathize. > > My goal, obviously, is to have everything working. But it takes time, > especially when alone. > > Also, there have been unexpected stuff in my personal life that have > left me with less time than before to work on ffmpeg. > > Now, if a feature has been broken and you want it back, you can have two > options: you can revert, or you can help move forward. > > If you revert, you will possibly fix the feature, but you will also > bring back all the issues that were fixed by these commits. Plus > possibly other issues due to code changes that expect the new behaviour. > And thus, you will have to revert and revert and revert. > > Also, you make more work for me, delaying by that much the time where > the work is done. > > If you want to help move forward, on the other hand... IMO, it is the > only sane option. > > Now, I also realize that helping move forward is easier said than done, > even more so since I have not documented the new design. Documentation > takes time, documenting in details something that does not work yet is a > waste of time. Still, there is already quite a lot in the doxygen > comments, and if somebody wants to help, I will give any information > needed. > > And I will do so for the issue at hand. > > The problem is that with the "shortest" option, filters that use the > "dualinput" system do not report EOF correctly. > > I have not yet been able to determine if the problem is in the dualinput > system, not returning an error when it should, or in the wrapper code, > not taking the return code into account properly. > > Still, I am not sure understanding that and fixing it would be a good > use of the time, because it may not be possible, and because the filters > with several inputs need to be rewritten with the new framework in mind: > only then can they reap the benefits, especially getting rid of the > "buffer queue overflow" problems. Nothing can get rid of that issue, except tootal lavfi rewrite. > > So, how to write a filter like that with the new framework? > > There is a single callback, called "activate". It must, in succession: > > - Examine the input links (using: > > http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170615/4490ac33/attachment.patch > ): > > - If a link has no queued frames but a status, use > ff_inlink_acknowledge_status(). > > - If the queued frames (and if relevant status) allow to produce a > frame on output, do it and return. > > - If the input status change allow to deduce that one output or > several are finished, report it with > ff_avfilter_link_set_in_status() and return. > > - Examine the output links (using a wrapper, to plan for future > evolutions such as mutexes) > > - If frame_wanted_out is set on an output, find out which inputs would > require a frame to allow to produce one on that output and request > them using ff_inlink_request_frame(). > > - If nothing could be done, just return FFERROR_NOT_READY (it will need > to be moved into filters.h). > > I think doing that work for the dualinput / framesync filters is not a > very complex work, but I am afraid it will require similar changes to > all filters using it. I tried this and it did not work at all. And I even contacted Nicolas privately and never got reply. > > > (A side note about lamination: if what you want to laminate is > water-resistant, adding a layer of soapy water will help: it will not > damage the adhesive but slow it down, and the bubbles, filled with water > instead of air, are much easier to lead to the edges. But do not do that > to apply a screen protector on a capacitive touchscreen, they do not > like to get wet!) > > Regards, > > -- > Nicolas George >
Le tridi 23 messidor, an CCXXV, Paul B Mahol a écrit : > Nothing can get rid of that issue, except tootal lavfi rewrite. Why do you say that? My plan gets rid of them quite easily, since now all links have a dynamically-sized FIFO built-in. > I tried this and it did not work at all. And I even contacted Nicolas > privately and never got reply. I found your mail in my inbox, and I vaguely remember reading them at the time, but no more. It was a very busy week. Sorry for missing them. Unfortunately, you neglected to include your code, so I cannot tell why it did not work. Regards,
diff --git a/libavfilter/buffersrc.c b/libavfilter/buffersrc.c index 1314397a32..77fd174219 100644 --- a/libavfilter/buffersrc.c +++ b/libavfilter/buffersrc.c @@ -173,6 +173,20 @@ int attribute_align_arg av_buffersrc_add_frame_flags(AVFilterContext *ctx, AVFra return ret; } +static int push_frame(AVFilterGraph *graph) +{ + int ret; + + while (1) { + ret = ff_filter_graph_run_once(graph); + if (ret == AVERROR(EAGAIN)) + break; + if (ret < 0) + return ret; + } + return 0; +} + static int av_buffersrc_add_frame_internal(AVFilterContext *ctx, AVFrame *frame, int flags) { @@ -185,6 +199,11 @@ static int av_buffersrc_add_frame_internal(AVFilterContext *ctx, if (!frame) { s->eof = 1; ff_avfilter_link_set_in_status(ctx->outputs[0], AVERROR_EOF, AV_NOPTS_VALUE); + if ((flags & AV_BUFFERSRC_FLAG_PUSH)) { + ret = push_frame(ctx->graph); + if (ret < 0) + return ret; + } return 0; } else if (s->eof) return AVERROR(EINVAL); @@ -239,6 +258,12 @@ static int av_buffersrc_add_frame_internal(AVFilterContext *ctx, if ((ret = ctx->output_pads[0].request_frame(ctx->outputs[0])) < 0) return ret; + if ((flags & AV_BUFFERSRC_FLAG_PUSH)) { + ret = push_frame(ctx->graph); + if (ret < 0) + return ret; + } + return 0; }
Reduce peak memory consumption with ffmpeg in certain cases. Signed-off-by: Nicolas George <george@nsup.org> --- libavfilter/buffersrc.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)