Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement IFrameEncoder / IFrameDecoder using libvorbis #758

Open
gavv opened this issue Jul 13, 2024 · 8 comments
Open

Implement IFrameEncoder / IFrameDecoder using libvorbis #758

gavv opened this issue Jul 13, 2024 · 8 comments
Assignees
Labels
codecs Audio and FEC codecs easy hacks The solution is expected to be straightforward even if you are new to the project enhancement help wanted An important and awaited task but we have no human resources for it yet much-needed This issue is needed most among other help-wanted issues

Comments

@gavv
Copy link
Member

gavv commented Jul 13, 2024

We have two interfaces IFrameEncoder and IFrameDecoder that are used to encode audio samples into packet payload and decode it back. Currently they're implemented using PcmEncoder and PcmDecoder (for uncompressed PCM).

Now we need to add two more implementations: VorbisEncoder and VorbisDecoder, that will use libvorbis library to do the job.

We also need to add new encoder & decoder to the list of tested codecs in test_frame_encoder_decoder.cpp.

@gavv gavv added enhancement help wanted An important and awaited task but we have no human resources for it yet easy hacks The solution is expected to be straightforward even if you are new to the project codecs Audio and FEC codecs labels Jul 13, 2024
@gavv gavv added the much-needed This issue is needed most among other help-wanted issues label Jul 13, 2024
@gavv gavv added this to Roc Toolkit Jul 13, 2024
@github-project-automation github-project-automation bot moved this to Frontlog in Roc Toolkit Jul 13, 2024
@gavv gavv moved this from Frontlog to Help wanted in Roc Toolkit Jul 13, 2024
@runei
Copy link

runei commented Jul 27, 2024

Hi, I am interested in solving this issue.

@gavv
Copy link
Member Author

gavv commented Jul 28, 2024

Welcome, thank you!

@gavv
Copy link
Member Author

gavv commented Jul 28, 2024

Let me know if you'll need help/hints with the build system.

Also, if libogg dependency can be disabled, I suppose we should do it, because all we need is vorbis encoder/decoder, we don't need to work with ogg files.

@runei
Copy link

runei commented Jul 30, 2024

Hi @gavv ,

I tried without the libogg, but it fails when installing vorbis. According to the README, "You'll need libogg (distributed separately) to compile this library.", which is why I tried to install it before libvorbis.

I managed to install vorbis with the build script after installing libogg. But I'm having trouble in installing the libogg with the script, since I used "make install" on the terminal.
Using "execute(ctx, 'make install')" on the script need permission on CI, as it creates the files at " /usr/local/lib".
So how can I install libogg within the same folder and utilize the generated files for the libvorbis installation?

@gavv
Copy link
Member Author

gavv commented Aug 1, 2024

Oh, I see. I assumed that libogg should support using libvorbis as codec, but instead, libvorbis supports using libogg for muxing.

For rfc5215, we'll need to disable ogg muxing, which seems to be feasible. libvorbis encodes packets into a struct from libogg (ogg_packet), however this struct does not yet have OGG headers, only opaque codec-specific data. OGG headers are added later when you use libogg API, and we can just don't use it and use data from ogg_packet directly.


When build-3rdparty.py builds a package, it never runs make install. This script is for building dependencies locally, not installing them system wide. Instead, in the end of building every package, there are calls to install_tree() and install_files() functions that install headers and libraries into a local directory specific for this package.

For example, here is what is "installed" for json-c:

tree -L 2 build/3rdparty/x86_64-pc-linux-gnu/clang-14.0.6-release/json-c-0.12-20140410/
build/3rdparty/x86_64-pc-linux-gnu/clang-14.0.6-release/json-c-0.12-20140410/
├── include
│   ├── doc
│   ├── tests
│   ├── arraylist.h
│   ├── bits.h
│   ├── config.h
│   ├── debug.h
│   ├── json_config.h
│   ├── json_c_version.h
│   ├── json.h
│   ├── json_inttypes.h
│   ├── json_object.h
│   ├── json_object_iterator.h
│   ├── json_object_private.h
│   ├── json_tokener.h
│   ├── json_util.h
│   ├── linkhash.h
│   ├── math_compat.h
│   ├── printbuf.h
│   └── random_seed.h
├── lib
│   └── libjson-c.a
├── src
│   ├── json-c-json-c-0.12-20140410
│   └── json-c-0.12-20140410.tar.gz
├── build.log
└── commit

These include and lib directories are added to the include and library search path in scons, so we can use the dependency in our code.


OK, so what we need to do if we need to build libogg and then pass it as a dependency to libvorbis?

Here is an example of similar situation:

