Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Update libvpx to v-0.9.7-p1

RESOLVED FIXED in mozilla10

Status

()

Core
Audio/Video
RESOLVED FIXED
6 years ago
a year ago

People

(Reporter: derf, Assigned: derf)

Tracking

(Depends on: 1 bug)

Trunk
mozilla10
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 4 obsolete attachments)

(Assignee)

Description

6 years ago
This release allows us to drop a number of our in-tree patches and provides some modest decoder speed-ups. It also adds error-concealment support, which will be required for WebRTC.

I'm including patches adding both the encoder and error concealment, but keeping them disabled at compile time. This code has not been through security review, but having it in-tree will be helpful for WebRTC development, and I'd like to get the build system changes they require reviewed now, since configure patches rot faster than a bruised pumpkin in Texas.
(Assignee)

Comment 1

6 years ago
Created attachment 565733 [details] [diff] [review]
Upgrade libvpx decoder to v0.9.7-p1
Attachment #565733 - Flags: review?(khuey)
Attachment #565733 - Flags: review?(chris)
(Assignee)

Comment 2

6 years ago
Created attachment 565734 [details] [diff] [review]
Add libvpx's error concealment support (disabled)
Attachment #565734 - Flags: review?(khuey)
Attachment #565734 - Flags: review?(chris)
(Assignee)

Comment 3

6 years ago
Created attachment 565735 [details] [diff] [review]
Add libvpx's encoder support (disabled)
Attachment #565735 - Flags: review?(khuey)
Attachment #565735 - Flags: review?(chris)
(Assignee)

Comment 4

6 years ago
I ran versions of these patches with error concealment and encoder support hard-coded on, instead of off through try, and they came out greenish:

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=e39db7884209

I haven't run the versions with those features disabled through try.
Comment on attachment 565733 [details] [diff] [review]
Upgrade libvpx decoder to v0.9.7-p1

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

I'm assuming configure.in and the Makefile.in is all you wanted me to look at.  They look fine.

::: configure.in
@@ +5359,1 @@
>                       ([--with-system-libvpx requested but symbol vpx_codec_dec_init_ver not found]))

Shouldn't this be an AC_MSG_ERROR?

@@ +5362,5 @@
> +            dnl We need at least v0.9.7 to fix several crash bugs (for which we
> +            dnl had local patches prior to v0.9.7).
> +            dnl
> +            dnl This is a terrible test for the library version, but we don't
> +            dnl have a good one. There is no version number in a public header,

lame

@@ +5364,5 @@
> +            dnl
> +            dnl This is a terrible test for the library version, but we don't
> +            dnl have a good one. There is no version number in a public header,
> +            dnl and testing the headers still doesn't guarantee we link against
> +            dnl the right version. While we could call vpx_codec_version() at

double lame
Attachment #565733 - Flags: review?(khuey) → review+
Comment on attachment 565734 [details] [diff] [review]
Add libvpx's error concealment support (disabled)

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

::: configure.in
@@ +4316,5 @@
>  MOZ_TREMOR=
>  MOZ_WAVE=1
>  MOZ_MEDIA=
>  MOZ_WEBM=1
> +MOZ_VP8_ERROR_CONCEALMENT=0

You don't want the 0 here, I think.

@@ +5338,5 @@
>  MOZ_LIBVPX_LIBS=
>  
>  if test -n "$MOZ_WEBM"; then
>      AC_DEFINE(MOZ_WEBM)
> +    if test -n "$MOZ_VP8_ERROR_CONCEALMENT" ; then

Cause that will trip this test, no?
(Assignee)

Comment 7

6 years ago
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #6)
> > +MOZ_VP8_ERROR_CONCEALMENT=0
> 
> You don't want the 0 here, I think.

Yes, dur. Let me fix that.
Comment on attachment 565735 [details] [diff] [review]
Add libvpx's encoder support (disabled)

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

