add encoder files to in-tree libvorbis

RESOLVED FIXED in Firefox 28

Status

()

Core
Audio/Video: Recording
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: rillian, Assigned: rillian)

Tracking

(Blocks: 1 bug)

unspecified
mozilla28
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:1.3+, firefox28 fixed)

Details

(Whiteboard: [ft:media-recording][qa-])

Attachments

(1 attachment)

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.
(Assignee)

Comment 1

4 years ago
I'm happy to take this as a follow up, but won't be able to work on it until next week.
Assignee: nobody → giles
Blocks: 883749

Updated

4 years ago
blocking-b2g: --- → 1.3+
Whiteboard: [ft:media-recording]
(Assignee)

Comment 2

4 years ago
Created attachment 830545 [details] [diff] [review]
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.
Attachment #830545 - Flags: review?(monty)
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.
Flags: needinfo?
(Assignee)

Comment 4

4 years ago
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.
Flags: needinfo?
(Assignee)

Updated

4 years ago
Blocks: 937455
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 :-)
Attachment #830545 - Flags: review?(monty) → review+
(Assignee)

Comment 6

4 years ago
Thanks.

I fixed the keyword substitution issues I found upstream.
(Assignee)

Comment 7

4 years ago
(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.
https://hg.mozilla.org/mozilla-central/rev/0084258689ff
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Depends on: 938221

Updated

4 years ago
Whiteboard: [ft:media-recording] → [ft:media-recording][qa-]
status-firefox28: --- → fixed
Component: Video/Audio → Video/Audio: Recording
You need to log in before you can comment on or make changes to this bug.