[FFmpeg-soc] [soc]: r5821 - libavfilter/vf_overlay.c

[Stefano Sabatini]

On date Friday 2010-06-11 03:06:30 -0700, Baptiste Coudurier encoded:
> On 6/9/10 4:49 PM, Baptiste Coudurier wrote:
> >On 06/09/2010 03:22 PM, Stefano Sabatini wrote:
> >>On date Friday 2010-06-04 22:15:35 +0200, bcoudurier encoded:
> >>>Author: bcoudurier
> >>>Date: Fri Jun 4 22:15:35 2010
> >>>New Revision: 5821
> >>>
> >>>Log:
> >>>Direct rendering in overlay filter.
> >>>RGB24 support.
> >>>Doesn't work with movie in movie yet, needs loop input feature for logos
> >>>either in movie filter or here.
> >>
> >>This broke movie in movie.
> >
> >Yes, I mentioned this in the commit log :)
> >
> >>Again, please *don't* make massive changes and introduce regressions
> >>without not even posting and discussing the changes before, testing
> >>with ffplay is a bonus.
> >>
> >>Consider to fix the regression or revert the patch if you can't do it
> >>in a reasonable amount of time, or at least try to justify because
> >>this change should be an improvement over the previous behaviour.
> 
> Besides, I believe you are overreacting here. If you can't realize why
> direct rendering is way better that what was in before, I can't help you.

I have this rewrite of ffmpeg which makes it much faster:
int main(void) { return 0; }

increasing speed should not be done at expenses of functionality, I
prefer a slow filter which works in every scenarios rather than a fast
filter which only works in particular cases.

> And if you had reviewed the filter you would have seen that there is a
> commented test about pts which would fetch the next frame from the
> overlayed source.
> My comment about loop input is pretty clear, if you actually read
> the code...
> If you overlay a logo, you want the input to loop, if you are
> overlayed a video, you don't want the last frame to loop.
>
> Besides the previous code with pts was weird, the output pts should
> always be the main video pts AFAIU.
> 
> I'd appreciate if people could actually review the changes and make
> constructive comments.

My point is that we should try to not break working stuff, people is
already using this repo, breaking previously working stuff should be
avoided, that's true especially for the filter providing the most
required functionality, saying on the user ML "sorry but my last
commit broke overlay, I hope someone will fix it soon or later" is not
going to cast good karma on the project.

If your changes required the implementation of a loop mechanism you're
recommended to implement that *before* to apply those changes, I'm for
sure not going to fix this as I have already too many things queued on
my todo.

I'm all for improving the current code and I appreciate your work, but
I'd like to review changes *before* to see them committed, especially
if they break previously working features.

Regards.