[vlc-devel] [PATCH v1 01/33] filter: add a function to do the picture filtering

[Alexandre Janniaux]

Hi,

On Mon, Sep 28, 2020 at 10:21:12AM +0200, Steve Lhomme wrote:
> On 2020-09-28 10:12, Alexandre Janniaux wrote:
> > Hi,
> >
> > On Mon, Sep 28, 2020 at 10:05:58AM +0200, Steve Lhomme wrote:
> > > On 2020-09-28 9:26, Alexandre Janniaux wrote:
> > > > Hi,
> > > >
> > > > I don't really appreciate to be asked to write a full refactor
> > > > of the filter code (which is quite time consuming) only to have
> > > > someone rework the code differently before I submit my work. :(
> > > >
> > > > https://patches.videolan.org/patch/28297/
> > >
> > > I don't understand. Your patch is about adding close callback to the filter.
> > > This is unrelated to this patch or even the patchset which doesn't touch
> > > that part of the filters.
> >
> > I've shared the work with you multiple times, but as written in the
> > discussion above, it's about reworking the filter_t interface into a
> > vtable interface, just like you did for the vout_display (which is
> > exactly when we talked about this last time).
> >
> > Yes the patch I've sent is just about adding a close callback and it
> > would have been much easier done this way, since it would allow much
> > more incremental changes. Moving everything to a vtable is what is
> > asked in the discussion, and I've already mentionned what kind of work
> > it was in the patch above.
> >
> >
> >      > I first switched filter_t to an op structure, which is painful too
> >      > to bring at once (~80 filters), but I reached issues when it comes
> >      > with function having multiple different pf_filter callback (especially,
> >      > video converter) and didn't come up with a decision between not doing
> >      > this, doing memory allocation of the vtable or a generic pf_filter
> >      > switching the real filter functions.
> >
> >
> > > If you have other patches that impact what I changed then show them. You
> > > can't expect me to guess what you have done in private 2 months ago.
> >
> > This patch and the discussion around them were both public and internal
> > to Videolabs, so I'm not sure what you did to not know about them.
>
> I asked you privately on Friday where your branch is and you never gave it
> to me. And I honestly don't know where is that branch/work you're talking
> about.
>
> Now that's not how collaborative work should be done. We may coordinate if
> we work on the same thing. But otherwise you can't expect people to know
> about all your unmerged code from months ago. And if you raise the issue, as
> you do know, it's still possible to do it.
>
> I have rebased my work branches a huge amount of time because something
> changed in master. That happens especially when you work on a branch for a
> long time. Telling people I'm working on something won't make a difference.
> I'm not going to prevent people on something unless I did the exact same
> thing. This is just a drawback of working on the same codebase.
>
> So now we have 2 branches that may change pf_video_filter ? One of us will
> have to rebase and edit his patches. So what ?

Ok, maybe the situation was not clear, let's sort it out
incrementally! Thank you for your feedback.

Regards,
--
Alexandre Janniaux
Videolabs