931060 - add encoder files to in-tree libvorbis

Status: RESOLVED FIXED

(Core :: Audio/Video: Recording, defect)

Description

[Ralph Giles (:rillian)]

Vorbis decoding is updated to libvorbis 1.3.3 in bug 929910. For Media Encoder we also need to import and build the encoder sources.

This is basically just landing a version of bchen's WIP patch from bug 883749.

Comment 1

[Ralph Giles (:rillian)]

I'm happy to take this as a follow up, but won't be able to work on it until next week.

Comment 2

[Ralph Giles (:rillian)]

Attachment: Add vorbisenc.

Add encoder source to our in-tree copy of libvorbis, add it to the build, and add relevant symbols to the export list.

Comment 3

[Monty Montgomery (:xiphmont)]

Do we want to import the entire encoder wholesale?  There's quite a lot of static data there, and I'm a bit surprised it's an option.

If we'd prefer only a few modes, would it be preferable to patch the encoder source down to something slimmer and then import, or does it make more sense to import first and then remove what we don't want (I'm going to assume the latter).

Also-- if AoTuV merge is something we think might want, we should discuss how/when/where as it will churn most of that static setup data.  Not a blocker or even an immediate worry in any sense, just something to think about ahead of time.

Comment 4

[Ralph Giles (:rillian)]

A stand-alone libvorbisenc.a is about 604 KB. If we can cut this significantly by omitting some modes, that would be great. However, this is blocking the MediaEncoder work, so I think we should land what upstream has now and work on a stripped-down encoder in a separate bug.

We can update the in-tree code again after an AoTuV merge happens upstream.

Comment 5

[Monty Montgomery (:xiphmont)]

Comment on attachment 830545 [details] [diff] [review]
Add vorbisenc.

Review of attachment 830545 [details] [diff] [review]:
-----------------------------------------------------------------

Straight import, verified all files matched upstream versioning for libvorbis 1.3.3 (Omnipresent).

::: layout/media/symbols.def.in
@@ +83,4 @@
>  vorbis_comment_clear
>  vorbis_comment_init
>  vorbis_dsp_clear
> +vorbis_encode_init_vbr

We are using only the high-level VBR setup?

::: media/libvorbis/lib/modes/residue_44p51.h
@@ +10,5 @@
> + *                                                                  *
> + ********************************************************************
> +
> + function: toplevel residue templates for 32/44.1/48kHz uncoupled
> + last mod: $Id$

Oops, fixed upstream (missing SVN flag)

::: media/libvorbis/lib/modes/setup_44p51.h
@@ +10,5 @@
> + *                                                                  *
> + ********************************************************************
> +
> + function: toplevel settings for 44.1/48kHz 5.1 surround modes
> + last mod: $Id$

Oops, will fix this upstream :-)

Comment 6

[Ralph Giles (:rillian)]

Thanks.

I fixed the keyword substitution issues I found upstream.

Comment 7

[Ralph Giles (:rillian)]

(In reply to Monty Montgomery (:xiphmont) from comment #5)

> We are using only the high-level VBR setup?

I tried to include all the symbols in bechen's WIP patch. We can adjust later if we need other entry points.

Comment 8

[Ralph Giles (:rillian)]

https://tbpl.mozilla.org/?tree=Try&rev=95219ec26ca2

Comment 9

[Ralph Giles (:rillian)]

Green try.

https://hg.mozilla.org/integration/mozilla-inbound/rev/0084258689ff

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/0084258689ff