Add 8 pixels of padding around each rectangle under VP9.

Add 8 pixels of padding around each rectangle under VP9.

This is required to avoid encoding artefacts.

BUG=448265
Committed: https://crrev.com/47ce03d7a748c85f84fb76e90915a2f3a5a4b981
Cr-Commit-Position: refs/heads/master@{#311583}

[Wez, 2015-01-13 18:14:31]

wez@chromium.org changed reviewers:
+ aconverse@chromium.org, sergeyu@chromium.org

[Wez, 2015-01-13 18:17:08]

PTAL

We can add a test to verify that frames output by the VP9 encoder are padded out
by 8 pixels in all directions, but that doesn't really verify that we've dealt
with the issue of artefacts, which is the real end-goal. Alex, any thoughts on a
minimal test for artefacts?

[Sergey Ulanov, 2015-01-13 19:19:42]

lgtm

https://codereview.chromium.org/851483005/diff/1/remoting/codec/video_encoder...
File remoting/codec/video_encoder_vpx.cc (right):

https://codereview.chromium.org/851483005/diff/1/remoting/codec/video_encoder...
remoting/codec/video_encoder_vpx.cc:385:
aligned_rects.push_back(AlignRect(webrtc::DesktopRect::MakeLTRB(
nit: you can just call updated_region->AddRect() here. There is no need to store
list of aligned rects in aligned_rects.

[Wez, 2015-01-13 19:29:05]

https://codereview.chromium.org/851483005/diff/1/remoting/codec/video_encoder...
File remoting/codec/video_encoder_vpx.cc (right):

https://codereview.chromium.org/851483005/diff/1/remoting/codec/video_encoder...
remoting/codec/video_encoder_vpx.cc:385:
aligned_rects.push_back(AlignRect(webrtc::DesktopRect::MakeLTRB(
On 2015/01/13 19:19:42, Sergey Ulanov wrote:
> nit: you can just call updated_region->AddRect() here. There is no need to
store
> list of aligned rects in aligned_rects.

The idea was to allow AddRects() to do whatever smart optimizations it's capable
of.

In particular how does the current DesktopRegion handle reallocations for
internal structures if you build the region rectangle-by-rectangle?

[Sergey Ulanov, 2015-01-13 19:50:18]

On 2015/01/13 19:29:05, Wez wrote:
>
https://codereview.chromium.org/851483005/diff/1/remoting/codec/video_encoder...
> File remoting/codec/video_encoder_vpx.cc (right):
> 
>
https://codereview.chromium.org/851483005/diff/1/remoting/codec/video_encoder...
> remoting/codec/video_encoder_vpx.cc:385:
> aligned_rects.push_back(AlignRect(webrtc::DesktopRect::MakeLTRB(
> On 2015/01/13 19:19:42, Sergey Ulanov wrote:
> > nit: you can just call updated_region->AddRect() here. There is no need to
> store
> > list of aligned rects in aligned_rects.
> 
> The idea was to allow AddRects() to do whatever smart optimizations it's
capable
> of.
> 
> In particular how does the current DesktopRegion handle reallocations for
> internal structures if you build the region rectangle-by-rectangle?

We currently don't have any optimizations there. AddRects() just calls AddRect()
for each rectangle. And I don't know any useful optimizations we could add
there. It could make sense for SkRegion (where insertion has always linear
complexity), but even there setRects() is not optimized.

[Wez, 2015-01-13 20:06:05]

Stop using AddRects()

[aconverse, 2015-01-15 00:16:19]

lgtm

[Wez, 2015-01-15 00:17:05]

The CQ bit was checked by wez@chromium.org

[commit-bot: I haz the power, 2015-01-15 00:18:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/851483005/20001

[commit-bot: I haz the power, 2015-01-15 00:19:48]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-01-15 00:20:42]

Patchset 2 (id:??) landed as
https://crrev.com/47ce03d7a748c85f84fb76e90915a2f3a5a4b981
Cr-Commit-Position: refs/heads/master@{#311583}