if 'pulseaudio' in autobuild_dependencies:
if not 'pulseaudio' in autobuild_explicit_version and not is_crosscompiling:
pa_ver = env.ParseToolVersion('pulseaudio --version')
if pa_ver:
thirdparty_versions['pulseaudio'] = pa_ver
pa_deps = [
'ltdl',
'json-c',
'sndfile',
]
if 'alsa' in autobuild_dependencies:
pa_deps += ['alsa']
env.BuildThirdParty(thirdparty_versions, 'ltdl')
env.BuildThirdParty(thirdparty_versions, 'json-c')
env.BuildThirdParty(thirdparty_versions, 'pulseaudio',
deps=pa_deps, libs=['pulse', 'pulse-simple'])

Here, ltdl and json-c are dependencies of pulseaudio. We first build dependencies, and then pass them via deps argument to BuildThirdParty(). BuildThirdParty, in turn, passes them to build-3rdparty.py script, where those dependencies are stored in ctx.pkg_deps.

When we call format_flags(), it iterates all those dependencies (if they're present) and add their include and lib directories to the flags.

Summary:

  • in 3rdparty/SConscript, if we build libvorbis, we should also build libogg, and pass it to libvorbis using deps arg
  • in build-3rdparty.py, we should support both libogg and libvorbis and properly use install_tree/install_files for both of them
  • in build-3rdparty.py, make sure that you use format_flags() - then it will automatically add libogg dependency to libvorbis

@gavv
Copy link
Member Author

gavv commented Aug 1, 2024

Feel free to ping me if you'll have troubles with those scripts, I know it's not the funniest part :)

@runei
Copy link

runei commented Aug 20, 2024

Hi @gavv , I am working on the handling of Vorbis stream headers. When encoding with vorbis, we should have 3 headers: the identification, comments and codebook headers. And have a few doubts on how to deal with this:

Should we have a dedicated frame for headers or should we split the header across multiple frames together with the data? Or do you have another idea?

My idea would be to create a function uint8_t* create_header_frame(); to handle the header separately, and execute this before the begin_frame

How should we handle cases where the header size exceeds the frame size? Could split the header through multiple frame cause any problems?

Does the encoded_byte_count function need to include the size of the header, or should it only count the encoded data?

I would need to adjust all the unit tests, as well as the packetizer uses the IFrameEncoder. Does this affect any other part of the code?

@gavv
Copy link
Member Author

gavv commented Sep 15, 2024

Hi, sorry for delay! I was busy relocating to another country. Here are my thoughts, after reading your comments and checking out RFC and libvorbis.


Ogg stream

First of all, in your current PR you're using ogg stream and serializing ogg pages. As mentioned above, we shouldn't do it, as specified in rfc5215. We must only send vorbis packets, and don't add ogg encapsulation.

In my understanding, we'll need to use ogg_packet struct, but not ogg_stream and ogg_page and not any of the ogg_*() functions. Just send bytes from ogg_packet.


Configuration headers

Regarding the 3 headers, rfc5215 states the following:

In order to set up an initial state for the client application, the configuration MUST be conveyed via the signalling channel used to set up the session. One example of such signalling is SDP [RFC4566] with the Offer/Answer Model [RFC3264]. Changes to the configuration MAY be communicated via a re-invite, conveying a new SDP, or sent in-band in the RTP channel. Implementations MUST support an in-band delivery of updated codebooks, and SHOULD support out-of-band codebook update using a new SDP file. The changes may be due to different codebooks as well as different bitrates of the RTP stream.

In other words there are 3 strict requirements:

  • sender must serialize configuration headers to SDP
  • receiver must deserialize configuration headers from SDP
  • receiver must handle in-band configuration headers updates

So I see the algorithm like this:

Sender:

1. Creates encoder
2. Obtains configuration headers (3 ogg_packet structs) from encoder
3. Serialize them according to section 3.2.1
4. Add serialized configuration to SDP
5. Passes samples to encoder, gets packets, and sends them

Receiver:

1. Creates decoder
2. Gets seriailized configuration from SDP
3. Deserializes it into 3 ogg_packet structs
4. Passes headers to decoder
5. Receives packets, passes to decoder, and extracts samples

In addition, if received packet is in-band configuration header update (section 3.1.1), we should parse it and apply configuration to encoder. I'm not 100% sure how to do it, but likely vorbis_synthesis_idheader() and vorbis_synthesis_restart() may help here.

Note: our own sender won't actually produce in-band configuration header update for now (we don't have a use case for it yet), but to conform RFC and be compatible with non-roc senders, we still should be able to handle them.

The approach above is very close to your idea with get_headers_frame()/initialize_headers(), but instead of byte buffers, we need to work with SDP (since we need codec-agnostic solution).

We can pass sdp::SessionDescription to IFrameEncoder, so that it can add its codec-specific configuration to the SDP. Then we can pass sdp::SessionDescription to IFrameDecoder, and it can read configuration from it. Same as with your approach, this should be done before encoding/decoding first packet.

Note: currently SDP is (partially) implemented in roc_sdp, but not actually used. In the release when we'll enable Vorbis, we'll also add CLI and API options to get/set SDP offer/answer. In later releases we'll add automatic SDP exchange via SAP/SDP and RTSP.


Libvorbis flow

