944977 - Build libvorbis in unified mode

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: unify-libvorbis-libvpx.patch

Move of libvorbis and libvpx could be built in unified mode, but some source files contain duplicate functions. Since these are third-party libraries and I didn't want to change the code in our tree, I just compiled those individual files in non-unified mode.

Comment 1

[Chris Peterson [:cpeterson]]

In this try build, the ambiguously named changeset "Bug NUMBER - Part NUMBER: DESCRIPTION" is this unify-libvorbis-libvpx.patch. <:)

https://tbpl.mozilla.org/?tree=Try&rev=7b53eba8bf2e

Comment 2

[Mike Hommey [:glandium]]

Please don't touch libvpx until bug 763495 lands.

Comment 3

[Chris Peterson [:cpeterson]]

Attachment: unify-libvorbis-v2.patch

Patch v2: Build media/libvorbis in unified mode.

As per glandium's comment 2, I removed libvpx from this patch so it only builds libvorbis in unified mode.

Comment 4

[cajbir (:cajbir)]

Comment on attachment 8340791 [details] [diff] [review]
unify-libvorbis-v2.patch

I think the maintainers of the libraries probably know more than me about what traps are involved in making these unified so they should probably review. Maybe Tim Terriberry or Ralph Giles.

Comment 5

[Timothy B. Terriberry (:derf)]

Comment on attachment 8340791 [details] [diff] [review]
unify-libvorbis-v2.patch

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

r=me with a few nits.

::: media/libvorbis/moz.build
@@ +29,5 @@
> +]
> +
> +# These files can't be unified because of function redefinitions.
> +SOURCES += [
> +    'lib/vorbis_codebook.c',

You should kick out vorbis_sharedbook.c instead, since that only contains setup/teardown code, while vorbis_codebook.c contains functions that are called many times per packet, and should be optimized together with the bulk of the codec.

@@ +37,1 @@
>      'lib/vorbis_window.c',

There's no actual conflict here, AFAICT. The problem is that the function in the .c file does not match that in the .h file.

I fixed this upstream in r19028. It's okay if you don't want to backport that fix, but you should leave a comment, so we'll remember to move this file back the next time we pull an update of libvorbis.

Comment 6

[Chris Peterson [:cpeterson]]

Attachment: part-1-backport-libvorbis-r19028.patch

I updated media/libvorbis/update.sh to apply a patch for libvorvis r19028.

btw, upstream r19028 will introduce some clang -Wincompatible-pointer-types-discards-qualifiers warnings. My bug944977-r19028.patch file also fixes those warnings locally.


media/libvorbis/lib/vorbis_block.c:774:18: warning: initializing 'float *' with an expression of type 'const float *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
Making all in include
          float *w=_vorbis_window_get(b->window[1]-hs);
                 ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

media/libvorbis/lib/vorbis_block.c:781:18: warning: initializing 'float *' with an expression of type 'const float *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
          float *w=_vorbis_window_get(b->window[0]-hs);
                 ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

media/libvorbis/lib/vorbis_block.c:790:18: warning: initializing 'float *' with an expression of type 'const float *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
          float *w=_vorbis_window_get(b->window[0]-hs);
                 ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

media/libvorbis/lib/vorbis_block.c:799:18: warning: initializing 'float *' with an expression of type 'const float *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
          float *w=_vorbis_window_get(b->window[0]-hs);
                 ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

media/libvorbis/lib/vorbis_block.c:1045:10: warning: returning 'const float *' from a function with result type 'float *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
  return _vorbis_window_get(b->window[W]-hs);
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Comment 7

[Timothy B. Terriberry (:derf)]

Comment on attachment 8341504 [details] [diff] [review]
part-1-backport-libvorbis-r19028.patch

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

Good call. I fixed these (and various other warnings) upstream in r19031.

Comment 8

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/727a945e6ddc
https://hg.mozilla.org/integration/mozilla-inbound/rev/bbdfac4aa444

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/727a945e6ddc
https://hg.mozilla.org/mozilla-central/rev/bbdfac4aa444