Quoting Martin Storsjö (2016-06-25 12:41:06)
> On Sat, 25 Jun 2016, Anton Khirnov wrote:
>
> > Quoting Martin Storsjö (2016-06-24 23:08:13)
> >> On Fri, 24 Jun 2016, Diego Biurrun wrote:
> >>
> >> > On Fri, Jun 24, 2016 at 10:00:31PM +0300, Martin Storsjö wrote:
> >> >> On Fri, 24 Jun 2016, Diego Biurrun wrote:
> >> >> > On Fri, Jun 24, 2016 at 01:07:08AM +0300, Martin Storsjö wrote:
> >> >> >> --- /dev/null
> >> >> >> +++ b/libavcodec/libopenh264.h
> >> >> >> @@ -0,0 +1,39 @@
> >> >> >> +
> >> >> >> +#ifndef AVCODEC_LIBOPENH264_H
> >> >> >> +#define AVCODEC_LIBOPENH264_H
> >> >> >> +
> >> >> >> +#endif
> >> >> >
> >> >> > missing #endif comment
> >> >>
> >> >> ... and if I fix that?
> >> >>
> >> >> A "LGTM otherwise" or similar (or an explicit note saying the
> >> >> opposite),
> >> >> for trivial review comments like these, would be useful, to save one
> >> >> extra
> >> >> round of resending and review...
> >> >
> >> > That nit just caught my eye, so I mentioned it quickly.
> >> >
> >> > I have mixed feelings about this beast, so I'm abstaining from the
> >> > decision of accepting it or not. I also haven't looked at it in enough
> >> > depth to say whether it's OK or not. Hence my silence on the patch in
> >> > general.
> >>
> >> Ok, thanks for saying so explicitly.
> >
> > For myself, I see no problem with having this in, as long as there is a
> > use case for it.
> >
> > But there should probably be a note somewhere (External libraries
> > section in general.texi?) clarifying that it's not really suitable for
> > general use.
>
> Amended the patch locally with this:
>
> --- a/doc/general.texi
> +++ b/doc/general.texi
> @@ -97,12 +97,19 @@ enable it.
>
> @section OpenH264
>
> -Libav can make use of the OpenH264 library for H.264 encoding.
> +Libav can make use of the OpenH264 library for H.264 encoding and decoding.
>
> Go to @url{http://www.openh264.org/} and follow the instructions for
> installing the library. Then pass @code{--enable-libopenh264} to configure
> to
> enable it.
>
> +For decoding, this library is much more limited than the built-in decoder
> +in libavcodec; currently, this library lacks support for decoding B-frames
> +and some other main/high profile features. (It currently only supports
> +constrained baseline profile and cabac.) Using it is mostly useful for
CABAC
> +testing and for taking advantage of Cisco's patent portfolio license
> +(@url{http://www.openh264.org/BINARY_LICENSE.txt}).
> +
Otherwise looks fine.
--
Anton Khirnov
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
