Closed Bug 1332139 Opened 8 years ago Closed 8 years ago

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

Categories

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

45 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: jesup, Assigned: jesup)

References

Details

Attachments

(3 files, 2 obsolete files)

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
Rank: 17
Attachment #8828188 - Attachment is obsolete: true
Attachment #8828188 - Flags: review?(na-g)
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.
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(!)
Attachment #8828226 - Flags: review?(ted)
Attachment #8828226 - Flags: review?(mh+mozilla)
Attachment #8828190 - Flags: review?(na-g) → review+
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
Attachment #8828226 - Flags: review?(ted)
Attachment #8828226 - Flags: review?(mh+mozilla)
Attachment #8828226 - Flags: review+
Pushed by rjesup@wgate.com:
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
Depends on: 1332664
https://hg.mozilla.org/mozilla-central/rev/004ad2adfd6e
https://hg.mozilla.org/mozilla-central/rev/bf8101665498
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
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
Attachment #8829194 - Attachment is obsolete: true
Attachment #8829194 - Flags: review?(mh+mozilla)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: