Support H.265/HEVC, VP8, VP9 #279

cyberj0g · 2021-12-09T14:15:24Z

Details:

Added support for H.265 codecs: libx265 (CPU, encoding and decoding), NVENC (GPU, encoding and decoding)
Added support for VP8/VP9 codecs: libvpx (CPU, encoding and decoding), NVENC (GPU, decoding only, where supported)
Added transcoding test for all combinations of codecs and acceleration

Testing:

install dependencies using install_ffmpeg.sh (WIP, please report errors)

run tests:

go test -v --tags=nvidia -timeout 30m -run TestNvidia_Transcoding ./ffmpeg/
go test -v --tags=nvidia -timeout 30m -run TestSW_Transcoding ./ffmpeg/

They will output lines like below, and fail, if anything is not supported (on CI, only CPU test will run for now). SW test should work anywhere, Nvidia test should pass on GPUs starting from desktop GTX 1070 - it can't decode VP9 on my GTX 1070m, despite support matrix stating it should.

    ffmpeg_test.go:754: Transcode VP8 (Accel: 0) -> H.265/HEVC (Accel: 0) Prepare: true Transcode: true Quality: true
    ffmpeg_test.go:754: Transcode VP8 (Accel: 0) -> VP8 (Accel: 0) Prepare: true Transcode: true Quality: true
    ffmpeg_test.go:754: Transcode VP8 (Accel: 0) -> VP9 (Accel: 0) Prepare: true Transcode: true Quality: true

cyberj0g · 2021-12-10T13:18:49Z

Finished testing new 'unified' install_ffmpeg.sh from go-livepeer and added it to circle-ci job. We shouldn't need a separate one for lpms now. For dev/test install, add debug-video to BUILD_TAGS e.g.:

BUILD_TAGS="experimental debug-video" ./install_ffmpeg.sh /projects/livepeer/src/

I checked new codecs tests with this install_ffmpeg.sh, but not sure if all other tests are fine. @JamesWanglf is welcome to add his changes and create PR in go-livepeer.

darkdarkdragon · 2021-12-10T15:02:33Z

Finished testing new 'unified' install_ffmpeg.sh from go-livepeer and added it to circle-ci job. We shouldn't need a separate one for lpms now. For dev/test install, add debug-video to BUILD_TAGS e.g.:
BUILD_TAGS="experimental debug-video" ./install_ffmpeg.sh /projects/livepeer/src/
I checked new codecs tests with this install_ffmpeg.sh, but not sure if all other tests are fine. @JamesWanglf is welcome to add his changes and create PR in go-livepeer.

@leszko What is the reason for including matroska,opus,webm,webm_chunk,webm_dash_manifest into production build?

cyberj0g · 2021-12-10T15:21:38Z

@darkdarkdragon I added these. It makes sense to support MKV and WebM containers for VP8/9, and AAC is not supported for WebM - that's why Opus is added. DASH is a great alternative to HLS (especially for WebM), we might want to support it in the near future too.

darkdarkdragon · 2021-12-10T16:00:15Z

@darkdarkdragon I added these. It makes sense to support MKV and WebM containers for VP8/9, and AAC is not supported for WebM - that's why Opus is added. DASH is a great alternative to HLS (especially for WebM), we might want to support it in the near future too.

Any addition to the production build increases attack surface - so we shouldn't add anything that don't officially support.
So I think we should remove webm_dash_manifest and matroska, and create new issue in go-livepeer repo for adding support for .webm segments.

Also, shouldn't --enable-libopus be added for opus support?

cyberj0g · 2021-12-10T16:19:26Z

Any addition to the production build increases attack surface - so we shouldn't add anything that don't officially support.

That's a good practice, but aren't we adding VP8/9 support at least for decoding with this PR? If so, common containers used with these are MKV and WEBM, and lpms now supports them. Or, since VP8/9 is an exotic choice for HLS+MPEG-TS, we should not enable these codecs at all until other streaming protocols are not implemented on go-livepeer side?

darkdarkdragon · 2021-12-10T17:01:43Z

I think webm would be enough? But it's better to create issue in go-livepeer to discuss what containers should we support.

Or, since VP8/9 is an exotic choice for HLS+MPEG-TS, we should not enable these codecs at all until other streaming protocols are not implemented on go-livepeer side?

