libwebp: update snapshot to v0.2.0-rc1

libwebp: update snapshot to v0.2.0-rc1

adds lossless encoder/decoder with alpha support. the same is to used to
add alpha+VP8 support.

BUG=139925
BUG=133161
TEST=webkit layout tests


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=150264

[jzern, 2012-08-04 22:53:00]

Sorry for the review abuse Andrew!

A couple of things that need to get worked through:

1) linking of android builds (android_getCpuFeatures / android_getCpuFamily)
These are part of sources/android/cpufeatures/cpu-features.c in the NDK. Is
there a way to add this dependency ala LOCAL_STATIC_LIBRARIES in Android.mk?
Pulling the source directly from the android_ndk_root seems like it would cause
issues with other libs.

2) webkit layout tests: */fast/canvas/canvas-toDataURL-webp.html
The output from these is fine, just the decoded output difference is above
tolerances. What's the normal path for something like this? Submit here then
file a bug on webkit? I imagine the test will need to be made flexible enough
to support both versions of libwebp.

[jzern, 2012-08-04 22:55:48]

On 2012/08/04 22:53:00, jzern wrote:
> Sorry for the review abuse Andrew!
> 
> A couple of things that need to get worked through:
> 
> 1) linking of android builds (android_getCpuFeatures / android_getCpuFamily)
> These are part of sources/android/cpufeatures/cpu-features.c in the NDK. Is
> there a way to add this dependency ala LOCAL_STATIC_LIBRARIES in Android.mk?
> Pulling the source directly from the android_ndk_root seems like it would
cause
> issues with other libs.
> 
This one isn't much of a blocker, I can drop the autodetect and use the dummy
check based on __ARM_NEON__. Just curious if there's a better way. I didn't see
anything obvious with libvpx.

> 2) webkit layout tests: */fast/canvas/canvas-toDataURL-webp.html
> The output from these is fine, just the decoded output difference is above
> tolerances. What's the normal path for something like this? Submit here then
> file a bug on webkit? I imagine the test will need to be made flexible enough
> to support both versions of libwebp.

[jzern, 2012-08-05 20:28:47]

On 2012/08/04 22:55:48, jzern wrote:
> On 2012/08/04 22:53:00, jzern wrote:
> > Sorry for the review abuse Andrew!
> > 
> > A couple of things that need to get worked through:
> > 
> > 1) linking of android builds (android_getCpuFeatures / android_getCpuFamily)
> > These are part of sources/android/cpufeatures/cpu-features.c in the NDK. Is
> > there a way to add this dependency ala LOCAL_STATIC_LIBRARIES in Android.mk?
> > Pulling the source directly from the android_ndk_root seems like it would
> cause
> > issues with other libs.
> > 
> This one isn't much of a blocker, I can drop the autodetect and use the dummy
> check based on __ARM_NEON__. Just curious if there's a better way. I didn't
see
> anything obvious with libvpx.
> 
> > 2) webkit layout tests: */fast/canvas/canvas-toDataURL-webp.html
> > The output from these is fine, just the decoded output difference is above
> > tolerances. What's the normal path for something like this? Submit here then
> > file a bug on webkit? I imagine the test will need to be made flexible
enough
> > to support both versions of libwebp.

Another short term option would be to revert the VP8 encoder changes that cause
the difference, I have a local branch which I think accomplishes this.

[jzern, 2012-08-06 18:59:08]

Take a look, I made some modifications for the encode layout tests and android
builds fine now.

On 2012/08/05 20:28:47, jzern wrote:
> On 2012/08/04 22:55:48, jzern wrote:
> > On 2012/08/04 22:53:00, jzern wrote:
> > > Sorry for the review abuse Andrew!
> > > 
> > > A couple of things that need to get worked through:
> > > 
> > > 1) linking of android builds (android_getCpuFeatures /
android_getCpuFamily)
> > > These are part of sources/android/cpufeatures/cpu-features.c in the NDK.
Is
> > > there a way to add this dependency ala LOCAL_STATIC_LIBRARIES in
Android.mk?
> > > Pulling the source directly from the android_ndk_root seems like it would
> > cause
> > > issues with other libs.
> > > 
> > This one isn't much of a blocker, I can drop the autodetect and use the
dummy
> > check based on __ARM_NEON__. Just curious if there's a better way. I didn't
> see
> > anything obvious with libvpx.
> > 

Imported cpu-features.[hc].

> > > 2) webkit layout tests: */fast/canvas/canvas-toDataURL-webp.html
> > > The output from these is fine, just the decoded output difference is above
> > > tolerances. What's the normal path for something like this? Submit here
then
> > > file a bug on webkit? I imagine the test will need to be made flexible
> enough
> > > to support both versions of libwebp.
> 
> Another short term option would be to revert the VP8 encoder changes that
cause
> the difference, I have a local branch which I think accomplishes this.

