On Mon, May 29, 2017 at 09:46:08PM +0300, Martin Storsjö wrote:
> On Mon, 29 May 2017, Diego Biurrun wrote:
> >On Mon, May 29, 2017 at 12:44:21PM +0200, wm4 wrote:
> >>On Mon, 29 May 2017 12:39:12 +0200
> >>Diego Biurrun <[email protected]> wrote:
> >>> On Mon, May 29, 2017 at 12:18:22PM +0200, wm4 wrote:
> >>> > On Mon, 29 May 2017 12:03:26 +0200
> >>> > Diego Biurrun <[email protected]> wrote: > > > On Mon, May 29, 2017
> >>at 11:32:49AM +0200, wm4 wrote: > > > > On Mon, 29 May 2017 10:56:36
> >>+0200
> >>> > > > Diego Biurrun <[email protected]> wrote: > > > > > ---
> >>> > > > > > > > > > Log message still not perfect.
> >>> > > > > > > > > > No longer tries to deduplicate parts of the
> >>implementation, just disentangles
> >>> > > > > the protocol declaration.
> >>> > > > > > > > > >  configure                 |  8 ++------
> >>> > > > >  libavformat/Makefile      |  3 +--
> >>> > > > >  libavformat/network.c     | 20 --------------------
> >>> > > > >  libavformat/protocols.c   |  3 +--
> >>> > > > >  libavformat/tls.c         | 39 
> >>> > > > > ++++++++++++++++++++++++++++++---------
> >>> > > > >  libavformat/tls.h         |  8 --------
> >>> > > > >  libavformat/tls_gnutls.c  | 31 ++++---------------------------
> >>> > > > >  libavformat/tls_openssl.c | 31 ++++---------------------------
> >>> > > > >  libavformat/utils.c       |  4 ++++
> >>> > > > >  9 files changed, 46 insertions(+), 101 deletions(-) > > > > >
> >>> > > We have a perfectly fine way to modularize protocols (or protocol
> >>> > > > "filters", like TLS, encryption, etc.) - and we're using it in a 
> >>> > > > good
> >>> > > > way. > > > > > > Example? > > > > Like I said, TLS protocols,
> >>encryption, any other form of nested
> >>> > protocols. Nested protocols are the libavformat abstraction to use for
> >>> > this, and TLS fits quite well into it.
> >>> > > > It works by giving each TLS implementation its own URLProtocol,
> >>which
> >>> > the current code does.
> >>> > > > I don't know what your patch is trying to achieve. > > Fix the
> >>bug(s) in your splitting of tls.c. tls_protocol no longer exists
> >>> but is referenced in configure. Nested protocols may be a nice abstraction
> >>> but it should not change the way users have to configure a build. There
> >>> should be one way to enable/disable the TLS protocol, not two now and
> >>> possibly four different ones in the future.
> >>
> >>Then why not fix configure instead?
> >
> >Because configure is not the place to fix this, cleanly or otherwise.
> >I've heard two arguments against my patch so far:
> >
> >1) The patch adds #ifdefs.
> >2) .c #includes are not pretty.
> >
> >I fixed 1) in v2 of the patch, 2) is a matter of taste that does not strike
> >me as a particularly convincing argument. I also suspect that the word
> >"disentangle" in the title of the first version of the patch rubbed a
> >few people the wrong way.
> >
> >To quote Martin from memory: "Maybe we do break configure command lines,
> >but we should not." We are discussing such a case here.
> 
> So, the actual end-user-visible things you're trying to fix is making
> --disable-protocol=tls behave as intended?

Yes. That's the important issue in need of repair. I also think that
my patch does a sensible bit of refactoring in the process.

Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to