[FFmpeg-devel,1/1] lavc: add support for OpenJPEG 2.3.0

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(-)

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?