Went with this change.

[Ami GONE FROM CHROMIUM, 2012-08-06 21:01:05]

LGTM

http://codereview.chromium.org/10832153/diff/15001/third_party/libwebp/README...
File third_party/libwebp/README.chromium (right):

http://codereview.chromium.org/10832153/diff/15001/third_party/libwebp/README...
third_party/libwebp/README.chromium:24: * Revert VP8 encoder changes to avoid
WebKit layout test conflicts
Open a bug to track removing these reverts and rebaselining expectations?

[Ami GONE FROM CHROMIUM, 2012-08-06 21:04:57]

On 2012/08/04 22:53:00, jzern wrote:
> 2) webkit layout tests: */fast/canvas/canvas-toDataURL-webp.html
> The output from these is fine, just the decoded output difference is above
> tolerances. What's the normal path for something like this? Submit here then
> file a bug on webkit? I imagine the test will need to be made flexible enough
> to support both versions of libwebp.

Typical path is to mark the test as failing in
third_party/WebKit/LayoutTests/platform/chromium/TestExpectations, roll chromium
forward, wait for that to show up in WebKit DEPS, wait for new results to show
up on the layouttest bots, and rebaseline webkit expectations, removing the
TestExpectations added as the first step.  Should take 1 or 2 days of wall time
for all this to transpire.

[Ami GONE FROM CHROMIUM, 2012-08-06 21:05:35]

On 2012/08/04 22:53:00, jzern wrote:
> I imagine the test will need to be made flexible enough
> to support both versions of libwebp.

Oh, and this isn't necessary usually.

[jzern, 2012-08-06 22:13:16]

On 2012/08/06 21:01:05, Ami Fischman wrote:
> LGTM
> 
>
http://codereview.chromium.org/10832153/diff/15001/third_party/libwebp/README...
> File third_party/libwebp/README.chromium (right):
> 
>
http://codereview.chromium.org/10832153/diff/15001/third_party/libwebp/README...
> third_party/libwebp/README.chromium:24: * Revert VP8 encoder changes to avoid
> WebKit layout test conflicts
> Open a bug to track removing these reverts and rebaselining expectations?

Sounds good. Just so I have the order correct, the thinking is:
1) submit as is so we can make the m22 cut (which I think is still possbile)
2) remove the reverts to coincide with this WebKit bug to be opened
3+) ...the remainder of your DEPS description...

[Ami GONE FROM CHROMIUM, 2012-08-06 22:16:40]

On 2012/08/06 22:13:16, jzern wrote:
> On 2012/08/06 21:01:05, Ami Fischman wrote:
> > LGTM
> > 
> >
>
http://codereview.chromium.org/10832153/diff/15001/third_party/libwebp/README...
> > File third_party/libwebp/README.chromium (right):
> > 
> >
>
http://codereview.chromium.org/10832153/diff/15001/third_party/libwebp/README...
> > third_party/libwebp/README.chromium:24: * Revert VP8 encoder changes to
avoid
> > WebKit layout test conflicts
> > Open a bug to track removing these reverts and rebaselining expectations?
> 
> Sounds good. Just so I have the order correct, the thinking is:
> 1) submit as is so we can make the m22 cut (which I think is still possbile)
> 2) remove the reverts to coincide with this WebKit bug to be opened
> 3+) ...the remainder of your DEPS description...

