| Message ID | 20240424110257.38715-1-ffmpeg@haasn.xyz |
|---|---|
| Headers | show |
| Series | replace scale2ref by scale=rw:rh | expand |
On 2024-04-24 04:21 pm, Niklas Haas wrote: > As discussed in my previous series for fixing scale2ref[1], this filter > is fundamentally broken, and the only real fix would be to switch to > activate(), or ideally FFFrameSync. > > [1] https://ffmpeg.org//pipermail/ffmpeg-devel/2024-March/323382.html > > The main thing making this difficult is the fact that scale2ref also > wants to output ref frames to its secondary output, which FFFrameSync > does not support, and which is ultimately at least part of the root > cause of trac #10795. > > Since this is in principle completely unnecessary (users can just > 'split' the ref input and have it be consumed by vf_scale), and to make > the design of this filter a bit more robust and maintainable, switch to > an approach where vf_scale itself gains the ability to reference > a secondary input stream, using the "ref_*" series of variables. > > This makes the current [i][ri]scale2ref[o][ro] equivalent to the only > slightly more verbose [ri]split[t][ro]; [i][t]scale=rw:rh[o]. (And > conversely, it is no longer necessary to use nullsink to consume an > unused [ro]) In principle, a good idea, but how does this impact memory use and speed in the not-so-uncommon scenario where multiple overlay targets are scaled 2 ref and then overlaid e.g. in current flow: [a][base]scale2ref[a][ref]; [b][ref]scale2ref[b][ref[; [c][ref]scale2ref[c][ref[; [d][ref]scale2ref[d][ref[; [ref][a]overlay[ref]; [ref][b]overlay[ref]; [ref][c]overlay[ref]; [ref][d]overlay[ref]; in new flow: [base]split=5[base][refa][refb][refc][refd]; [a][refa]scale[a]; [b][refb]scale[b]; [c][refc]scale[c]; [d][refd]scale[d]; [base][a]overlay[outa]; [outa][b]overlay[outb]; [outb][c]overlay[outc]; [outc][d]overlay[out]; Regards, Gyan
On 24/04/2024 13:18, Gyan Doshi wrote: > > > On 2024-04-24 04:21 pm, Niklas Haas wrote: >> As discussed in my previous series for fixing scale2ref[1], this filter >> is fundamentally broken, and the only real fix would be to switch to >> activate(), or ideally FFFrameSync. >> >> [1] https://ffmpeg.org//pipermail/ffmpeg-devel/2024-March/323382.html >> >> The main thing making this difficult is the fact that scale2ref also >> wants to output ref frames to its secondary output, which FFFrameSync >> does not support, and which is ultimately at least part of the root >> cause of trac #10795. >> >> Since this is in principle completely unnecessary (users can just >> 'split' the ref input and have it be consumed by vf_scale), and to make >> the design of this filter a bit more robust and maintainable, switch to >> an approach where vf_scale itself gains the ability to reference >> a secondary input stream, using the "ref_*" series of variables. >> >> This makes the current [i][ri]scale2ref[o][ro] equivalent to the only >> slightly more verbose [ri]split[t][ro]; [i][t]scale=rw:rh[o]. (And >> conversely, it is no longer necessary to use nullsink to consume an >> unused [ro]) > > In principle, a good idea, but how does this impact memory use and speed > in the not-so-uncommon scenario where multiple overlay targets are > scaled 2 ref and then overlaid > e.g. > > in current flow: > > [a][base]scale2ref[a][ref]; > [b][ref]scale2ref[b][ref[; > [c][ref]scale2ref[c][ref[; > [d][ref]scale2ref[d][ref[; > [ref][a]overlay[ref]; > [ref][b]overlay[ref]; > [ref][c]overlay[ref]; > [ref][d]overlay[ref]; > > in new flow: > > [base]split=5[base][refa][refb][refc][refd]; > [a][refa]scale[a]; > [b][refb]scale[b]; > [c][refc]scale[c]; > [d][refd]scale[d]; > [base][a]overlay[outa]; > [outa][b]overlay[outb]; > [outb][c]overlay[outc]; > [outc][d]overlay[out]; Given that scale never modifies the reference, it just ups the refcount of those frames in the split filter, and will thus not change the memory-use whatsoever.
On Wed, 24 Apr 2024 16:48:54 +0530 Gyan Doshi <ffmpeg@gyani.pro> wrote: > > > On 2024-04-24 04:21 pm, Niklas Haas wrote: > > As discussed in my previous series for fixing scale2ref[1], this filter > > is fundamentally broken, and the only real fix would be to switch to > > activate(), or ideally FFFrameSync. > > > > [1] https://ffmpeg.org//pipermail/ffmpeg-devel/2024-March/323382.html > > > > The main thing making this difficult is the fact that scale2ref also > > wants to output ref frames to its secondary output, which FFFrameSync > > does not support, and which is ultimately at least part of the root > > cause of trac #10795. > > > > Since this is in principle completely unnecessary (users can just > > 'split' the ref input and have it be consumed by vf_scale), and to make > > the design of this filter a bit more robust and maintainable, switch to > > an approach where vf_scale itself gains the ability to reference > > a secondary input stream, using the "ref_*" series of variables. > > > > This makes the current [i][ri]scale2ref[o][ro] equivalent to the only > > slightly more verbose [ri]split[t][ro]; [i][t]scale=rw:rh[o]. (And > > conversely, it is no longer necessary to use nullsink to consume an > > unused [ro]) > > In principle, a good idea, but how does this impact memory use and speed > in the not-so-uncommon scenario where multiple overlay targets are > scaled 2 ref and then overlaid > e.g. > > in current flow: > > [a][base]scale2ref[a][ref]; > [b][ref]scale2ref[b][ref[; > [c][ref]scale2ref[c][ref[; > [d][ref]scale2ref[d][ref[; > [ref][a]overlay[ref]; > [ref][b]overlay[ref]; > [ref][c]overlay[ref]; > [ref][d]overlay[ref]; > > in new flow: > > [base]split=5[base][refa][refb][refc][refd]; > [a][refa]scale[a]; > [b][refb]scale[b]; > [c][refc]scale[c]; > [d][refd]scale[d]; > [base][a]overlay[outa]; > [outa][b]overlay[outb]; > [outb][c]overlay[outc]; > [outc][d]overlay[out]; > > > Regards, > Gyan I have not tested it exactly, but based on my understanding of libavfilter it shouldn't affect memory usage at all. `split` does not duplicate frame data, it merely creates more references. And since all of the `overlay` filters are upstream of [out], they all consume both of their inputs before any forward progress can be made. So there is no filter in this graph that can buffer more than 1 frame. Actually, I would suspect memory usage to be slightly *lower* on average, because ff_filter_activate_default() first consumes all possible frames from input 1, then all possible frames from input 2, etc.; whereas FFFrameSync consumes from both inputs simultaneously. > > _______________________________________________ > 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 Wed, 24 Apr 2024 12:51:55 +0200 Niklas Haas <ffmpeg@haasn.xyz> wrote: > As discussed in my previous series for fixing scale2ref[1], this filter > is fundamentally broken, and the only real fix would be to switch to > activate(), or ideally FFFrameSync. > > [1] https://ffmpeg.org//pipermail/ffmpeg-devel/2024-March/323382.html > > The main thing making this difficult is the fact that scale2ref also > wants to output ref frames to its secondary output, which FFFrameSync > does not support, and which is ultimately at least part of the root > cause of trac #10795. > > Since this is in principle completely unnecessary (users can just > 'split' the ref input and have it be consumed by vf_scale), and to make > the design of this filter a bit more robust and maintainable, switch to > an approach where vf_scale itself gains the ability to reference > a secondary input stream, using the "ref_*" series of variables. > > This makes the current [i][ri]scale2ref[o][ro] equivalent to the only > slightly more verbose [ri]split[t][ro]; [i][t]scale=rw:rh[o]. (And > conversely, it is no longer necessary to use nullsink to consume an > unused [ro]) > > Incidentally, I think it would be nice if lavfi could *automatically* > split filter links referenced multiple times, so we could just write > e.g. [i][ri]scale=rw:rh[o]; [ri][o]overlay[out] and have it work. Maybe > I'll try getting that to work, next. > > Either way, after the deprecation period has elapsed, we can delete > scale2ref from the codebase in order to make vf_scale.c IMHO > significantly simpler versus the status quo. > > _______________________________________________ > 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". Will merge in around 24h if there is no objection, as this is fixing a bug marked important.