1332139 - Drop ifdefs in webrtc vp9 interface code for handling old versions of libvpx

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect, P1)

Description

[Randell Jesup [:jesup] (needinfo me)]

Since libvpx 1.6.1 has landed, we can drop the ugly ifdefs in the webrtc vp9 interface code which avoided structures not in 1.4

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Remove LIBVPX_SVC hack for vp9 needed to work with libvpx 1.4 from webrtc

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Remove LIBVPX_SVC hack for vp9 needed to work with libvpx 1.4 from webrtc

Also had to undo part of bug 1302935 (https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=1302935&attachment=8791776)

Comment 3

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 8828190 [details] [diff] [review]
Remove LIBVPX_SVC hack for vp9 needed to work with libvpx 1.4 from webrtc

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

This looks largely like a reversal of the patch for Bug 1248335, with the removal of an additional guard around a later VP9 cherry pick. If those assumptions are correct: r+ with nit, pending try.

::: media/webrtc/trunk/webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc
@@ +429,5 @@
>        (num_temporal_layers_ > 1 || num_spatial_layers_ > 1) ? 1 : 0);
>    if (num_temporal_layers_ > 1 || num_spatial_layers_ > 1) {
>      vpx_codec_control(encoder_, VP9E_SET_SVC_PARAMETERS,
>                        &svc_internal_.svc_params);
>    }

nit: some whitespace snuck in after this #endif line on the original patch. You may want to zap it, as it will be diff noise in trunk moving forward.

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Attachment: make system changes to fix libvpx include paths (prefer media/libvpx)

it was using system libvpx includes for webrtc... now uses media/libvpx unless --with-system-libvpx is set.  I tested it with --with-system-libvpx as well, though I can't actually build with that, since mine is 1.3(!)

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 8828226 [details] [diff] [review]
make system changes to fix libvpx include paths (prefer media/libvpx)

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

::: build/gyp.mozbuild
@@ +110,5 @@
>  if CONFIG['MACOS_SDK_DIR']:
>      gyp_vars['mac_sdk_path'] = CONFIG['MACOS_SDK_DIR']
> +
> +if not CONFIG['MOZ_SYSTEM_LIBVPX']:
> +    gyp_vars['libvpx_dir'] =  "/media/libvpx/libvpx"

nit: we use single-quoted strings in moz.build

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/004ad2adfd6e
Remove LIBVPX_SVC hack for vp9 needed to work with libvpx 1.4 from webrtc r=ng
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf8101665498
make system changes to fix libvpx include paths (prefer media/libvpx) r=ted

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/004ad2adfd6e
https://hg.mozilla.org/mozilla-central/rev/bf8101665498

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Backed out changeset 004ad2adfd6e ()

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Test for svc_context.h to control code that depends on it in webrtc

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8829194 [details] [diff] [review]
Test for svc_context.h to control code that depends on it in webrtc

Uploaded to the wrong bug