Ideally you file the bug *first* and refer to it with a TODO from the README
before landing the CL.  (which shouldn't affect making the branch cut).

[jzern, 2012-08-06 22:29:43]

On 2012/08/06 22:16:40, Ami Fischman wrote:
> On 2012/08/06 22:13:16, jzern wrote:
> > On 2012/08/06 21:01:05, Ami Fischman wrote:
> > > LGTM
> > > 
> > >
> >
>
http://codereview.chromium.org/10832153/diff/15001/third_party/libwebp/README...
> > > File third_party/libwebp/README.chromium (right):
> > > 
> > >
> >
>
http://codereview.chromium.org/10832153/diff/15001/third_party/libwebp/README...
> > > third_party/libwebp/README.chromium:24: * Revert VP8 encoder changes to
> avoid
> > > WebKit layout test conflicts
> > > Open a bug to track removing these reverts and rebaselining expectations?
> > 
> > Sounds good. Just so I have the order correct, the thinking is:
> > 1) submit as is so we can make the m22 cut (which I think is still possbile)
> > 2) remove the reverts to coincide with this WebKit bug to be opened
> > 3+) ...the remainder of your DEPS description...
> 
> Ideally you file the bug *first* and refer to it with a TODO from the README
> before landing the CL.  (which shouldn't affect making the branch cut).

Yes sorry the order on 1/2 were interchangeable I meant more that the reversions
stay for now. I see the layout tests have changed upstream, so I'll put together
a build to make sure the tests still fail. I'll enter the corresponding bug
and/or fixed revision in the README before submitting.

[jzern, 2012-08-07 00:17:01]

On 2012/08/06 22:29:43, jzern wrote:
> On 2012/08/06 22:16:40, Ami Fischman wrote:
> > 
> > Ideally you file the bug *first* and refer to it with a TODO from the README
> > before landing the CL.  (which shouldn't affect making the branch cut).
> 
> Yes sorry the order on 1/2 were interchangeable I meant more that the
reversions
> stay for now. I see the layout tests have changed upstream, so I'll put
together
> a build to make sure the tests still fail. I'll enter the corresponding bug
> and/or fixed revision in the README before submitting.

Added a link to: https://bugs.webkit.org/show_bug.cgi?id=93310
I was actually looking at an out of date WebKit tree, the layout test hadn't
changed and fails as expected.

[Noel Gordon, 2012-08-07 00:20:33]

On 2012/08/06 21:05:35, Ami Fischman wrote:
> On 2012/08/04 22:53:00, jzern wrote:
> > I imagine the test will need to be made flexible enough
> > to support both versions of libwebp.
> 
> Oh, and this isn't necessary usually.


WebKit does not depend on any image decoder version, so I don't see why we'd
depend on a version of libwebp.

The question is, why did the pixel decodings change in a way that is not
backward compatible with prior versions of libwebp?

[jzern, 2012-08-07 00:24:54]

http://codereview.chromium.org/10832153/diff/15001/third_party/libwebp/README...
File third_party/libwebp/README.chromium (right):

http://codereview.chromium.org/10832153/diff/15001/third_party/libwebp/README...
third_party/libwebp/README.chromium:24: * Revert VP8 encoder changes to avoid
WebKit layout test conflicts
On 2012/08/06 21:01:06, Ami Fischman wrote:
> Open a bug to track removing these reverts and rebaselining expectations?

Done.

[jzern, 2012-08-07 00:26:49]

On 2012/08/07 00:20:33, noel chromium wrote:
> On 2012/08/06 21:05:35, Ami Fischman wrote:
> > On 2012/08/04 22:53:00, jzern wrote:
> > > I imagine the test will need to be made flexible enough
> > > to support both versions of libwebp.
> > 
> > Oh, and this isn't necessary usually.
> 
> 
> WebKit does not depend on any image decoder version, so I don't see why we'd
> depend on a version of libwebp.
> 
> The question is, why did the pixel decodings change in a way that is not
> backward compatible with prior versions of libwebp?

Not the decode, but the encode via canvas-toDataURL...

[commit-bot: I haz the power, 2012-08-07 01:29:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jzern@chromium.org/10832153/16001

[commit-bot: I haz the power, 2012-08-07 04:04:47]

Change committed as 150264

[Nico, 2012-08-10 20:01:54]

https://chromiumcodereview.appspot.com/10832153/diff/16001/third_party/libweb...
File third_party/libwebp/libwebp.gyp (right):

https://chromiumcodereview.appspot.com/10832153/diff/16001/third_party/libweb...
third_party/libwebp/libwebp.gyp:62: 'target_name': 'libwebp_dsp_neon',
This causes the following build warning in chrome/mac:


[560/11003] LIBTOOL-STATIC libwebp_dsp_neon.a, POSTBUILDS
libtool: warning for library: libwebp_dsp_neon.a the table of contents is empty
(no object file members in the library define global symbols)


We try very hard to be warning-free, and this is now our only build warning. Can
you please take a look? One possible technique, used by a neon target in webkit,
is to make this target type 'none' on non-neon platforms. Here's how this is
done in third_party/WebKit/Source/WebCore/WebCore.gyp:

    {
      'target_name': 'webcore_arm_neon',
      'conditions': [
        ['target_arch=="arm"', {
          'type': 'static_library',
          'dependencies': [
            'webcore_prerequisites',
          ],
          'hard_dependency': 1,
          'sources': [
            '<@(webcore_files)',
          ],
          'sources/': [
            ['exclude', '.*'],
            ['include', 'platform/graphics/filters/arm/.*NEON\\.(cpp|h)'],
          ],
          'cflags': ['-marm'],
          'conditions': [
            ['OS=="android"', {
              'cflags!': ['-mthumb'],
            }],
          ],
        },{  # target_arch!="arm"
          'type': 'none',
        }],
      ],
    },

[jzern, 2012-08-10 21:02:25]

On 2012/08/10 20:01:54, Nico wrote:
> [...]
> 
> We try very hard to be warning-free, and this is now our only build warning.
Can
> you please take a look? One possible technique, used by a neon target in
webkit,
> is to make this target type 'none' on non-neon platforms. Here's how this is
> done in third_party/WebKit/Source/WebCore/WebCore.gyp:
> 
Thanks for the suggestion, I'll have a look. I opened the following bug to
track:

  https://code.google.com/p/chromium/issues/detail?id=141949