diff mbox

[FFmpeg-devel,v7] - Added Turing codec interface for ffmpeg

Message ID 59139428E3A17E4B867549526134D166122D9770@bgb01xud1009
State Superseded
Headers show

Commit Message

Saverio Blasi Feb. 21, 2017, 5:15 p.m. UTC
Hi all,

We have recently circulated this new iteration (see below) of our work towards integrating our HEVC Turing codec within FFMpeg. Assuming that there are no more requests for changes, we would like to understand what is the timeline for integration within the project.

Thanks a lot,

All the best,
----------------------------------------------------
Saverio Blasi, PhD
Senior Research Engineer 
BBC Research & Development
Centre House|56 Wood Lane|London|W12 7SB




-----Original Message-----
From: Saverio Blasi [mailto:saverio.blasi@bbc.co.uk] 
Sent: 13 February 2017 13:37
To: ffmpeg-devel@ffmpeg.org
Cc: Saverio Blasi <Saverio.Blasi@bbc.co.uk>
Subject: [PATCH v7] - Added Turing codec interface for ffmpeg

- This patch contains the changes to interface the Turing codec (http://turingcodec.org/) with ffmpeg. The patch was modified to address the comments in the review as follows:
  - Added a pkg-config file to list all dependencies required by libturing. This should address the issue pointed out by Hendrik Leppkes on Fri 18/11/2016
  - As per suggestions of wm4, two functions (add_option and finalise_options) have been created. The former appends new options while the latter sets up the argv array of pointers to char* accordingly. add_option re-allocates the buffer for options using av_realloc
  - Additionally, both these functions handle the errors in case the memory wasn't allocated correctly
  - malloc|free|realloc have been substituted with their corresponding av_{malloc|free|realloc} version
  - Check on bit-depth has been removed since the ffmpeg already casts the right pix_fmt and bit depth
  - pix_fmts is now set in ff_libturing_encoder as in h264dec.c.
  - Changed usage of av_free with av_freep and fixed calls to free arrays
  - Added brackets to all if and for statements
  - Avoid repetition of code to free arrays in case of failure to initialise the libturing encoder
  - Some fixes to address the review from wm4 and Mark Thompson received on Wed 08/02/2017
  - Fixed indentation
---
 LICENSE.md             |   1 +
 configure              |   5 +
 libavcodec/Makefile    |   1 +
 libavcodec/allcodecs.c |   1 +
 libavcodec/libturing.c | 320 +++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 328 insertions(+)
 create mode 100644 libavcodec/libturing.c

Comments

Mark Thompson Feb. 23, 2017, 8:47 p.m. UTC | #1
On 21/02/17 17:15, Saverio Blasi wrote:
> Hi all,
> 
> We have recently circulated this new iteration (see below) of our work towards integrating our HEVC Turing codec within FFMpeg. Assuming that there are no more requests for changes, we would like to understand what is the timeline for integration within the project.
> 
> Thanks a lot,
> 
> All the best,
> ----------------------------------------------------
> Saverio Blasi, PhD
> Senior Research Engineer 
> BBC Research & Development
> Centre House|56 Wood Lane|London|W12 7SB
> 
> 
> 
> 
> -----Original Message-----
> From: Saverio Blasi [mailto:saverio.blasi@bbc.co.uk] 
> Sent: 13 February 2017 13:37
> To: ffmpeg-devel@ffmpeg.org
> Cc: Saverio Blasi <Saverio.Blasi@bbc.co.uk>
> Subject: [PATCH v7] - Added Turing codec interface for ffmpeg
> 
> - This patch contains the changes to interface the Turing codec (http://turingcodec.org/) with ffmpeg. The patch was modified to address the comments in the review as follows:
>   - Added a pkg-config file to list all dependencies required by libturing. This should address the issue pointed out by Hendrik Leppkes on Fri 18/11/2016
>   - As per suggestions of wm4, two functions (add_option and finalise_options) have been created. The former appends new options while the latter sets up the argv array of pointers to char* accordingly. add_option re-allocates the buffer for options using av_realloc
>   - Additionally, both these functions handle the errors in case the memory wasn't allocated correctly
>   - malloc|free|realloc have been substituted with their corresponding av_{malloc|free|realloc} version
>   - Check on bit-depth has been removed since the ffmpeg already casts the right pix_fmt and bit depth
>   - pix_fmts is now set in ff_libturing_encoder as in h264dec.c.
>   - Changed usage of av_free with av_freep and fixed calls to free arrays
>   - Added brackets to all if and for statements
>   - Avoid repetition of code to free arrays in case of failure to initialise the libturing encoder
>   - Some fixes to address the review from wm4 and Mark Thompson received on Wed 08/02/2017
>   - Fixed indentation
> ---
>  LICENSE.md             |   1 +
>  configure              |   5 +
>  libavcodec/Makefile    |   1 +
>  libavcodec/allcodecs.c |   1 +
>  libavcodec/libturing.c | 320 +++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 328 insertions(+)
>  create mode 100644 libavcodec/libturing.c
> 
> diff --git a/LICENSE.md b/LICENSE.md
> index 640633c..86e5371 100644
> --- a/LICENSE.md
> +++ b/LICENSE.md
> @@ -85,6 +85,7 @@ The following libraries are under GPL:
>  - frei0r
>  - libcdio
>  - librubberband
> +- libturing
>  - libvidstab
>  - libx264
>  - libx265
> diff --git a/configure b/configure
> index 7154142..a27cb8b 100755
> --- a/configure
> +++ b/configure
> @@ -255,6 +255,7 @@ External library support:
>    --enable-libssh          enable SFTP protocol via libssh [no]
>    --enable-libtesseract    enable Tesseract, needed for ocr filter [no]
>    --enable-libtheora       enable Theora encoding via libtheora [no]
> +  --enable-libturing       enable H.265/HEVC encoding via libturing [no]
>    --enable-libtwolame      enable MP2 encoding via libtwolame [no]
>    --enable-libv4l2         enable libv4l2/v4l-utils [no]
>    --enable-libvidstab      enable video stabilization using vid.stab [no]
> @@ -1562,6 +1563,7 @@ EXTERNAL_LIBRARY_LIST="
>      libssh
>      libtesseract
>      libtheora
> +    libturing
>      libtwolame
>      libv4l2
>      libvidstab
> @@ -2858,6 +2860,7 @@ libspeex_decoder_deps="libspeex"
>  libspeex_encoder_deps="libspeex"
>  libspeex_encoder_select="audio_frame_queue"
>  libtheora_encoder_deps="libtheora"
> +libturing_encoder_deps="libturing"
>  libtwolame_encoder_deps="libtwolame"
>  libvo_amrwbenc_encoder_deps="libvo_amrwbenc"
>  libvorbis_decoder_deps="libvorbis"
> @@ -5131,6 +5134,7 @@ die_license_disabled gpl frei0r  die_license_disabled gpl libcdio  die_license_disabled gpl librubberband  die_license_disabled gpl libsmbclient
> +die_license_disabled gpl libturing
>  die_license_disabled gpl libvidstab
>  die_license_disabled gpl libx264
>  die_license_disabled gpl libx265
> @@ -5789,6 +5793,7 @@ enabled libssh            && require_pkg_config libssh libssh/sftp.h sftp_init
>  enabled libspeex          && require_pkg_config speex speex/speex.h speex_decoder_init -lspeex
>  enabled libtesseract      && require_pkg_config tesseract tesseract/capi.h TessBaseAPICreate
>  enabled libtheora         && require libtheora theora/theoraenc.h th_info_init -ltheoraenc -ltheoradec -logg
> +enabled libturing         && require_pkg_config libturing turing.h turing_version
>  enabled libtwolame        && require libtwolame twolame.h twolame_init -ltwolame &&
>                               { check_lib twolame.h twolame_encode_buffer_float32_interleaved -ltwolame ||
>                                 die "ERROR: libtwolame must be installed and version must be >= 0.3.10"; } diff --git a/libavcodec/Makefile b/libavcodec/Makefile index 43a6add..de5af1d 100644

Are you missing some link options?

I was wanting to test this, so I installed from your git (stable branch).  Trying to configure gives me:

 11608  check_pkg_config libturing turing.h turing_version
 11609  pkg-config --exists --print-errors libturing
 11610  check_func_headers turing.h turing_version -I/usr/local/include -L/usr/local/lib -lturing -lhavoc
 11611  check_ld cc -I/usr/local/include -L/usr/local/lib -lturing -lhavoc
 11612  check_cc -I/usr/local/include -L/usr/local/lib
 11613  BEGIN /tmp/ffconf.V1e5ldp7.c
 11614      1   #include <turing.h>
 11615      2   #include <stdint.h>
 11616      3   long check_turing_version(void) { return (long) turing_version; }
 11617      4   int main(void) { int ret = 0;
 11618      5    ret |= ((intptr_t)check_turing_version) & 0xFFFF;
 11619      6   return ret; }
 11620  END /tmp/ffconf.V1e5ldp7.c
 11621  gcc -D_ISOC99_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600 -DPIC -std=c11 -fPIC -pthread -I/usr/local/include -L/usr/local/lib -c -o /tmp/ffconf.i9eT8703.o /tmp/ffconf.V1e5ldp7.c
 11622  gcc -Wl,--as-needed -Wl,-z,noexecstack -I/usr/local/include -L/usr/local/lib -o /tmp/ffconf.oWzD6Ewr /tmp/ffconf.i9eT8703.o -lturing -lhavoc -lm -lz -pthread
 11623  /usr/local/lib/libturing.a(Encoder.cpp.o): In function `void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>(char*, char*, std::forward_iterator_tag) [clone .isra.390]':
 11624  Encoder.cpp:(.text+0x50): undefined reference to `std::__throw_logic_error(char const*)'
 11625  Encoder.cpp:(.text+0x9b): undefined reference to `std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_create(unsigned long&, unsigned long)'
 11626  /usr/local/lib/libturing.a(Encoder.cpp.o): In function `hashToString(std::vector<int, std::allocator<int> >, int)':
... over nine thousand lines deleted ...
 21104  ThreadPool.cpp:(.text._ZNSt5dequeIPN10ThreadPool4TaskESaIS2_EE17_M_reallocate_mapEmb[_ZNSt5dequeIPN10ThreadPool4TaskESaIS2_EE17_M_reallocate_mapEmb]+0x19e): undefined reference to `std::__throw_bad_alloc()'
 21105  /usr/local/lib/libturing.a(ThreadPool.cpp.o):(.data.rel.ro._ZTINSt6thread11_State_implISt12_Bind_simpleIFSt7_Mem_fnIM10ThreadPoolFvvEEPS3_EEEE[_ZTINSt6thread11_State_implISt12_Bind_simpleIFSt7_Mem_fnIM10ThreadPoolFvvEEPS3_EEEE]+0x0): undefined reference to `vtable for __cxxabiv1::__si_class_type_info'
 21106  /usr/local/lib/libturing.a(ThreadPool.cpp.o):(.data.rel.ro._ZTINSt6thread11_State_implISt12_Bind_simpleIFSt7_Mem_fnIM10ThreadPoolFvvEEPS3_EEEE[_ZTINSt6thread11_State_implISt12_Bind_simpleIFSt7_Mem_fnIM10ThreadPoolFvvEEPS3_EEEE]+0x10): undefined reference to `typeinfo for std::thread::_State'
 21107  collect2: error: ld returned 1 exit status
 21108  ERROR: libturing not found using pkg-config

That is, the static libraries contain references to a load of symbols which haven't been pulled in anywhere.  Adding --extra-libs='-lstdc++' removed a few thousand of the errors, but it clearly needs some other stuff as well.

Modulo being unable to test it this looks ok, so if you would explain how I've messed up so that I can test it then I would be happy to commit in a day or two if noone else objects.

Thanks,

- Mark
Mark Thompson Feb. 24, 2017, 9:04 p.m. UTC | #2
On 23/02/17 20:47, Mark Thompson wrote:
> On 21/02/17 17:15, Saverio Blasi wrote:
>> Hi all,
>>
>> We have recently circulated this new iteration (see below) of our work towards integrating our HEVC Turing codec within FFMpeg. Assuming that there are no more requests for changes, we would like to understand what is the timeline for integration within the project.
>>
>> Thanks a lot,
>>
>> All the best,
>> ----------------------------------------------------
>> Saverio Blasi, PhD
>> Senior Research Engineer 
>> BBC Research & Development
>> Centre House|56 Wood Lane|London|W12 7SB
>>
>>
>>
>>
>> -----Original Message-----
>> From: Saverio Blasi [mailto:saverio.blasi@bbc.co.uk] 
>> Sent: 13 February 2017 13:37
>> To: ffmpeg-devel@ffmpeg.org
>> Cc: Saverio Blasi <Saverio.Blasi@bbc.co.uk>
>> Subject: [PATCH v7] - Added Turing codec interface for ffmpeg
>>
>> - This patch contains the changes to interface the Turing codec (http://turingcodec.org/) with ffmpeg. The patch was modified to address the comments in the review as follows:
>>   - Added a pkg-config file to list all dependencies required by libturing. This should address the issue pointed out by Hendrik Leppkes on Fri 18/11/2016
>>   - As per suggestions of wm4, two functions (add_option and finalise_options) have been created. The former appends new options while the latter sets up the argv array of pointers to char* accordingly. add_option re-allocates the buffer for options using av_realloc
>>   - Additionally, both these functions handle the errors in case the memory wasn't allocated correctly
>>   - malloc|free|realloc have been substituted with their corresponding av_{malloc|free|realloc} version
>>   - Check on bit-depth has been removed since the ffmpeg already casts the right pix_fmt and bit depth
>>   - pix_fmts is now set in ff_libturing_encoder as in h264dec.c.
>>   - Changed usage of av_free with av_freep and fixed calls to free arrays
>>   - Added brackets to all if and for statements
>>   - Avoid repetition of code to free arrays in case of failure to initialise the libturing encoder
>>   - Some fixes to address the review from wm4 and Mark Thompson received on Wed 08/02/2017
>>   - Fixed indentation
>> ---
>>  LICENSE.md             |   1 +
>>  configure              |   5 +
>>  libavcodec/Makefile    |   1 +
>>  libavcodec/allcodecs.c |   1 +
>>  libavcodec/libturing.c | 320 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 328 insertions(+)
>>  create mode 100644 libavcodec/libturing.c
>>
>> diff --git a/LICENSE.md b/LICENSE.md
>> index 640633c..86e5371 100644
>> --- a/LICENSE.md
>> +++ b/LICENSE.md
>> @@ -85,6 +85,7 @@ The following libraries are under GPL:
>>  - frei0r
>>  - libcdio
>>  - librubberband
>> +- libturing
>>  - libvidstab
>>  - libx264
>>  - libx265
>> diff --git a/configure b/configure
>> index 7154142..a27cb8b 100755
>> --- a/configure
>> +++ b/configure
>> @@ -255,6 +255,7 @@ External library support:
>>    --enable-libssh          enable SFTP protocol via libssh [no]
>>    --enable-libtesseract    enable Tesseract, needed for ocr filter [no]
>>    --enable-libtheora       enable Theora encoding via libtheora [no]
>> +  --enable-libturing       enable H.265/HEVC encoding via libturing [no]
>>    --enable-libtwolame      enable MP2 encoding via libtwolame [no]
>>    --enable-libv4l2         enable libv4l2/v4l-utils [no]
>>    --enable-libvidstab      enable video stabilization using vid.stab [no]
>> @@ -1562,6 +1563,7 @@ EXTERNAL_LIBRARY_LIST="
>>      libssh
>>      libtesseract
>>      libtheora
>> +    libturing
>>      libtwolame
>>      libv4l2
>>      libvidstab
>> @@ -2858,6 +2860,7 @@ libspeex_decoder_deps="libspeex"
>>  libspeex_encoder_deps="libspeex"
>>  libspeex_encoder_select="audio_frame_queue"
>>  libtheora_encoder_deps="libtheora"
>> +libturing_encoder_deps="libturing"
>>  libtwolame_encoder_deps="libtwolame"
>>  libvo_amrwbenc_encoder_deps="libvo_amrwbenc"
>>  libvorbis_decoder_deps="libvorbis"
>> @@ -5131,6 +5134,7 @@ die_license_disabled gpl frei0r  die_license_disabled gpl libcdio  die_license_disabled gpl librubberband  die_license_disabled gpl libsmbclient
>> +die_license_disabled gpl libturing
>>  die_license_disabled gpl libvidstab
>>  die_license_disabled gpl libx264
>>  die_license_disabled gpl libx265
>> @@ -5789,6 +5793,7 @@ enabled libssh            && require_pkg_config libssh libssh/sftp.h sftp_init
>>  enabled libspeex          && require_pkg_config speex speex/speex.h speex_decoder_init -lspeex
>>  enabled libtesseract      && require_pkg_config tesseract tesseract/capi.h TessBaseAPICreate
>>  enabled libtheora         && require libtheora theora/theoraenc.h th_info_init -ltheoraenc -ltheoradec -logg
>> +enabled libturing         && require_pkg_config libturing turing.h turing_version
>>  enabled libtwolame        && require libtwolame twolame.h twolame_init -ltwolame &&
>>                               { check_lib twolame.h twolame_encode_buffer_float32_interleaved -ltwolame ||
>>                                 die "ERROR: libtwolame must be installed and version must be >= 0.3.10"; } diff --git a/libavcodec/Makefile b/libavcodec/Makefile index 43a6add..de5af1d 100644
> 
> Are you missing some link options?
> 
> I was wanting to test this, so I installed from your git (stable branch).  Trying to configure gives me:
> 
>  11608  check_pkg_config libturing turing.h turing_version
>  11609  pkg-config --exists --print-errors libturing
>  11610  check_func_headers turing.h turing_version -I/usr/local/include -L/usr/local/lib -lturing -lhavoc
>  11611  check_ld cc -I/usr/local/include -L/usr/local/lib -lturing -lhavoc
>  11612  check_cc -I/usr/local/include -L/usr/local/lib
>  11613  BEGIN /tmp/ffconf.V1e5ldp7.c
>  11614      1   #include <turing.h>
>  11615      2   #include <stdint.h>
>  11616      3   long check_turing_version(void) { return (long) turing_version; }
>  11617      4   int main(void) { int ret = 0;
>  11618      5    ret |= ((intptr_t)check_turing_version) & 0xFFFF;
>  11619      6   return ret; }
>  11620  END /tmp/ffconf.V1e5ldp7.c
>  11621  gcc -D_ISOC99_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600 -DPIC -std=c11 -fPIC -pthread -I/usr/local/include -L/usr/local/lib -c -o /tmp/ffconf.i9eT8703.o /tmp/ffconf.V1e5ldp7.c
>  11622  gcc -Wl,--as-needed -Wl,-z,noexecstack -I/usr/local/include -L/usr/local/lib -o /tmp/ffconf.oWzD6Ewr /tmp/ffconf.i9eT8703.o -lturing -lhavoc -lm -lz -pthread
>  11623  /usr/local/lib/libturing.a(Encoder.cpp.o): In function `void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>(char*, char*, std::forward_iterator_tag) [clone .isra.390]':
>  11624  Encoder.cpp:(.text+0x50): undefined reference to `std::__throw_logic_error(char const*)'
>  11625  Encoder.cpp:(.text+0x9b): undefined reference to `std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_create(unsigned long&, unsigned long)'
>  11626  /usr/local/lib/libturing.a(Encoder.cpp.o): In function `hashToString(std::vector<int, std::allocator<int> >, int)':
> ... over nine thousand lines deleted ...
>  21104  ThreadPool.cpp:(.text._ZNSt5dequeIPN10ThreadPool4TaskESaIS2_EE17_M_reallocate_mapEmb[_ZNSt5dequeIPN10ThreadPool4TaskESaIS2_EE17_M_reallocate_mapEmb]+0x19e): undefined reference to `std::__throw_bad_alloc()'
>  21105  /usr/local/lib/libturing.a(ThreadPool.cpp.o):(.data.rel.ro._ZTINSt6thread11_State_implISt12_Bind_simpleIFSt7_Mem_fnIM10ThreadPoolFvvEEPS3_EEEE[_ZTINSt6thread11_State_implISt12_Bind_simpleIFSt7_Mem_fnIM10ThreadPoolFvvEEPS3_EEEE]+0x0): undefined reference to `vtable for __cxxabiv1::__si_class_type_info'
>  21106  /usr/local/lib/libturing.a(ThreadPool.cpp.o):(.data.rel.ro._ZTINSt6thread11_State_implISt12_Bind_simpleIFSt7_Mem_fnIM10ThreadPoolFvvEEPS3_EEEE[_ZTINSt6thread11_State_implISt12_Bind_simpleIFSt7_Mem_fnIM10ThreadPoolFvvEEPS3_EEEE]+0x10): undefined reference to `typeinfo for std::thread::_State'
>  21107  collect2: error: ld returned 1 exit status
>  21108  ERROR: libturing not found using pkg-config
> 
> That is, the static libraries contain references to a load of symbols which haven't been pulled in anywhere.  Adding --extra-libs='-lstdc++' removed a few thousand of the errors, but it clearly needs some other stuff as well.
> 
> Modulo being unable to test it this looks ok, so if you would explain how I've messed up so that I can test it then I would be happy to commit in a day or two if noone else objects.

Right, I had a bit more of a look at it and was able to "fix" it by adding the boost libraries to the ffmpeg configure line as well.

Actually running it it all looks good to me, if as fast as a particularly lethargic snail (like all H.265 encoders targetting quality).

valgrind came up with some errors, but they appear to be on the libturing side rather than in the ffmpeg wrapper (i.e. please do fix them, but they don't matter to this patch).

Thanks,

- Mark



Valgrind errors (please do run it yourself, I imagine you have the resources to do so faster than the ~4fph I manage):

A memory leak (once per frame):

==4044== 120 bytes in 5 blocks are definitely lost in loss record 2 of 2
==4044==    at 0x4C2B21F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4044==    by 0xE8DD99: TaskEncodeInput<Handler<Encode<void>, StateEncode> >::run() (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xE73968: ThreadPool::worker() (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0x760DEFE: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.22)
==4044==    by 0x7AF4463: start_thread (pthread_create.c:333)
==4044==    by 0x7DF29DE: clone (clone.S:105)
==4044== 

Some invalid memory accesses which manage to avoid segfaulting because of page granularity:

==4044== Invalid read of size 16
==4044==    at 0x1609CC6A: ???
==4044==    by 0xF0EDD8: int measureSatd<unsigned char>(havoc_table_hadamard_satd<unsigned char>*, Raster<unsigned char>, Raster<unsigned char>, int, int) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF1AF76: FixedPoint<long, 16> measurePuCost<Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF34EE8: void Search<prediction_unit>::State<unsigned char>::go2<Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(prediction_unit const&, Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF35637: void Search<prediction_unit>::go<Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(prediction_unit const&, Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF899E8: void Syntax<coding_unit>::go<Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(coding_unit const&, Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF8A3C4: void searchInterCuPartMode<unsigned char, Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&, coding_quadtree const&, int, CandidateStash<unsigned char>*&, CandidateStash<unsigned char>*&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF8A9EA: bool searchInterCu<unsigned char, Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&, coding_quadtree const&, CandidateStash<unsigned char>*&, CandidateStash<unsigned char>*&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF8AE3C: void Search<coding_unit>::go<Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(coding_unit const&, Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF8BCF5: void Syntax<coding_quadtree>::go<Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(coding_quadtree const&, Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF8C394: void Search<coding_quadtree>::go<Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(coding_quadtree const&, Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF8C6B8: void Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>::operator()<coding_quadtree>(coding_quadtree) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==  Address 0x1b5c02d4 is 518,388 bytes inside a block of size 518,400 alloc'd
==4044==    at 0x4C2CF16: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4044==    by 0x4C2D021: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4044==    by 0xE74932: Picture<unsigned char>::Picture(int, int, int, int, int, int) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xE33FE8: turing_encoder::encode(turing_picture*) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xE2B0C8: turing_encode_picture (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0x713ABD: libturing_encode_frame (libturing.c:265)
==4044==    by 0x8D0A52: avcodec_encode_video2 (utils.c:2000)
==4044==    by 0x8D0CF1: do_encode (utils.c:2947)
==4044==    by 0x8D6B5B: avcodec_send_frame (utils.c:2996)
==4044==    by 0x29C424: do_video_out (ffmpeg.c:1261)
==4044==    by 0x29D7EF: reap_filters (ffmpeg.c:1467)
==4044==    by 0x2A453D: transcode_step (ffmpeg.c:4349)
==4044==    by 0x2A453D: transcode (ffmpeg.c:4393)

==4044== Invalid read of size 16
==4044==    at 0x15CA2CCF: ???
==4044==    by 0xF34E0B: void Search<prediction_unit>::State<unsigned char>::go2<Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(prediction_unit const&, Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF35637: void Search<prediction_unit>::go<Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(prediction_unit const&, Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF89CB1: void Syntax<coding_unit>::go<Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(coding_unit const&, Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF8A3C4: void searchInterCuPartMode<unsigned char, Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&, coding_quadtree const&, int, CandidateStash<unsigned char>*&, CandidateStash<unsigned char>*&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF8A65D: bool searchInterCu<unsigned char, Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&, coding_quadtree const&, CandidateStash<unsigned char>*&, CandidateStash<unsigned char>*&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF8AE3C: void Search<coding_unit>::go<Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(coding_unit const&, Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF8BCF5: void Syntax<coding_quadtree>::go<Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(coding_quadtree const&, Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF8C394: void Search<coding_quadtree>::go<Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(coding_quadtree const&, Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF8C6B8: void Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>::operator()<coding_quadtree>(coding_quadtree) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF8BD35: void Syntax<coding_quadtree>::go<Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(coding_quadtree const&, Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xF8C2C0: void Search<coding_quadtree>::go<Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode> >(coding_quadtree const&, Handler<Search<Mode<1u> >, Candidate<unsigned char>, StateEncodeSubstream<unsigned char>, StateEncodePicture2<unsigned char>, StateEncode>&) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==  Address 0x1b83d478 is 2,073,592 bytes inside a block of size 2,073,600 alloc'd
==4044==    at 0x4C2CF16: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4044==    by 0x4C2D021: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4044==    by 0xE74932: Picture<unsigned char>::Picture(int, int, int, int, int, int) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xE33FE8: turing_encoder::encode(turing_picture*) (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0xE2B0C8: turing_encode_picture (in /home/mrt/video/ffmpeg/turing/build/ffmpeg_g)
==4044==    by 0x713ABD: libturing_encode_frame (libturing.c:265)
==4044==    by 0x8D0A52: avcodec_encode_video2 (utils.c:2000)
==4044==    by 0x8D0CF1: do_encode (utils.c:2947)
==4044==    by 0x8D6B5B: avcodec_send_frame (utils.c:2996)
==4044==    by 0x29C424: do_video_out (ffmpeg.c:1261)
==4044==    by 0x29D7EF: reap_filters (ffmpeg.c:1467)
==4044==    by 0x2A453D: transcode_step (ffmpeg.c:4349)
==4044==    by 0x2A453D: transcode (ffmpeg.c:4393)

Also a lot of "Conditional jump or move depends on uninitialised value(s)".
Saverio Blasi Feb. 27, 2017, 8:29 a.m. UTC | #3
> Right, I had a bit more of a look at it and was able to "fix" it by adding the boost libraries to the ffmpeg configure line as well.

>

> Actually running it it all looks good to me, if as fast as a particularly lethargic snail (like all H.265 encoders targetting quality).

>

> valgrind came up with some errors, but they appear to be on the libturing side rather than in the ffmpeg wrapper (i.e. please do fix them, but they don't matter to this patch).

>

> Thanks,

>

> - Mark


Dear Mark,

Thanks a lot for this, we are very happy to know the patch will be integrated. 
Regarding the problems you mention, we will have a look at the output of valgrind, and are also working on speeding up the encoder. 

All the best,
----------------------------------------------------
Saverio Blasi, PhD
Senior Research Engineer 
BBC Research & Development
Centre House|56 Wood Lane|London|W12 7SB
Mark Thompson Feb. 27, 2017, 11:13 a.m. UTC | #4
On 27/02/17 08:29, Saverio Blasi wrote:
>> Right, I had a bit more of a look at it and was able to "fix" it by adding the boost libraries to the ffmpeg configure line as well.
>>
>> Actually running it it all looks good to me, if as fast as a particularly lethargic snail (like all H.265 encoders targetting quality).
>>
>> valgrind came up with some errors, but they appear to be on the libturing side rather than in the ffmpeg wrapper (i.e. please do fix them, but they don't matter to this patch).
>>
>> Thanks,
>>
>> - Mark
> 
> Dear Mark,
> 
> Thanks a lot for this, we are very happy to know the patch will be integrated. 
> Regarding the problems you mention, we will have a look at the output of valgrind, and are also working on speeding up the encoder.

Also the pkgconfig file:
* It refers to the libturing build directory, which need not exist after installation.
* I think libstdc++ and the boost libraries are always required, not just when linking statically.  (Not sure how this interacts with other parts, though - it seems to not like the system boost libraries on Ubuntu 16.04 (1.58): do you need to install your custom boost libraries as well?)

It should be possible to build and run with just "--enable-gpl --enable-libturing" on a clean "make install"ed libturing.

Thanks,

- Mark
Carl Eugen Hoyos Feb. 27, 2017, 11:17 a.m. UTC | #5
2017-02-27 12:13 GMT+01:00 Mark Thompson <sw@jkqxz.net>:

> * I think libstdc++ and the boost libraries are always required

If this is true, not using pkgconfig is the more user-friendly solution.

Carl Eugen
Saverio Blasi Feb. 28, 2017, 10:27 a.m. UTC | #6
Dear Mark,

We provide our custom Boost libraries. In our tests we are able to build and run using just "--enable-gpl --enable-libturing". We would prefer to keep the pkgconfig file. I am not sure I fully understand what is required to be changed there (if anything), could you please be a bit more specific?

Thanks a lot,
Saverio

-----Original Message-----
From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Mark Thompson

Sent: 27 February 2017 11:14
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v7] - Added Turing codec interface for ffmpeg

On 27/02/17 08:29, Saverio Blasi wrote:
>> Right, I had a bit more of a look at it and was able to "fix" it by adding the boost libraries to the ffmpeg configure line as well.

>>

>> Actually running it it all looks good to me, if as fast as a particularly lethargic snail (like all H.265 encoders targetting quality).

>>

>> valgrind came up with some errors, but they appear to be on the libturing side rather than in the ffmpeg wrapper (i.e. please do fix them, but they don't matter to this patch).

>>

>> Thanks,

>>

>> - Mark

>

> Dear Mark,

>

> Thanks a lot for this, we are very happy to know the patch will be integrated.

> Regarding the problems you mention, we will have a look at the output of valgrind, and are also working on speeding up the encoder.


Also the pkgconfig file:
* It refers to the libturing build directory, which need not exist after installation.
* I think libstdc++ and the boost libraries are always required, not just when linking statically.  (Not sure how this interacts with other parts, though - it seems to not like the system boost libraries on Ubuntu 16.04 (1.58): do you need to install your custom boost libraries as well?)

It should be possible to build and run with just "--enable-gpl --enable-libturing" on a clean "make install"ed libturing.

Thanks,

- Mark

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


-----------------------------
http://www.bbc.co.uk
This e-mail (and any attachments) is confidential and
may contain personal views which are not the views of the BBC unless specifically stated.
If you have received it in
error, please delete it from your system.
Do not use, copy or disclose the
information in any way nor act in reliance on it and notify the sender
immediately.
Please note that the BBC monitors e-mails
sent or received.
Further communication will signify your consent to
this.
-----------------------------
Mark Thompson Feb. 28, 2017, 11:01 a.m. UTC | #7
On 28/02/17 10:27, Saverio Blasi wrote:
> Dear Mark,
> 
> We provide our custom Boost libraries. In our tests we are able to build and run using just "--enable-gpl --enable-libturing". We would prefer to keep the pkgconfig file. I am not sure I fully understand what is required to be changed there (if anything), could you please be a bit more specific?

Well, where do those libraries end up?  I agree the pkgconfig file is the correct way to do this, but it currently points at the libturing build directory which need not exist when building ffmpeg.

You should be able to install the library with some prefix (below using the default /usr/local) and then use it in ffmpeg without any other magic:

mkdir libturing-build
cd libturing-build
cmake ../path/to/libturing/source/
make
make install
cd ..
rm -rf libturing-build

mkdir ffmpeg-build
cd ffmpeg-build
../path/to/ffmpeg/source/configure --enable-gpl --enable-libturing
make
./ffmpeg ... -c:v libturing ...
make install
ffmpeg ... -c:v libturing ...

For me, this sequence currently fails to link for the configure test because the custom boost libraries are not found (were not installed anywhere, as far as I can tell).

Have you tried this on a vanilla install of a standard distribution?  It sounds like you might have more stuff in your development setup which allows it to work.

- Mark


> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Mark Thompson
> Sent: 27 February 2017 11:14
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v7] - Added Turing codec interface for ffmpeg
> 
> On 27/02/17 08:29, Saverio Blasi wrote:
>>> Right, I had a bit more of a look at it and was able to "fix" it by adding the boost libraries to the ffmpeg configure line as well.
>>>
>>> Actually running it it all looks good to me, if as fast as a particularly lethargic snail (like all H.265 encoders targetting quality).
>>>
>>> valgrind came up with some errors, but they appear to be on the libturing side rather than in the ffmpeg wrapper (i.e. please do fix them, but they don't matter to this patch).
>>>
>>> Thanks,
>>>
>>> - Mark
>>
>> Dear Mark,
>>
>> Thanks a lot for this, we are very happy to know the patch will be integrated.
>> Regarding the problems you mention, we will have a look at the output of valgrind, and are also working on speeding up the encoder.
> 
> Also the pkgconfig file:
> * It refers to the libturing build directory, which need not exist after installation.
> * I think libstdc++ and the boost libraries are always required, not just when linking statically.  (Not sure how this interacts with other parts, though - it seems to not like the system boost libraries on Ubuntu 16.04 (1.58): do you need to install your custom boost libraries as well?)
> 
> It should be possible to build and run with just "--enable-gpl --enable-libturing" on a clean "make install"ed libturing.
> 
> Thanks,
> 
> - Mark
Saverio Blasi March 16, 2017, 10:54 a.m. UTC | #8
Dear Mark,

Sorry for the long delay in replying.
We have now pushed some changes to our main Turing codec repository to address your comments below. Our custom boost libraries should now be correctly installed so there is no need for preserving any additional Turing build folder. These changes are completely transparent to our ffmpeg patch, which remains valid at v7.

We would appreciate comments on this.

Thanks,

All the best,
----------------------------------------------------
Saverio Blasi, PhD
Senior Research Engineer 
BBC Research & Development
Centre House|56 Wood Lane|London|W12 7SB




-----Original Message-----
From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Mark Thompson

Sent: 28 February 2017 11:02
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v7] - Added Turing codec interface for ffmpeg

On 28/02/17 10:27, Saverio Blasi wrote:
> Dear Mark,

> 

> We provide our custom Boost libraries. In our tests we are able to build and run using just "--enable-gpl --enable-libturing". We would prefer to keep the pkgconfig file. I am not sure I fully understand what is required to be changed there (if anything), could you please be a bit more specific?


Well, where do those libraries end up?  I agree the pkgconfig file is the correct way to do this, but it currently points at the libturing build directory which need not exist when building ffmpeg.

You should be able to install the library with some prefix (below using the default /usr/local) and then use it in ffmpeg without any other magic:

mkdir libturing-build
cd libturing-build
cmake ../path/to/libturing/source/
make
make install
cd ..
rm -rf libturing-build

mkdir ffmpeg-build
cd ffmpeg-build
../path/to/ffmpeg/source/configure --enable-gpl --enable-libturing make ./ffmpeg ... -c:v libturing ...
make install
ffmpeg ... -c:v libturing ...

For me, this sequence currently fails to link for the configure test because the custom boost libraries are not found (were not installed anywhere, as far as I can tell).

Have you tried this on a vanilla install of a standard distribution?  It sounds like you might have more stuff in your development setup which allows it to work.

- Mark


> -----Original Message-----

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf 

> Of Mark Thompson

> Sent: 27 February 2017 11:14

> To: FFmpeg development discussions and patches 

> <ffmpeg-devel@ffmpeg.org>

> Subject: Re: [FFmpeg-devel] [PATCH v7] - Added Turing codec interface 

> for ffmpeg

> 

> On 27/02/17 08:29, Saverio Blasi wrote:

>>> Right, I had a bit more of a look at it and was able to "fix" it by adding the boost libraries to the ffmpeg configure line as well.

>>>

>>> Actually running it it all looks good to me, if as fast as a particularly lethargic snail (like all H.265 encoders targetting quality).

>>>

>>> valgrind came up with some errors, but they appear to be on the libturing side rather than in the ffmpeg wrapper (i.e. please do fix them, but they don't matter to this patch).

>>>

>>> Thanks,

>>>

>>> - Mark

>>

>> Dear Mark,

>>

>> Thanks a lot for this, we are very happy to know the patch will be integrated.

>> Regarding the problems you mention, we will have a look at the output of valgrind, and are also working on speeding up the encoder.

> 

> Also the pkgconfig file:

> * It refers to the libturing build directory, which need not exist after installation.

> * I think libstdc++ and the boost libraries are always required, not 

> just when linking statically.  (Not sure how this interacts with other 

> parts, though - it seems to not like the system boost libraries on 

> Ubuntu 16.04 (1.58): do you need to install your custom boost 

> libraries as well?)

> 

> It should be possible to build and run with just "--enable-gpl --enable-libturing" on a clean "make install"ed libturing.

> 

> Thanks,

> 

> - Mark


_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Clément Bœsch March 19, 2017, 6:56 p.m. UTC | #9
On Tue, Feb 21, 2017 at 05:15:59PM +0000, Saverio Blasi wrote:
[...]
>  enabled libspeex          && require_pkg_config speex speex/speex.h speex_decoder_init -lspeex
>  enabled libtesseract      && require_pkg_config tesseract tesseract/capi.h TessBaseAPICreate
>  enabled libtheora         && require libtheora theora/theoraenc.h th_info_init -ltheoraenc -ltheoradec -logg

> +enabled libturing         && require_pkg_config libturing turing.h turing_version

You probably want to specify a minimal version

[...]
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 
> +02110-1301 USA  */
> +

This looks mangled.

> +#include <turing.h>

Please add a line break after this

> +#include "libavutil/internal.h"
> +#include "libavutil/common.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/pixdesc.h"
> +#include "avcodec.h"
> +#include "internal.h"
> +
> +#define MAX_OPTION_LENGTH 256
> +
> +typedef struct libturingEncodeContext {
> +    const AVClass *class;
> +    turing_encoder *encoder;
> +    const char *options;
> +} libturingEncodeContext;
> +
> +typedef struct optionContext {

> +    char** argv;
> +    char* options;
> +    char* s;

Style here and in other places: * stick to the var

> +    int options_buffer_size;
> +    int buffer_filled;
> +    int options_added;
> +} optionContext;
> +

> +static av_cold int libturing_encode_close(AVCodecContext *avctx) {

Style: here and in other places missing line break before "{" for
functions.

> +    libturingEncodeContext *ctx = avctx->priv_data;
> +

> +    if (ctx->encoder) {
> +        turing_destroy_encoder(ctx->encoder);
> +    }

Note: the NULL check should probably be part of the libturing API to
simplify code paths for the users (just like the stdlib free(3)
convention).

> +    return 0;
> +}
> +
> +static av_cold int add_option(const char* current_option, 
> +optionContext* option_ctx) {

This function should be replaced with AVBPrint API. It will be much
simpler (that function will basically disappear) and will allow the caller
to check for errors only once.

[...]
> +static av_cold int libturing_encode_init(AVCodecContext *avctx) {
> +    libturingEncodeContext *ctx = avctx->priv_data;
> +    const int bit_depth = av_pix_fmt_desc_get(avctx->pix_fmt)->comp[0].depth;
> +    int error_code = 0;
> +

> +    optionContext encoder_options;

use "encoder_options = {0}" so you don't need to fill each and every field
later on, and risk to forget one in the future.

[...]
> +                int const illegal_option =
> +                    !strcmp("input-res", en->key) ||
> +                    !strcmp("frame-rate", en->key) ||
> +                    !strcmp("f", en->key) ||
> +                    !strcmp("frames", en->key) ||
> +                    !strcmp("sar", en->key) ||
> +                    !strcmp("bit-depth", en->key) ||
> +                    !strcmp("internal-bit-depth", en->key);

you could use av_match_name(en->key, "input-res,frame-rate,f,...") here.

> +                if (illegal_option) {
> +                    av_log(avctx, AV_LOG_WARNING, "%s=%s ignored - this parameter is inferred from ffmpeg.\n", en->key, en->value);
> +                } else {
> +                    if (turing_check_binary_option(en->key)) {
> +                        snprintf(option_string, MAX_OPTION_LENGTH, "--%s", en->key);
> +                    } else {
> +                        snprintf(option_string, MAX_OPTION_LENGTH, "--%s=%s", en->key, en->value);
> +                    }
> +                    if ((error_code = add_option(option_string, &encoder_options)) > 0) {
> +                        goto fail;

leaking dict here.

> +                    }
> +                }
> +            }
> +            av_dict_free(&dict);
> +        }
> +    }
> +
> +    if ((error_code = add_option("dummy-input-filename", &encoder_options)) > 0) {
> +        goto fail;
> +    }
> +
> +    if ((error_code = finalise_options(&encoder_options)) > 0) {
> +        goto fail;
> +    }
> +
> +    settings.argv = (char const**)encoder_options.argv;
> +    settings.argc = encoder_options.options_added;
> +

> +    for (int i=0; i<settings.argc; ++i) {

- int is not allowed here
- the style is broken (missing spaces)
- ++i is a c++ idiom and doesn't belong here

> +        av_log(avctx, AV_LOG_VERBOSE, "arg %d: %s\n", i, settings.argv[i]);
> +    }
> +
> +    ctx->encoder = turing_create_encoder(settings);
> +
> +    if (!ctx->encoder) {
> +        av_log(avctx, AV_LOG_ERROR, "Failed to create libturing encoder.\n");
> +        error_code = AVERROR_INVALIDDATA;
> +        goto fail;
> +    }
> +
> +    if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
> +        turing_bitstream const *bitstream;
> +        bitstream = turing_encode_headers(ctx->encoder);
> +        if (bitstream->size <= 0) {
> +            av_log(avctx, AV_LOG_ERROR, "Failed to encode headers.\n");
> +            turing_destroy_encoder(ctx->encoder);
> +            error_code = AVERROR_INVALIDDATA;
> +            goto fail;
> +        }
> +
> +        avctx->extradata_size = bitstream->size;
> +
> +        avctx->extradata = av_mallocz(avctx->extradata_size + AV_INPUT_BUFFER_PADDING_SIZE);
> +        if (!avctx->extradata) {
> +            av_log(avctx, AV_LOG_ERROR, "Failed to allocate HEVC extradata %d bytes\n", avctx->extradata_size);
> +            turing_destroy_encoder(ctx->encoder);
> +            error_code =  AVERROR(ENOMEM);
> +            goto fail;
> +        }
> +
> +        memcpy(avctx->extradata, bitstream->p, bitstream->size);
> +    }
> +
> +    av_freep(&encoder_options.argv);
> +    av_freep(&encoder_options.options);
> +    return 0;
> +
> +fail:

> +    av_log(avctx, AV_LOG_ERROR, "Error while initialising the Turing codec.\n");

This message is useless as it won't provide any additional information.

Dropping that log allows you to replace the "fail" label with an "out"
label and drop the 3 lines above your current "fail" label.

> +    av_freep(&encoder_options.argv);
> +    av_freep(&encoder_options.options);
> +    return error_code;
> +}
> +
[...]
> +    ret = ff_alloc_packet2(avctx, pkt, output->bitstream.size, 0);
> +    if (ret < 0) {

> +        av_log(avctx, AV_LOG_ERROR, "Error getting output packet.\n");

Useless log, please drop it

> +        return ret;
> +    }
> +
> +    memcpy(pkt->data, output->bitstream.p, output->bitstream.size);
> +
> +    pkt->pts = output->pts;
> +    pkt->dts = output->dts;
> +    if (output->keyframe) {
> +        pkt->flags |= AV_PKT_FLAG_KEY;
> +    }
> +
> +    *got_packet = 1;
> +    return 0;
> +}
> +
> +static const AVOption options[] = {

> +    { "turing-params", "configure additional turing encoder parameters", offsetof(libturingEncodeContext, options), AV_OPT_TYPE_STRING,{ 0 }, 0, 0, AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM },

"{ 0 }, 0, 0" is ugly; drop them and use a named ".flags = AV_OPT..."

[...]

Regards,
diff mbox

Patch

diff --git a/LICENSE.md b/LICENSE.md
index 640633c..86e5371 100644
--- a/LICENSE.md
+++ b/LICENSE.md
@@ -85,6 +85,7 @@  The following libraries are under GPL:
 - frei0r
 - libcdio
 - librubberband
+- libturing
 - libvidstab
 - libx264
 - libx265
diff --git a/configure b/configure
index 7154142..a27cb8b 100755
--- a/configure
+++ b/configure
@@ -255,6 +255,7 @@  External library support:
   --enable-libssh          enable SFTP protocol via libssh [no]
   --enable-libtesseract    enable Tesseract, needed for ocr filter [no]
   --enable-libtheora       enable Theora encoding via libtheora [no]
+  --enable-libturing       enable H.265/HEVC encoding via libturing [no]
   --enable-libtwolame      enable MP2 encoding via libtwolame [no]
   --enable-libv4l2         enable libv4l2/v4l-utils [no]
   --enable-libvidstab      enable video stabilization using vid.stab [no]
@@ -1562,6 +1563,7 @@  EXTERNAL_LIBRARY_LIST="
     libssh
     libtesseract
     libtheora
+    libturing
     libtwolame
     libv4l2
     libvidstab
@@ -2858,6 +2860,7 @@  libspeex_decoder_deps="libspeex"
 libspeex_encoder_deps="libspeex"
 libspeex_encoder_select="audio_frame_queue"
 libtheora_encoder_deps="libtheora"
+libturing_encoder_deps="libturing"
 libtwolame_encoder_deps="libtwolame"
 libvo_amrwbenc_encoder_deps="libvo_amrwbenc"
 libvorbis_decoder_deps="libvorbis"
@@ -5131,6 +5134,7 @@  die_license_disabled gpl frei0r  die_license_disabled gpl libcdio  die_license_disabled gpl librubberband  die_license_disabled gpl libsmbclient
+die_license_disabled gpl libturing
 die_license_disabled gpl libvidstab
 die_license_disabled gpl libx264
 die_license_disabled gpl libx265
@@ -5789,6 +5793,7 @@  enabled libssh            && require_pkg_config libssh libssh/sftp.h sftp_init
 enabled libspeex          && require_pkg_config speex speex/speex.h speex_decoder_init -lspeex
 enabled libtesseract      && require_pkg_config tesseract tesseract/capi.h TessBaseAPICreate
 enabled libtheora         && require libtheora theora/theoraenc.h th_info_init -ltheoraenc -ltheoradec -logg
+enabled libturing         && require_pkg_config libturing turing.h turing_version
 enabled libtwolame        && require libtwolame twolame.h twolame_init -ltwolame &&
                              { check_lib twolame.h twolame_encode_buffer_float32_interleaved -ltwolame ||
                                die "ERROR: libtwolame must be installed and version must be >= 0.3.10"; } diff --git a/libavcodec/Makefile b/libavcodec/Makefile index 43a6add..de5af1d 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -883,6 +883,7 @@  OBJS-$(CONFIG_LIBSHINE_ENCODER)           += libshine.o
 OBJS-$(CONFIG_LIBSPEEX_DECODER)           += libspeexdec.o
 OBJS-$(CONFIG_LIBSPEEX_ENCODER)           += libspeexenc.o
 OBJS-$(CONFIG_LIBTHEORA_ENCODER)          += libtheoraenc.o
+OBJS-$(CONFIG_LIBTURING_ENCODER)          += libturing.o
 OBJS-$(CONFIG_LIBTWOLAME_ENCODER)         += libtwolame.o
 OBJS-$(CONFIG_LIBVO_AMRWBENC_ENCODER)     += libvo-amrwbenc.o
 OBJS-$(CONFIG_LIBVORBIS_DECODER)          += libvorbisdec.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c index f92b2b7..f650591 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -615,6 +615,7 @@  void avcodec_register_all(void)
     REGISTER_ENCODER(LIBSHINE,          libshine);
     REGISTER_ENCDEC (LIBSPEEX,          libspeex);
     REGISTER_ENCODER(LIBTHEORA,         libtheora);
+    REGISTER_ENCODER(LIBTURING,         libturing);
     REGISTER_ENCODER(LIBTWOLAME,        libtwolame);
     REGISTER_ENCODER(LIBVO_AMRWBENC,    libvo_amrwbenc);
     REGISTER_ENCDEC (LIBVORBIS,         libvorbis);
diff --git a/libavcodec/libturing.c b/libavcodec/libturing.c new file mode 100644 index 0000000..a224bba
--- /dev/null
+++ b/libavcodec/libturing.c
@@ -0,0 +1,320 @@ 
+/*
+ * libturing encoder
+ *
+ * Copyright (c) 2017 Turing Codec contributors
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 
+02110-1301 USA  */
+
+#include <turing.h>
+#include "libavutil/internal.h"
+#include "libavutil/common.h"
+#include "libavutil/opt.h"
+#include "libavutil/pixdesc.h"
+#include "avcodec.h"
+#include "internal.h"
+
+#define MAX_OPTION_LENGTH 256
+
+typedef struct libturingEncodeContext {
+    const AVClass *class;
+    turing_encoder *encoder;
+    const char *options;
+} libturingEncodeContext;
+
+typedef struct optionContext {
+    char** argv;
+    char* options;
+    char* s;
+    int options_buffer_size;
+    int buffer_filled;
+    int options_added;
+} optionContext;
+
+static av_cold int libturing_encode_close(AVCodecContext *avctx) {
+    libturingEncodeContext *ctx = avctx->priv_data;
+
+    if (ctx->encoder) {
+        turing_destroy_encoder(ctx->encoder);
+    }
+    return 0;
+}
+
+static av_cold int add_option(const char* current_option, 
+optionContext* option_ctx) {
+    int option_length = strlen(current_option);
+    char *temp_ptr;
+
+
+    if (option_ctx->buffer_filled + option_length + 1 > option_ctx->options_buffer_size) {
+        if(option_ctx->options == NULL) {
+            option_ctx->options = av_malloc(option_length + 1);
+            if(option_ctx->options == NULL) {
+                return AVERROR(ENOMEM);
+            }
+        } else {
+            temp_ptr = av_realloc(option_ctx->options, option_ctx->options_buffer_size + option_length + 1);
+            if (temp_ptr == NULL) {
+                return AVERROR(ENOMEM);
+            }
+            option_ctx->options = temp_ptr;
+        }
+        option_ctx->options_buffer_size += option_length + 1;
+        option_ctx->s = option_ctx->options + option_ctx->buffer_filled;
+    }
+    strcpy(option_ctx->s, current_option);
+    option_ctx->s += 1 + option_length;
+    option_ctx->options_added++;
+    option_ctx->buffer_filled += option_length + 1;
+    return 0;
+}
+
+static av_cold int finalise_options(optionContext* option_ctx) {
+    if (option_ctx->options_added) {
+        char *p;
+        option_ctx->argv = av_malloc(option_ctx->options_added * sizeof(char*));
+        if (option_ctx->argv == NULL) {
+            return AVERROR(ENOMEM);
+        }
+        p = option_ctx->options;
+        for(int option_idx=0; option_idx<option_ctx->options_added; option_idx++) {
+            option_ctx->argv[option_idx] = p;
+            p += strlen(p) + 1;
+        }
+    }
+    return 0;
+}
+
+static av_cold int libturing_encode_init(AVCodecContext *avctx) {
+    libturingEncodeContext *ctx = avctx->priv_data;
+    const int bit_depth = av_pix_fmt_desc_get(avctx->pix_fmt)->comp[0].depth;
+    int error_code = 0;
+
+    optionContext encoder_options;
+    turing_encoder_settings settings;
+    char option_string[MAX_OPTION_LENGTH];
+    double frame_rate;
+
+    frame_rate = (double)avctx->time_base.den / (avctx->time_base.num * 
+ avctx->ticks_per_frame);
+
+    encoder_options.buffer_filled = 0;
+    encoder_options.options_added = 0;
+    encoder_options.options_buffer_size = 0;
+    encoder_options.options = NULL;
+    encoder_options.s = encoder_options.options;
+    encoder_options.argv = NULL;
+
+    // Add baseline command line options
+    if ((error_code = add_option("turing", &encoder_options)) > 0) {
+        goto fail;
+    }
+
+    if ((error_code = add_option("--frames=0", &encoder_options)) > 0) {
+        goto fail;
+    }
+
+    snprintf(option_string, MAX_OPTION_LENGTH, "--input-res=%dx%d", avctx->width, avctx->height);
+    if ((error_code = add_option(option_string, &encoder_options)) > 0) {
+        goto fail;
+    }
+
+    snprintf(option_string, MAX_OPTION_LENGTH, "--frame-rate=%f", frame_rate);
+    if ((error_code = add_option(option_string, &encoder_options)) > 0) {
+        goto fail;
+    }
+
+    snprintf(option_string, MAX_OPTION_LENGTH, "--bit-depth=%d", bit_depth);
+    if ((error_code = add_option(option_string, &encoder_options)) > 0) {
+        goto fail;
+    }
+
+    if (avctx->sample_aspect_ratio.num > 0 && avctx->sample_aspect_ratio.den > 0) {
+        int sar_num, sar_den;
+
+        av_reduce(&sar_num, &sar_den,
+            avctx->sample_aspect_ratio.num,
+            avctx->sample_aspect_ratio.den, 65535);
+        snprintf(option_string, MAX_OPTION_LENGTH, "--sar=%d:%d", sar_num, sar_den);
+        if ((error_code = add_option(option_string, &encoder_options)) > 0) {
+            goto fail;
+        }
+    }
+
+    // Parse additional command line options
+    if (ctx->options) {
+        AVDictionary *dict = NULL;
+        AVDictionaryEntry *en = NULL;
+
+        if (!av_dict_parse_string(&dict, ctx->options, "=", ":", 0)) {
+            while ((en = av_dict_get(dict, "", en, AV_DICT_IGNORE_SUFFIX))) {
+                int const illegal_option =
+                    !strcmp("input-res", en->key) ||
+                    !strcmp("frame-rate", en->key) ||
+                    !strcmp("f", en->key) ||
+                    !strcmp("frames", en->key) ||
+                    !strcmp("sar", en->key) ||
+                    !strcmp("bit-depth", en->key) ||
+                    !strcmp("internal-bit-depth", en->key);
+                if (illegal_option) {
+                    av_log(avctx, AV_LOG_WARNING, "%s=%s ignored - this parameter is inferred from ffmpeg.\n", en->key, en->value);
+                } else {
+                    if (turing_check_binary_option(en->key)) {
+                        snprintf(option_string, MAX_OPTION_LENGTH, "--%s", en->key);
+                    } else {
+                        snprintf(option_string, MAX_OPTION_LENGTH, "--%s=%s", en->key, en->value);
+                    }
+                    if ((error_code = add_option(option_string, &encoder_options)) > 0) {
+                        goto fail;
+                    }
+                }
+            }
+            av_dict_free(&dict);
+        }
+    }
+
+    if ((error_code = add_option("dummy-input-filename", &encoder_options)) > 0) {
+        goto fail;
+    }
+
+    if ((error_code = finalise_options(&encoder_options)) > 0) {
+        goto fail;
+    }
+
+    settings.argv = (char const**)encoder_options.argv;
+    settings.argc = encoder_options.options_added;
+
+    for (int i=0; i<settings.argc; ++i) {
+        av_log(avctx, AV_LOG_VERBOSE, "arg %d: %s\n", i, settings.argv[i]);
+    }
+
+    ctx->encoder = turing_create_encoder(settings);
+
+    if (!ctx->encoder) {
+        av_log(avctx, AV_LOG_ERROR, "Failed to create libturing encoder.\n");
+        error_code = AVERROR_INVALIDDATA;
+        goto fail;
+    }
+
+    if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
+        turing_bitstream const *bitstream;
+        bitstream = turing_encode_headers(ctx->encoder);
+        if (bitstream->size <= 0) {
+            av_log(avctx, AV_LOG_ERROR, "Failed to encode headers.\n");
+            turing_destroy_encoder(ctx->encoder);
+            error_code = AVERROR_INVALIDDATA;
+            goto fail;
+        }
+
+        avctx->extradata_size = bitstream->size;
+
+        avctx->extradata = av_mallocz(avctx->extradata_size + AV_INPUT_BUFFER_PADDING_SIZE);
+        if (!avctx->extradata) {
+            av_log(avctx, AV_LOG_ERROR, "Failed to allocate HEVC extradata %d bytes\n", avctx->extradata_size);
+            turing_destroy_encoder(ctx->encoder);
+            error_code =  AVERROR(ENOMEM);
+            goto fail;
+        }
+
+        memcpy(avctx->extradata, bitstream->p, bitstream->size);
+    }
+
+    av_freep(&encoder_options.argv);
+    av_freep(&encoder_options.options);
+    return 0;
+
+fail:
+    av_log(avctx, AV_LOG_ERROR, "Error while initialising the Turing codec.\n");
+    av_freep(&encoder_options.argv);
+    av_freep(&encoder_options.options);
+    return error_code;
+}
+
+static int libturing_encode_frame(AVCodecContext *avctx, AVPacket *pkt, 
+const AVFrame *pic, int *got_packet) {
+    libturingEncodeContext *ctx = avctx->priv_data;
+    turing_encoder_output const *output;
+    int ret = 0;
+
+    if (pic) {
+        turing_picture picture;
+
+        picture.image[0].p = pic->data[0];
+        picture.image[1].p = pic->data[1];
+        picture.image[2].p = pic->data[2];
+        picture.image[0].stride = pic->linesize[0];
+        picture.image[1].stride = pic->linesize[1];
+        picture.image[2].stride = pic->linesize[2];
+        picture.pts = pic->pts;
+        output = turing_encode_picture(ctx->encoder, &picture);
+    } else {
+        output = turing_encode_picture(ctx->encoder, 0);
+    }
+
+    if (output->bitstream.size < 0) {
+        return AVERROR_EXTERNAL;
+    }
+
+    if (output->bitstream.size ==0) {
+        return 0;
+    }
+
+    ret = ff_alloc_packet2(avctx, pkt, output->bitstream.size, 0);
+    if (ret < 0) {
+        av_log(avctx, AV_LOG_ERROR, "Error getting output packet.\n");
+        return ret;
+    }
+
+    memcpy(pkt->data, output->bitstream.p, output->bitstream.size);
+
+    pkt->pts = output->pts;
+    pkt->dts = output->dts;
+    if (output->keyframe) {
+        pkt->flags |= AV_PKT_FLAG_KEY;
+    }
+
+    *got_packet = 1;
+    return 0;
+}
+
+static const AVOption options[] = {
+    { "turing-params", "configure additional turing encoder parameters", offsetof(libturingEncodeContext, options), AV_OPT_TYPE_STRING,{ 0 }, 0, 0, AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM },
+    { NULL }
+};
+
+static const AVClass class = {
+    .class_name = "libturing",
+    .item_name  = av_default_item_name,
+    .option     = options,
+    .version    = LIBAVUTIL_VERSION_INT,
+};
+
+AVCodec ff_libturing_encoder = {
+    .name           = "libturing",
+    .long_name      = NULL_IF_CONFIG_SMALL("libturing HEVC"),
+    .type           = AVMEDIA_TYPE_VIDEO,
+    .id             = AV_CODEC_ID_HEVC,
+    .init           = libturing_encode_init,
+    .encode2        = libturing_encode_frame,
+    .close          = libturing_encode_close,
+    .priv_data_size = sizeof(libturingEncodeContext),
+    .priv_class     = &class,
+    .capabilities   = AV_CODEC_CAP_DELAY,
+    .pix_fmts       = (const enum AVPixelFormat[]){AV_PIX_FMT_YUV420P10, AV_PIX_FMT_YUV420P, AV_PIX_FMT_NONE},
+};
--
1.8.5.3