::: configure.in
@@ +4317,5 @@
>  MOZ_WAVE=1
>  MOZ_MEDIA=
>  MOZ_WEBM=1
>  MOZ_VP8_ERROR_CONCEALMENT=0
> +MOZ_VP8_ENCODER=0

Again, the same issue with the 0.

@@ +5342,5 @@
>      AC_DEFINE(MOZ_WEBM)
>      if test -n "$MOZ_VP8_ERROR_CONCEALMENT" ; then
>          AC_DEFINE(MOZ_VP8_ERROR_CONCEALMENT)
>      fi
> +    if test -n "$MOZ_VP8_ENCODER" ; then

And here.
(Assignee)

Comment 9

6 years ago
Created attachment 565979 [details] [diff] [review]
Add libvpx's error concealment support (disabled)
Attachment #565734 - Attachment is obsolete: true
Attachment #565734 - Flags: review?(khuey)
Attachment #565734 - Flags: review?(chris)
Attachment #565979 - Flags: review?(khuey)
Attachment #565979 - Flags: review?(chris)
(Assignee)

Comment 10

6 years ago
Created attachment 565981 [details] [diff] [review]
Add libvpx's encoder support (disabled)
Attachment #565735 - Attachment is obsolete: true
Attachment #565735 - Flags: review?(khuey)
Attachment #565735 - Flags: review?(chris)
Attachment #565981 - Flags: review?(khuey)
Attachment #565981 - Flags: review?(chris)
(Assignee)

Updated

6 years ago
Attachment #565981 - Attachment is patch: true
Attachment #565979 - Flags: review?(khuey) → review+
Attachment #565981 - Flags: review?(khuey) → review+
Comment on attachment 565733 [details] [diff] [review]
Upgrade libvpx decoder to v0.9.7-p1

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

::: media/libvpx/update.sh
@@ +314,5 @@
>  # Patch to compile with Sun Studio on Solaris
>  patch -p3 < solaris.patch
>  
> +# Patch to fix errors including C headers in C++
> +patch -p3 < compile_errors.patch

Pretty lame that we need this. :(
Attachment #565733 - Flags: review?(chris) → review+
Attachment #565979 - Flags: review?(chris) → review+
Attachment #565981 - Flags: review?(chris) → review+
(Assignee)

Comment 12

6 years ago
Created attachment 566984 [details] [diff] [review]
Add libvpx's error concealment support (disabled) v3

So, proving once again that anything untested is broken, the stuff I had in vpx_config.h to override the libvpx configuration flags with our own flags was broken when the features were actually disabled. Uploading new versions of these patches that fix this problem.

Greenish on try: https://tbpl.mozilla.org/?tree=Try&rev=695b04558e7f (except for OS X64, which appears to have two sets of test results for some reason I can't fathom, but I'm pretty sure that's tbpl being broken, not the patches).

Carrying forward r=cpearce,khuey.
Attachment #565979 - Attachment is obsolete: true
Attachment #566984 - Flags: review+
(Assignee)

Comment 13

6 years ago
Created attachment 566985 [details] [diff] [review]
Add libvpx's encoder support (disabled) v3
Attachment #565981 - Attachment is obsolete: true
Attachment #566985 - Flags: review+
(Assignee)

Comment 14

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/c7542ce9069a
Assignee: nobody → tterribe
https://hg.mozilla.org/mozilla-central/rev/64603f6ddda1
https://hg.mozilla.org/mozilla-central/rev/f5eded31718a
https://hg.mozilla.org/mozilla-central/rev/c7542ce9069a
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
(Assignee)

Updated

6 years ago
Blocks: 665909

Updated

6 years ago
Depends on: 696390

Updated

6 years ago
Depends on: 696622
(Assignee)

Updated

6 years ago
Blocks: 688178
Blocks: 763495
Depends on: 608066
You need to log in before you can comment on or make changes to this bug.