We're really not using HLS output of go-livepeer node nowdays - it is being handled by Mist (I think in the near future, when we will integrate Mist and go-livepeer better we'll remove RTMP and HLS support from go-livepeer node)
So it is enough for go-livepeer node to being able to take .webm file and respond back with .mp4 or .ts (or .webm).

cyberj0g · 2021-12-11T08:50:02Z

Created a new issue for additional container formats, and moved these flags to debug build for now.

oscar-davids

Overall LGTM!

As a separate matter, one thing I'm thinking of is to statically link x265 and libvpx like x264. Because if we link it dynamically, we need to set LD_LIBRARY_PATH or add related .so files to the distribution. WDYT?

and I wonder how this part will work on the VP8/VP9/H265.

cyberj0g · 2021-12-16T11:22:47Z

one thing I'm thinking of is to statically link x265 and libvpx like x264

Spent quite a bit of time and got Ffmpeg to build with new codecs as static libraries. However, libx265 is a C++ lib, and I couldn't get CGO to build lpms properly when it statically linked, and don't know if it's a good idea to switch to g++ in lpms. At this point, I believe it's not worth the effort to get it to build statically with libx265. If someone is willing to continue looking into that, my efforts stopped there (set proper PKG_CONFIG_PATH and use linked install_ffmpeg.sh):

CGO_CFLAGS="-Wno-error" CGO_CPPFLAGS="-Wno-error" CC=g++ go build -a cmd/transcoding/transcoding.go
# runtime/cgo
In file included from gcc_context.c:8:
libcgo.h:97:15: error: unnecessary parentheses in declaration of ‘_cgo_get_context_function’ [-Werror=parentheses]
   97 | extern void (*(_cgo_get_context_function(void)))(struct context_arg*);
      |               ^
cc1plus: all warnings being treated as errors

Without CC=g++, I'm getting:

/usr/bin/ld: /projects/livepeer/src/compiled/lib/libx265.a(encoder.cpp.o): undefined reference to symbol '_ZTVN10__cxxabiv121__vmi_class_type_infoE@@CXXABI_1.3'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu//libstdc++.so.6: error adding symbols: DSO missing from command line

cyberj0g · 2021-12-16T13:00:50Z

and I wonder how this part will work on the VP8/VP9/H265.

It doesn't, none of these parameters work for HEVC or VP8/9 to my knowledge, so they are ignored.

iameli · 2021-12-28T19:48:02Z

@cyberj0g Does the node boot without libx265, only loading it dynamically when needed? I don't think we probably want to add an external libx265 dependency if not.

cyberj0g · 2021-12-29T09:26:50Z

@cyberj0g Does the node boot without libx265, only loading it dynamically when needed? I don't think we probably want to add an external libx265 dependency if not.

I think we agreed to not include software versions of new codecs into production build for now, so it won't be an issue. In debug mode, --enable-shared is added, so all dependencies are linked dynamically. As an experiment, I quickly tried building without --enable-shared with all dependencies static, except libx265.so - it didn't work, probably something is missing from Ffmpeg's configure.sh to support mixing static and shared dependencies.

yondonfu · 2022-01-05T01:50:32Z

ffmpeg/encoder.c

@@ -232,7 +232,7 @@ int open_output(struct output_ctx *octx, struct input_ctx *ictx)
    if (octx->fps.den) vc->time_base = av_buffersink_get_time_base(octx->vf.sink_ctx);
    else if (ictx->vc->time_base.num && ictx->vc->time_base.den) vc->time_base = ictx->vc->time_base;
    else vc->time_base = ictx->ic->streams[ictx->vi]->time_base;
-    if (octx->bitrate) vc->rc_min_rate = vc->rc_max_rate = vc->rc_buffer_size = octx->bitrate;
+    if (octx->bitrate) vc->rc_min_rate = vc->bit_rate = vc->rc_max_rate = vc->rc_buffer_size = octx->bitrate;


Were there problems with the bitrate prior to this change?

Yes, IIRC hevc_nvenc didn't work with rc_max_rate set without setting bit_rate. I might be wrong, but for H.264 it was incorrect to not set bit_rate, if requested, either, because otherwise it won't switch to average bitrate mode. And the idea behind setting bit_rate equal to rc_max_rate is to force constant bitrate.

