Message ID | 20171005134535.13208-2-mjbshaw@gmail.com |
---|---|
State | Accepted |
Commit | 41d6d627024393c142cf7cd93eff1d5a049105f5 |
Headers | show |
On 10/5/2017 10:45 AM, Michael Bradshaw wrote: > From: Michael Bradshaw <mjbshaw@google.com> > > Signed-off-by: Michael Bradshaw <mjbshaw@google.com> > --- > configure | 5 ++++- > libavcodec/libopenjpegdec.c | 8 +++++--- > libavcodec/libopenjpegenc.c | 10 ++++++---- > 3 files changed, 15 insertions(+), 8 deletions(-) > > diff --git a/configure b/configure > index 391c141e7a..77c9a18c3c 100755 > --- a/configure > +++ b/configure > @@ -1930,6 +1930,7 @@ HEADERS_LIST=" > machine_ioctl_meteor_h > malloc_h > opencv2_core_core_c_h > + openjpeg_2_3_openjpeg_h Is there a reason OpenJPEG uses a different folder to store the header with each release from the 2.x family? It's really bloating both configure and the wrappers. > openjpeg_2_2_openjpeg_h > openjpeg_2_1_openjpeg_h > openjpeg_2_0_openjpeg_h > @@ -5950,7 +5951,9 @@ enabled libopencv && { check_header opencv2/core/core_c.h && > require opencv opencv2/core/core_c.h cvCreateImageHeader -lopencv_core -lopencv_imgproc; } || > require_pkg_config libopencv opencv opencv/cxcore.h cvCreateImageHeader; } > enabled libopenh264 && require_pkg_config libopenh264 openh264 wels/codec_api.h WelsGetCodecVersion > -enabled libopenjpeg && { { check_lib libopenjpeg openjpeg-2.2/openjpeg.h opj_version -lopenjp2 -DOPJ_STATIC && add_cppflags -DOPJ_STATIC; } || > +enabled libopenjpeg && { { check_lib libopenjpeg openjpeg-2.3/openjpeg.h opj_version -lopenjp2 -DOPJ_STATIC && add_cppflags -DOPJ_STATIC; } || > + check_lib libopenjpeg openjpeg-2.3/openjpeg.h opj_version -lopenjp2 || > + { check_lib libopenjpeg openjpeg-2.2/openjpeg.h opj_version -lopenjp2 -DOPJ_STATIC && add_cppflags -DOPJ_STATIC; } || > check_lib libopenjpeg openjpeg-2.2/openjpeg.h opj_version -lopenjp2 || > { check_lib libopenjpeg openjpeg-2.1/openjpeg.h opj_version -lopenjp2 -DOPJ_STATIC && add_cppflags -DOPJ_STATIC; } || > check_lib libopenjpeg openjpeg-2.1/openjpeg.h opj_version -lopenjp2 || > diff --git a/libavcodec/libopenjpegdec.c b/libavcodec/libopenjpegdec.c > index 1210123265..67d47bd6a0 100644 > --- a/libavcodec/libopenjpegdec.c > +++ b/libavcodec/libopenjpegdec.c > @@ -34,7 +34,9 @@ > #include "internal.h" > #include "thread.h" > > -#if HAVE_OPENJPEG_2_2_OPENJPEG_H > +#if HAVE_OPENJPEG_2_3_OPENJPEG_H > +# include <openjpeg-2.3/openjpeg.h> > +#elif HAVE_OPENJPEG_2_2_OPENJPEG_H > # include <openjpeg-2.2/openjpeg.h> > #elif HAVE_OPENJPEG_2_1_OPENJPEG_H > # include <openjpeg-2.1/openjpeg.h> > @@ -46,7 +48,7 @@ > # include <openjpeg.h> > #endif > > -#if HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H || HAVE_OPENJPEG_2_0_OPENJPEG_H > +#if HAVE_OPENJPEG_2_3_OPENJPEG_H || HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H || HAVE_OPENJPEG_2_0_OPENJPEG_H > # define OPENJPEG_MAJOR_VERSION 2 > # define OPJ(x) OPJ_##x > #else > @@ -431,7 +433,7 @@ static int libopenjpeg_decode_frame(AVCodecContext *avctx, > opj_stream_set_read_function(stream, stream_read); > opj_stream_set_skip_function(stream, stream_skip); > opj_stream_set_seek_function(stream, stream_seek); > -#if HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H > +#if HAVE_OPENJPEG_2_3_OPENJPEG_H || HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H > opj_stream_set_user_data(stream, &reader, NULL); > #elif HAVE_OPENJPEG_2_0_OPENJPEG_H > opj_stream_set_user_data(stream, &reader); Oh, i see. They don't care about breaking API... > diff --git a/libavcodec/libopenjpegenc.c b/libavcodec/libopenjpegenc.c > index b67e533d1d..92b4433b04 100644 > --- a/libavcodec/libopenjpegenc.c > +++ b/libavcodec/libopenjpegenc.c > @@ -32,7 +32,9 @@ > #include "avcodec.h" > #include "internal.h" > > -#if HAVE_OPENJPEG_2_2_OPENJPEG_H > +#if HAVE_OPENJPEG_2_3_OPENJPEG_H > +# include <openjpeg-2.3/openjpeg.h> > +#elif HAVE_OPENJPEG_2_2_OPENJPEG_H > # include <openjpeg-2.2/openjpeg.h> > #elif HAVE_OPENJPEG_2_1_OPENJPEG_H > # include <openjpeg-2.1/openjpeg.h> > @@ -44,7 +46,7 @@ > # include <openjpeg.h> > #endif > > -#if HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H || HAVE_OPENJPEG_2_0_OPENJPEG_H > +#if HAVE_OPENJPEG_2_3_OPENJPEG_H || HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H || HAVE_OPENJPEG_2_0_OPENJPEG_H > # define OPENJPEG_MAJOR_VERSION 2 > # define OPJ(x) OPJ_##x > #else > @@ -307,7 +309,7 @@ static av_cold int libopenjpeg_encode_init(AVCodecContext *avctx) > > opj_set_default_encoder_parameters(&ctx->enc_params); > > -#if HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H > +#if HAVE_OPENJPEG_2_3_OPENJPEG_H || HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H > switch (ctx->cinema_mode) { > case OPJ_CINEMA2K_24: > ctx->enc_params.rsiz = OPJ_PROFILE_CINEMA_2K; > @@ -771,7 +773,7 @@ static int libopenjpeg_encode_frame(AVCodecContext *avctx, AVPacket *pkt, > opj_stream_set_write_function(stream, stream_write); > opj_stream_set_skip_function(stream, stream_skip); > opj_stream_set_seek_function(stream, stream_seek); > -#if HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H > +#if HAVE_OPENJPEG_2_3_OPENJPEG_H || HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H > opj_stream_set_user_data(stream, &writer, NULL); > #elif HAVE_OPENJPEG_2_0_OPENJPEG_H > opj_stream_set_user_data(stream, &writer); >
On Thu, Oct 5, 2017 at 9:55 AM, James Almer <jamrial@gmail.com> wrote: > On 10/5/2017 10:45 AM, Michael Bradshaw wrote: > > From: Michael Bradshaw <mjbshaw@google.com> > > > > Signed-off-by: Michael Bradshaw <mjbshaw@google.com> > > --- > > configure | 5 ++++- > > libavcodec/libopenjpegdec.c | 8 +++++--- > > libavcodec/libopenjpegenc.c | 10 ++++++---- > > 3 files changed, 15 insertions(+), 8 deletions(-) > > > > diff --git a/configure b/configure > > index 391c141e7a..77c9a18c3c 100755 > > --- a/configure > > +++ b/configure > > @@ -1930,6 +1930,7 @@ HEADERS_LIST=" > > machine_ioctl_meteor_h > > malloc_h > > opencv2_core_core_c_h > > + openjpeg_2_3_openjpeg_h > > Is there a reason OpenJPEG uses a different folder to store the header > with each release from the 2.x family? It's really bloating both > configure and the wrappers. Yeah, it's really annoying. Once we drop support for 1.x versions we'll be able to clean up the majority of this garbage (though not all of it, unfortunately). I'd personally like to drop support for OpenJPEG 1.x immediately; the only place where it's still used is in package managers for LTS Linux distros, and I have no qualms about telling users to manually build/install a more recent version of OpenJPEG (especially since there are so many bug fixes in recent OpenJPEG versions, many of which are security related).
2017-10-06 18:26 GMT+02:00 Michael Bradshaw <mjbshaw@gmail.com>: > Once we drop support for 1.x versions we'll be able to clean up > the majority of this garbage (though not all of it, unfortunately). > I'd personally like to drop support for OpenJPEG 1.x immediately I suggest you drop it after the release. Thank you, Carl Eugen
On 10/6/2017 1:26 PM, Michael Bradshaw wrote: > On Thu, Oct 5, 2017 at 9:55 AM, James Almer <jamrial@gmail.com> wrote: > >> On 10/5/2017 10:45 AM, Michael Bradshaw wrote: >>> From: Michael Bradshaw <mjbshaw@google.com> >>> >>> Signed-off-by: Michael Bradshaw <mjbshaw@google.com> >>> --- >>> configure | 5 ++++- >>> libavcodec/libopenjpegdec.c | 8 +++++--- >>> libavcodec/libopenjpegenc.c | 10 ++++++---- >>> 3 files changed, 15 insertions(+), 8 deletions(-) >>> >>> diff --git a/configure b/configure >>> index 391c141e7a..77c9a18c3c 100755 >>> --- a/configure >>> +++ b/configure >>> @@ -1930,6 +1930,7 @@ HEADERS_LIST=" >>> machine_ioctl_meteor_h >>> malloc_h >>> opencv2_core_core_c_h >>> + openjpeg_2_3_openjpeg_h >> >> Is there a reason OpenJPEG uses a different folder to store the header >> with each release from the 2.x family? It's really bloating both >> configure and the wrappers. > > > Yeah, it's really annoying. Once we drop support for 1.x versions we'll be > able to clean up the majority of this garbage (though not all of it, > unfortunately). I'd personally like to drop support for OpenJPEG 1.x > immediately; the only place where it's still used is in package managers > for LTS Linux distros, and I have no qualms about telling users to manually > build/install a more recent version of OpenJPEG (especially since there are > so many bug fixes in recent OpenJPEG versions, many of which are security > related). As Carl said, since we're a few days away from a new release it'll be best to do it after it's branched off master. But for now, apply this patch so the new release can use OpenJPEG 2.3 I see OpenJPEG2 ships a pkg-config file, which would let us drop all the extra configure checks for each different include folder. That will clear all the current configure bloat and prevent new checks to be added in the future.
On 10/6/2017 10:26 PM, James Almer wrote: > On 10/6/2017 1:26 PM, Michael Bradshaw wrote: >> On Thu, Oct 5, 2017 at 9:55 AM, James Almer <jamrial@gmail.com> wrote: >> >>> On 10/5/2017 10:45 AM, Michael Bradshaw wrote: >>>> From: Michael Bradshaw <mjbshaw@google.com> >>>> >>>> Signed-off-by: Michael Bradshaw <mjbshaw@google.com> >>>> --- >>>> configure | 5 ++++- >>>> libavcodec/libopenjpegdec.c | 8 +++++--- >>>> libavcodec/libopenjpegenc.c | 10 ++++++---- >>>> 3 files changed, 15 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/configure b/configure >>>> index 391c141e7a..77c9a18c3c 100755 >>>> --- a/configure >>>> +++ b/configure >>>> @@ -1930,6 +1930,7 @@ HEADERS_LIST=" >>>> machine_ioctl_meteor_h >>>> malloc_h >>>> opencv2_core_core_c_h >>>> + openjpeg_2_3_openjpeg_h >>> >>> Is there a reason OpenJPEG uses a different folder to store the header >>> with each release from the 2.x family? It's really bloating both >>> configure and the wrappers. >> >> >> Yeah, it's really annoying. Once we drop support for 1.x versions we'll be >> able to clean up the majority of this garbage (though not all of it, >> unfortunately). I'd personally like to drop support for OpenJPEG 1.x >> immediately; the only place where it's still used is in package managers >> for LTS Linux distros, and I have no qualms about telling users to manually >> build/install a more recent version of OpenJPEG (especially since there are >> so many bug fixes in recent OpenJPEG versions, many of which are security >> related). > > As Carl said, since we're a few days away from a new release it'll be > best to do it after it's branched off master. But for now, apply this > patch so the new release can use OpenJPEG 2.3 > > I see OpenJPEG2 ships a pkg-config file, which would let us drop all the > extra configure checks for each different include folder. That will > clear all the current configure bloat and prevent new checks to be added > in the future. The 3.4 release has been tagged. Can you look into removing the OpenJPEG 1.x support and switching the checks to pkg-config to finally clean up the configure and wrappers' bloat?
diff --git a/configure b/configure index 391c141e7a..77c9a18c3c 100755 --- a/configure +++ b/configure @@ -1930,6 +1930,7 @@ HEADERS_LIST=" machine_ioctl_meteor_h malloc_h opencv2_core_core_c_h + openjpeg_2_3_openjpeg_h openjpeg_2_2_openjpeg_h openjpeg_2_1_openjpeg_h openjpeg_2_0_openjpeg_h @@ -5950,7 +5951,9 @@ enabled libopencv && { check_header opencv2/core/core_c.h && require opencv opencv2/core/core_c.h cvCreateImageHeader -lopencv_core -lopencv_imgproc; } || require_pkg_config libopencv opencv opencv/cxcore.h cvCreateImageHeader; } enabled libopenh264 && require_pkg_config libopenh264 openh264 wels/codec_api.h WelsGetCodecVersion -enabled libopenjpeg && { { check_lib libopenjpeg openjpeg-2.2/openjpeg.h opj_version -lopenjp2 -DOPJ_STATIC && add_cppflags -DOPJ_STATIC; } || +enabled libopenjpeg && { { check_lib libopenjpeg openjpeg-2.3/openjpeg.h opj_version -lopenjp2 -DOPJ_STATIC && add_cppflags -DOPJ_STATIC; } || + check_lib libopenjpeg openjpeg-2.3/openjpeg.h opj_version -lopenjp2 || + { check_lib libopenjpeg openjpeg-2.2/openjpeg.h opj_version -lopenjp2 -DOPJ_STATIC && add_cppflags -DOPJ_STATIC; } || check_lib libopenjpeg openjpeg-2.2/openjpeg.h opj_version -lopenjp2 || { check_lib libopenjpeg openjpeg-2.1/openjpeg.h opj_version -lopenjp2 -DOPJ_STATIC && add_cppflags -DOPJ_STATIC; } || check_lib libopenjpeg openjpeg-2.1/openjpeg.h opj_version -lopenjp2 || diff --git a/libavcodec/libopenjpegdec.c b/libavcodec/libopenjpegdec.c index 1210123265..67d47bd6a0 100644 --- a/libavcodec/libopenjpegdec.c +++ b/libavcodec/libopenjpegdec.c @@ -34,7 +34,9 @@ #include "internal.h" #include "thread.h" -#if HAVE_OPENJPEG_2_2_OPENJPEG_H +#if HAVE_OPENJPEG_2_3_OPENJPEG_H +# include <openjpeg-2.3/openjpeg.h> +#elif HAVE_OPENJPEG_2_2_OPENJPEG_H # include <openjpeg-2.2/openjpeg.h> #elif HAVE_OPENJPEG_2_1_OPENJPEG_H # include <openjpeg-2.1/openjpeg.h> @@ -46,7 +48,7 @@ # include <openjpeg.h> #endif -#if HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H || HAVE_OPENJPEG_2_0_OPENJPEG_H +#if HAVE_OPENJPEG_2_3_OPENJPEG_H || HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H || HAVE_OPENJPEG_2_0_OPENJPEG_H # define OPENJPEG_MAJOR_VERSION 2 # define OPJ(x) OPJ_##x #else @@ -431,7 +433,7 @@ static int libopenjpeg_decode_frame(AVCodecContext *avctx, opj_stream_set_read_function(stream, stream_read); opj_stream_set_skip_function(stream, stream_skip); opj_stream_set_seek_function(stream, stream_seek); -#if HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H +#if HAVE_OPENJPEG_2_3_OPENJPEG_H || HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H opj_stream_set_user_data(stream, &reader, NULL); #elif HAVE_OPENJPEG_2_0_OPENJPEG_H opj_stream_set_user_data(stream, &reader); diff --git a/libavcodec/libopenjpegenc.c b/libavcodec/libopenjpegenc.c index b67e533d1d..92b4433b04 100644 --- a/libavcodec/libopenjpegenc.c +++ b/libavcodec/libopenjpegenc.c @@ -32,7 +32,9 @@ #include "avcodec.h" #include "internal.h" -#if HAVE_OPENJPEG_2_2_OPENJPEG_H +#if HAVE_OPENJPEG_2_3_OPENJPEG_H +# include <openjpeg-2.3/openjpeg.h> +#elif HAVE_OPENJPEG_2_2_OPENJPEG_H # include <openjpeg-2.2/openjpeg.h> #elif HAVE_OPENJPEG_2_1_OPENJPEG_H # include <openjpeg-2.1/openjpeg.h> @@ -44,7 +46,7 @@ # include <openjpeg.h> #endif -#if HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H || HAVE_OPENJPEG_2_0_OPENJPEG_H +#if HAVE_OPENJPEG_2_3_OPENJPEG_H || HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H || HAVE_OPENJPEG_2_0_OPENJPEG_H # define OPENJPEG_MAJOR_VERSION 2 # define OPJ(x) OPJ_##x #else @@ -307,7 +309,7 @@ static av_cold int libopenjpeg_encode_init(AVCodecContext *avctx) opj_set_default_encoder_parameters(&ctx->enc_params); -#if HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H +#if HAVE_OPENJPEG_2_3_OPENJPEG_H || HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H switch (ctx->cinema_mode) { case OPJ_CINEMA2K_24: ctx->enc_params.rsiz = OPJ_PROFILE_CINEMA_2K; @@ -771,7 +773,7 @@ static int libopenjpeg_encode_frame(AVCodecContext *avctx, AVPacket *pkt, opj_stream_set_write_function(stream, stream_write); opj_stream_set_skip_function(stream, stream_skip); opj_stream_set_seek_function(stream, stream_seek); -#if HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H +#if HAVE_OPENJPEG_2_3_OPENJPEG_H || HAVE_OPENJPEG_2_2_OPENJPEG_H || HAVE_OPENJPEG_2_1_OPENJPEG_H opj_stream_set_user_data(stream, &writer, NULL); #elif HAVE_OPENJPEG_2_0_OPENJPEG_H opj_stream_set_user_data(stream, &writer);