[FFmpeg-devel] [PATCH 08/11] avutil/half2float: move non-inline init code out of header

[Andreas Rheinhardt]

Michael Niedermayer:
> On Thu, Aug 11, 2022 at 11:31:32PM +0200, Andreas Rheinhardt wrote:
>> Michael Niedermayer:
>>> On Thu, Aug 11, 2022 at 10:50:10PM +0200, Andreas Rheinhardt wrote:
>>>> Michael Niedermayer:
>>>>> On Wed, Aug 10, 2022 at 10:47:09PM +0200, Timo Rothenpieler wrote:
>>>>>> ---
>>>>>>  libavcodec/Makefile     |  8 +++---
>>>>>>  libavcodec/exr.c        |  2 +-
>>>>>>  libavcodec/exrenc.c     |  2 +-
>>>>>>  libavcodec/float2half.c | 19 +++++++++++++
>>>>>>  libavcodec/half2float.c | 19 +++++++++++++
>>>>>>  libavcodec/pnmdec.c     |  2 +-
>>>>>>  libavcodec/pnmenc.c     |  2 +-
>>>>>>  libavutil/float2half.c  | 53 ++++++++++++++++++++++++++++++++++
>>>>>>  libavutil/float2half.h  | 36 ++---------------------
>>>>>>  libavutil/half2float.c  | 63 +++++++++++++++++++++++++++++++++++++++++
>>>>>>  libavutil/half2float.h  | 46 ++----------------------------
>>>>>>  11 files changed, 166 insertions(+), 86 deletions(-)
>>>>>>  create mode 100644 libavcodec/float2half.c
>>>>>>  create mode 100644 libavcodec/half2float.c
>>>>>>  create mode 100644 libavutil/float2half.c
>>>>>>  create mode 100644 libavutil/half2float.c
>>>>>>
>>>>>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>>>>>> index 029f1bad3d..cb80f73d99 100644
>>>>>> --- a/libavcodec/Makefile
>>>>>> +++ b/libavcodec/Makefile
>>>>>> @@ -337,8 +337,8 @@ OBJS-$(CONFIG_EIGHTSVX_FIB_DECODER)    += 8svx.o
>>>>>>  OBJS-$(CONFIG_ESCAPE124_DECODER)       += escape124.o
>>>>>>  OBJS-$(CONFIG_ESCAPE130_DECODER)       += escape130.o
>>>>>>  OBJS-$(CONFIG_EVRC_DECODER)            += evrcdec.o acelp_vectors.o lsp.o
>>>>>> -OBJS-$(CONFIG_EXR_DECODER)             += exr.o exrdsp.o
>>>>>> -OBJS-$(CONFIG_EXR_ENCODER)             += exrenc.o
>>>>>> +OBJS-$(CONFIG_EXR_DECODER)             += exr.o exrdsp.o half2float.o
>>>>>> +OBJS-$(CONFIG_EXR_ENCODER)             += exrenc.o float2half.o
>>>>>>  OBJS-$(CONFIG_FASTAUDIO_DECODER)       += fastaudio.o
>>>>>>  OBJS-$(CONFIG_FFV1_DECODER)            += ffv1dec.o ffv1.o
>>>>>>  OBJS-$(CONFIG_FFV1_ENCODER)            += ffv1enc.o ffv1.o
>>>>>> @@ -570,8 +570,8 @@ OBJS-$(CONFIG_PGMYUV_DECODER)          += pnmdec.o pnm.o
>>>>>>  OBJS-$(CONFIG_PGMYUV_ENCODER)          += pnmenc.o
>>>>>>  OBJS-$(CONFIG_PGSSUB_DECODER)          += pgssubdec.o
>>>>>>  OBJS-$(CONFIG_PGX_DECODER)             += pgxdec.o
>>>>>> -OBJS-$(CONFIG_PHM_DECODER)             += pnmdec.o pnm.o
>>>>>> -OBJS-$(CONFIG_PHM_ENCODER)             += pnmenc.o
>>>>>> +OBJS-$(CONFIG_PHM_DECODER)             += pnmdec.o pnm.o half2float.o
>>>>>> +OBJS-$(CONFIG_PHM_ENCODER)             += pnmenc.o float2half.o
>>>>>>  OBJS-$(CONFIG_PHOTOCD_DECODER)         += photocd.o
>>>>>>  OBJS-$(CONFIG_PICTOR_DECODER)          += pictordec.o cga_data.o
>>>>>>  OBJS-$(CONFIG_PIXLET_DECODER)          += pixlet.o
>>>>>> diff --git a/libavcodec/exr.c b/libavcodec/exr.c
>>>>>> index 825354873d..a3582bfdd6 100644
>>>>>> --- a/libavcodec/exr.c
>>>>>> +++ b/libavcodec/exr.c
>>>>>> @@ -2208,7 +2208,7 @@ static av_cold int decode_init(AVCodecContext *avctx)
>>>>>>      float one_gamma = 1.0f / s->gamma;
>>>>>>      avpriv_trc_function trc_func = NULL;
>>>>>>  
>>>>>> -    init_half2float_tables(&s->h2f_tables);
>>>>>> +    ff_init_half2float_tables(&s->h2f_tables);
>>>>> [...]
>>>>>> diff --git a/libavutil/float2half.c b/libavutil/float2half.c
>>>>>> new file mode 100644
>>>>>> index 0000000000..dba14cef5d
>>>>>> --- /dev/null
>>>>>> +++ b/libavutil/float2half.c
>>>>> [...]
>>>>>> +void ff_init_float2half_tables(float2half_tables *t)
>>>>>
>>>>> this will need avpriv or break linking with shared libs
>>>>>
>>>>
>>>> No, because this code is duplicated into all libraries that need it.
>>>> (In case of static linking, only one of the variants will be used
>>>> (namely the first one encountered in the link.)
>>>
>>> libswscale/libswscale.so: undefined reference to `ff_init_half2float_tables'
>>> libswscale/libswscale.so: undefined reference to `ff_init_half2float_tables'
>>> clang: error: linker command failed with exit code 1 (use -v to see invocation)
>>> Makefile:131: recipe for target 'ffplay_g' failed
>>> make: *** [ffplay_g] Error 1
>>> make: *** Waiting for unfinished jobs....
>>> clang: error: linker command failed with exit code 1 (use -v to see invocation)
>>> libswscale/libswscale.so: undefined reference to `ff_init_half2float_tables'
>>> Makefile:131: recipe for target 'ffprobe_g' failed
>>> make: *** [ffprobe_g] Error 1
>>> clang: error: linker command failed with exit code 1 (use -v to see invocation)
>>> Makefile:131: recipe for target 'ffmpeg_g' failed
>>> make: *** [ffmpeg_g] Error 1
>>>
>>
>> That is with the whole patchset applied, isn't it!? Duplicating the init
>> stuff into swscale has been forgotten.
> 
> Ive tried the new latest patchset and this still happens
> 

As explained here:
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-August/300045.html, the
reason is that the patch that makes use of this in libswscale adds the
half2float.o dependency to libavutil.

> make distclean ; ../configure --enable-shared --cc='ccache clang-6.0'  --enable-pthreads --samples=fate/fate-suite/ --enable-version3  --extra-cflags='-O1 -fno-omit-frame-pointer'  && make -j32
> 
> LD	ffmpeg_g
> LD	ffplay_g
> LD	ffprobe_g
> libswscale/libswscale.so: undefined reference to `ff_init_half2float_tables'
> clang: error: linker command failed with exit code 1 (use -v to see invocation)
> Makefile:131: recipe for target 'ffprobe_g' failed
> make: *** [ffprobe_g] Error 1
> make: *** Waiting for unfinished jobs....
> libswscale/libswscale.so: undefined reference to `ff_init_half2float_tables'
> clang: error: linker command failed with exit code 1 (use -v to see invocation)
> Makefile:131: recipe for target 'ffplay_g' failed
> make: *** [ffplay_g] Error 1
> libswscale/libswscale.so: undefined reference to `ff_init_half2float_tables'
> clang: error: linker command failed with exit code 1 (use -v to see invocation)
> Makefile:131: recipe for target 'ffmpeg_g' failed
> make: *** [ffmpeg_g] Error 1
> 
> git grep ff_init_half2float_tables
> libavcodec/exr.c:    ff_init_half2float_tables(&s->h2f_tables);
> libavcodec/pnmdec.c:    ff_init_half2float_tables(&s->h2f_tables);
> libavutil/half2float.c:void ff_init_half2float_tables(Half2FloatTables *t)
> libavutil/half2float.h:void ff_init_half2float_tables(Half2FloatTables *t);
> libswscale/slice.c:        ff_init_half2float_tables(c->h2f_tables);
> 
> libavcodec/half2float.c:#include "libavutil/half2float.c"
> libswscale/half2float.c:#include "libavutil/half2float.c"
> 
> git grep half2float.o
> libavcodec/Makefile:OBJS-$(CONFIG_EXR_DECODER)             += exr.o exrdsp.o half2float.o
> libavcodec/Makefile:OBJS-$(CONFIG_PHM_DECODER)             += pnmdec.o pnm.o half2float.o
> libavutil/Makefile:       half2float.o                                                     \
> 
> also i tried this:
> -#include "libavutil/half2float.c"
> +#inklude "libavutil/half2float.c"
> 
> and still it fails at linking stage not compile, really impressive the compiler
> speaks german C too
> 

No, the libswscale/half2float.c file is just not compiled at all, see above.

> 
> Heres the patches applied:
> 
> 46e36b5371 swscale/input: add rgbaf16 input support
> 34446f3971 swscale: add opaque parameter to input functions
> 88cf148514 avutil/half2float: use native _Float16 if available
> af6c41450c avutil/half2float: move non-inline init code out of header
> 2f2ac4c8c9 avutil/half2float: move tables to header-internal structs
> 19ac9a570e avutil/half2float: adjust conversion of NaN
> 45180741cd avutil: move half-precision float helper to avutil
> 
> (some more unrelated patches)
> 
> thx