Gotcha. Seems reasonable, but I'm curious if this has resulted in any issues for anyone.

cc @iameli - have we seen issues with bitrate before?

ffmpeg/ffmpeg_test.go

ffmpeg/decoder.c

cyberj0g

Thanks for review @yondonfu, I addressed all comments.

ffmpeg/ffmpeg_test.go

ffmpeg/decoder.c

cyberj0g · 2022-01-05T11:57:56Z

ffmpeg/encoder.c

@@ -232,7 +232,7 @@ int open_output(struct output_ctx *octx, struct input_ctx *ictx)
    if (octx->fps.den) vc->time_base = av_buffersink_get_time_base(octx->vf.sink_ctx);
    else if (ictx->vc->time_base.num && ictx->vc->time_base.den) vc->time_base = ictx->vc->time_base;
    else vc->time_base = ictx->ic->streams[ictx->vi]->time_base;
-    if (octx->bitrate) vc->rc_min_rate = vc->rc_max_rate = vc->rc_buffer_size = octx->bitrate;
+    if (octx->bitrate) vc->rc_min_rate = vc->bit_rate = vc->rc_max_rate = vc->rc_buffer_size = octx->bitrate;


Yes, IIRC hevc_nvenc didn't work with rc_max_rate set without setting bit_rate. I might be wrong, but for H.264 it was incorrect to not set bit_rate, if requested, either, because otherwise it won't switch to average bitrate mode. And the idea behind setting bit_rate equal to rc_max_rate is to force constant bitrate.

ffmpeg/ffmpeg_test.go

yondonfu

LGTM! I'd like to get an approval from another set of eyes that has familiarity with this codebase as well. cc @oscar-davids since you already left an initial review.

I left a minor comment about updating a comment - feel free to address prior to merging. Also, I recommend rebasing off master to pick up a fix that landed recently that should address the failing test in CI and to squash the commits to clean up the commit history before merging.

ffmpeg/ffmpeg_test.go

darkdarkdragon

LGTM

oscar-davids

LGTM!
please rebase on master to fix CI errors before merging.

…red for tests

cyberj0g requested review from jailuthra, oscar-davids and yondonfu December 9, 2021 14:15

livepeer deleted a comment from leszko Dec 10, 2021

livepeer deleted a comment from darkdarkdragon Dec 10, 2021

livepeer deleted a comment from leszko Dec 10, 2021

livepeer deleted a comment from darkdarkdragon Dec 10, 2021

livepeer deleted a comment from leszko Dec 10, 2021

cyberj0g mentioned this pull request Dec 11, 2021

Support additional container formats for VP8/9 livepeer/go-livepeer#2134

Closed

cyberj0g mentioned this pull request Dec 13, 2021

Unified install_ffmpeg.sh for go-livepeer and lpms with HEVC and VP8/9 support livepeer/go-livepeer#2135

Merged

3 tasks

yondonfu requested a review from darkdarkdragon December 13, 2021 14:26

oscar-davids reviewed Dec 15, 2021

View reviewed changes

cyberj0g mentioned this pull request Dec 16, 2021

wip: multiplatform hvec/vp8/vp9 software encoders livepeer/go-livepeer#2136

Closed

This was referenced Dec 16, 2021

Support H265/HEVC, VP8, VP9 livepeer/go-livepeer#2123

Closed

Make the GPU test working after running install_ffmpeg.sh #280

Merged

cyberj0g mentioned this pull request Dec 23, 2021

Transcoder capabilities discovery and validation livepeer/go-livepeer#2150

Merged

3 tasks

yondonfu requested changes Jan 5, 2022

View reviewed changes

yondonfu reviewed Jan 5, 2022

View reviewed changes

ffmpeg/decoder.c Show resolved Hide resolved

cyberj0g commented Jan 5, 2022

View reviewed changes

yondonfu approved these changes Jan 6, 2022

View reviewed changes

ffmpeg/ffmpeg_test.go Show resolved Hide resolved

darkdarkdragon approved these changes Jan 6, 2022

View reviewed changes

oscar-davids approved these changes Jan 6, 2022

View reviewed changes

cyberj0g added 12 commits January 10, 2022 17:40