As far as I can tell from docs & sources, libvorbis works this way:

  • You write continuous stream to encoder.
  • It splits stream into fixed-size blocks (chunks of N samples).
  • Each block is usually encoded into 1 packet.
  • Though, sometimes, a block may produce multiple packets.
  • You can't control block size (?).
  • You can't encoded packet size (?).
  • You can query block size.
  • You can't query encoded packet size before encoding (?).
  • Encoded packets are variable-size.

This design places us before a few challenges which I missed before. (BTW changes that I describe below will be useful not just for vorbis, but for many other codecs like Opus).

Please let me know if I'm missing something. Bellow I assume that this is true.


Variable packet size

encoded_byte_count() was intended to return exact size of encoded payload (i.e. everything after RTP header, including Vorbis headers and compressed audio). It works fine for PCM, but apparently it doesn't fit codecs like Vorbis with variable-size packets. I guess we should rework encoder and composer interfaces. With libvorbis, we can't know packet size before we actually encode it.

Currently the algorithm is:

  1. Call IFrameEncoder::encoded_byte_count() to get payload byte size for given sample count.
  2. Call IComposer::prepare() with payload size.
  3. Encode payload into prepared packet's buffer
  4. Probably call IComposer::pad() (see below).
  5. Later, in the end of pipeline, call ICompose::compose().

We could change it to something like this:

  1. NEW: Call IFrameEncoder::max_payload_size() to get maximum possible payload byte size for given sample count.
  2. Call IComposer::prepare() with maximum payload size.
  3. Encode payload into prepared packet's buffer
  4. NEW: Ask encoder how many bytes were actually written.
  5. NEW: Call IComposer::shrink() to shrink packet's buffer from maximum payload size to actual payload size.
  6. Probably call IComposer::pad() (see below).
  7. Later, in the end of pipeline, call IComposer::compose().

To make it work, we'll need a few changes:

  • IFrameEncoder: replace encoded_byte_count() with max_payload_size(); for PCM it'll be the same, for Vorbis it will return some estimated upper bound, e.g. vorbis_info_blocksize() * n_chans * some_coeff.
  • IFrameEncoder: end_frame() should return actual number of bytes encoded.
  • IComposer: add shrink() method that sets buffer size to given smaller value.

I suppose we'll also need similar changes in IFrameDecoder.


Multiple packets per frame

Since libvorbis is allowed to produce multiple packets for one block, we should reflect it in IFrameEncoder interface as well.

Current approach is:

  1. Pass packet's buffer to encoder.
  2. Write samples_per_packet to encoder.
  3. Tell encoder to finish packet.

We could use the following approach:

  1. Pass packet's buffer to encoder.
  2. Write samples to encoder until we write samples_per_packet samples, OR encoder reports that no more samples can be written.
  3. Tell encoder to finish packet and ask it how much bytes were actually written to packet.
  4. Ask encoder if there are any more buffered packets. If yes, create new packet, pass it's buffer to encoder, and go to step (3).

Note: the reason why we pass packet's buffer to encoder as the first step is to allow simple buffer-less encoders like PCM to use packet's buffer directly without doing extra copy.


Padding packets

Currently audio::Packetizer produces packets which all have exactly same size (defines by encoded_byte_count(samples_per_packet)). If a packet is going to be shorter than this value because of flush() call, packetizer pads it to have needed size.

This is done so because FECFRAME (the protocol which we use for loss repair) requires that all packets within FEC blocks should have same size. This requirements conflicts with vorbis (and similar codecs) where we don't control packet size.

Unfortunately I don't see a clean solution here, so I suggest the following work around:

  1. In Packetizer, continuously compute moving maximum of the packet size, i.e. maximum size of encoded packet for e.g. last 10'000 packets. (See roc_stat module, it has classes for that).
  2. To make this value more stable, round it e.g. to nearest multiple of 64 bytes.
  3. Pad every packet to have this size.
  4. If it happens that moving maximum, and hence packet size, changes in the middle of FEC block, receiver will disable loss repair until the next FEC block. This adjustment can be done later outside of this PR.

We'll need to test this approach to see how stable the packet size would be in practice and how often would it cause receiver to skip loss repair. At the first glance it seems to be a rare event.

Also let me know if you have better ideas!


Affected parts of code

IFrameEncoder is used only from Packetizer, and IFrameDecoder from Depacketizer. IFrameEncoder and IFrameDecoder currently are implemented only for PCM.

IComposer is used more widely, and have a few implementations (rtp::Composer, fec::Composer, rtcp::Composer). Though, changes in its interface and implementations are much smaller.

See also this page: https://roc-streaming.org/toolkit/docs/internals/packets_frames.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codecs Audio and FEC codecs easy hacks The solution is expected to be straightforward even if you are new to the project enhancement help wanted An important and awaited task but we have no human resources for it yet much-needed This issue is needed most among other help-wanted issues
Projects
Status: Help wanted
Development

No branches or pull requests

2 participants