support other popular codecs

c8a8889

use install_ffmpeg.sh from go-livepeer, refactor tests

d3dff3a

set environment variable to build ffmpeg with tools and modules requi…

a7886ed

…red for tests

Fix NV-SW encoder selection

c8da31f

Change HEVC display name to be alphanumeric

7762b8e

refactor tests

4cd8899

refactor tests

57c5fb5

refactor tests

387c2c6

Fix unsupported codec test comments

b1239e4

Bump to trigger CI

aec6d13

Set env for dynamic libraries

2b7e186

Test cleanup, temporary use install_ffmpeg.sh from go-livepeer branch

c170fd4

cyberj0g force-pushed the ip/hevc branch from 2c171c5 to c170fd4 Compare January 11, 2022 12:46

cyberj0g added 3 commits January 11, 2022 15:14

Create cache key with right install_ffmpeg.sh

906bbed

Bump to trigger CI

44f242a

Switch back to master install_ffmpeg.sh

f75d5e9

cyberj0g merged commit 0eb91f2 into master Jan 11, 2022

oscar-davids mentioned this pull request Jun 17, 2022

remove metadata comparison in the video comparison #336

Merged

cyberj0g mentioned this pull request Apr 3, 2023

Add AV1 Decoding/Encoding Support livepeer/go-livepeer#2780

Open

hjpotter92 deleted the ip/hevc branch June 1, 2023 07:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support H.265/HEVC, VP8, VP9 #279

Support H.265/HEVC, VP8, VP9 #279

cyberj0g commented Dec 9, 2021

cyberj0g commented Dec 10, 2021 •

edited

Loading

darkdarkdragon commented Dec 10, 2021 •

edited

Loading

cyberj0g commented Dec 10, 2021

darkdarkdragon commented Dec 10, 2021

cyberj0g commented Dec 10, 2021

darkdarkdragon commented Dec 10, 2021

cyberj0g commented Dec 11, 2021 •

edited

Loading

oscar-davids left a comment •

edited

Loading

cyberj0g commented Dec 16, 2021

cyberj0g commented Dec 16, 2021 •

edited

Loading

iameli commented Dec 28, 2021

cyberj0g commented Dec 29, 2021

yondonfu Jan 5, 2022

cyberj0g Jan 5, 2022

yondonfu Jan 6, 2022

cyberj0g left a comment

cyberj0g Jan 5, 2022

yondonfu left a comment

darkdarkdragon left a comment

oscar-davids left a comment •

edited

Loading

Support H.265/HEVC, VP8, VP9 #279

Support H.265/HEVC, VP8, VP9 #279

Conversation

cyberj0g commented Dec 9, 2021

cyberj0g commented Dec 10, 2021 • edited Loading

darkdarkdragon commented Dec 10, 2021 • edited Loading

cyberj0g commented Dec 10, 2021

darkdarkdragon commented Dec 10, 2021

cyberj0g commented Dec 10, 2021

darkdarkdragon commented Dec 10, 2021

cyberj0g commented Dec 11, 2021 • edited Loading

oscar-davids left a comment • edited Loading

Choose a reason for hiding this comment

cyberj0g commented Dec 16, 2021

cyberj0g commented Dec 16, 2021 • edited Loading

iameli commented Dec 28, 2021

cyberj0g commented Dec 29, 2021

yondonfu Jan 5, 2022

Choose a reason for hiding this comment

cyberj0g Jan 5, 2022

Choose a reason for hiding this comment

yondonfu Jan 6, 2022

Choose a reason for hiding this comment

cyberj0g left a comment

Choose a reason for hiding this comment

cyberj0g Jan 5, 2022

Choose a reason for hiding this comment

yondonfu left a comment

Choose a reason for hiding this comment

darkdarkdragon left a comment

Choose a reason for hiding this comment

oscar-davids left a comment • edited Loading

Choose a reason for hiding this comment

cyberj0g commented Dec 10, 2021 •

edited

Loading

darkdarkdragon commented Dec 10, 2021 •

edited

Loading

cyberj0g commented Dec 11, 2021 •

edited

Loading

oscar-davids left a comment •

edited

Loading

cyberj0g commented Dec 16, 2021 •

edited

Loading

oscar-davids left a comment •

edited

Loading