Delete FrameObject::size member.

Delete FrameObject::size member.

Replaced with a size() method, returning the corresponding attribute
(_length) of the underlying EncodedImage.

BUG=None

Committed: https://crrev.com/37abf53116b693a24518f19ed34fd017355255c9
Cr-Commit-Position: refs/heads/master@{#14809}

[nisse-webrtc, 2016-10-27 11:24:33]

nisse@webrtc.org changed reviewers:
+ philipel@webrtc.org

[nisse-webrtc, 2016-10-27 11:24:34]

https://codereview.webrtc.org/2444193010/diff/1/webrtc/modules/video_coding/f...
File webrtc/modules/video_coding/frame_object.h (right):

https://codereview.webrtc.org/2444193010/diff/1/webrtc/modules/video_coding/f...
webrtc/modules/video_coding/frame_object.h:39: size_t size() { return _length; }
It should be _length here, not _size, right?

[philipel, 2016-10-27 11:27:15]

lgtm, but I'm not OWNER.

https://codereview.webrtc.org/2444193010/diff/1/webrtc/modules/video_coding/f...
File webrtc/modules/video_coding/frame_object.h (right):

https://codereview.webrtc.org/2444193010/diff/1/webrtc/modules/video_coding/f...
webrtc/modules/video_coding/frame_object.h:39: size_t size() { return _length; }
On 2016/10/27 11:24:34, nisse-webrtc wrote:
> It should be _length here, not _size, right?

Thats right!

[nisse-webrtc, 2016-10-27 11:34:24]

nisse@webrtc.org changed reviewers:
+ sprang@webrtc.org

[nisse-webrtc, 2016-10-27 11:34:24]

Erik, PTAL. There's also some background discussion on cl 
https://codereview.webrtc.org/2453143003/

[sprang_webrtc, 2016-10-27 12:40:02]

lgtm

https://codereview.webrtc.org/2444193010/diff/1/webrtc/modules/video_coding/f...
File webrtc/modules/video_coding/frame_object.h (right):

https://codereview.webrtc.org/2444193010/diff/1/webrtc/modules/video_coding/f...
webrtc/modules/video_coding/frame_object.h:39: size_t size() { return _length; }
On 2016/10/27 11:27:15, philipel wrote:
> On 2016/10/27 11:24:34, nisse-webrtc wrote:
> > It should be _length here, not _size, right?
> 
> Thats right!

Ugh.. Any chance you could rename them size_ and capacity_ or something like
that instead?

[nisse-webrtc, 2016-10-27 12:49:53]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2016-10-27 12:50:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[nisse-webrtc, 2016-10-27 12:59:37]

On 2016/10/27 12:40:02, språng wrote:

> Ugh.. Any chance you could rename them size_ and capacity_ or something like
> that instead?

EncodedImage could definitely use some cleanup. I'm not sure how much external
code which
depends on the details, though. It would be good to get some common
understanding of what we're 
aiming for. Philipel intends to eventually replace EncodedImage with the new
class FrameObject 
(not a good name, imho), if I understood him correctly.

I think we should collect *all* info representing an encoded frame and its
metadata as a single
object, so we can use rtc::VideoSinkInterface<EncodedSomething> when plumbing
the pipeline 
together. But it's desirable to not have too much duplicate info on the wire; if
one can derive,
e.g., frame type and list of reference frames, from the bitstream, adding that
info into real packet headers
is some waste of bandwidth. But if it's omitted, we need some clean way to
recover the needed info
on receive.

As for _size vs _length in the current EncodedImage, after a quick look I think
they more or less
always have the same value, so it might be best to simply delete one of them.

I also don't know what the current state of encoders and decoders are with
respect to fragments of
frames, e.g., is there any code that can handle the case
EncodedImage:_completeFrame being false?

[philipel, 2016-10-27 13:15:22]

On 2016/10/27 12:59:37, nisse-webrtc wrote:
> On 2016/10/27 12:40:02, språng wrote:
> 
> > Ugh.. Any chance you could rename them size_ and capacity_ or something like
> > that instead?
> 
> EncodedImage could definitely use some cleanup. I'm not sure how much external
> code which
> depends on the details, though. It would be good to get some common
> understanding of what we're 
> aiming for. Philipel intends to eventually replace EncodedImage with the new
> class FrameObject 
> (not a good name, imho), if I understood him correctly.

Yes, I intend to replace this class with FrameObject, which I agree is a bad
name.

> I think we should collect *all* info representing an encoded frame and its
> metadata as a single
> object, so we can use rtc::VideoSinkInterface<EncodedSomething> when plumbing
> the pipeline 
> together. But it's desirable to not have too much duplicate info on the wire;
if
> one can derive,
> e.g., frame type and list of reference frames, from the bitstream, adding that
> info into real packet headers
> is some waste of bandwidth. But if it's omitted, we need some clean way to
> recover the needed info
> on receive.

I think pthatcher@ had a similar idea, to use FrameObject both on the sender and
receiver side.

> As for _size vs _length in the current EncodedImage, after a quick look I
think
> they more or less
> always have the same value, so it might be best to simply delete one of them.

They are not always the same, in the case of h264 it requires up to 64
additional
bits of buffer space since it use an optimized bitstream reader.

> I also don't know what the current state of encoders and decoders are with
> respect to fragments of
> frames, e.g., is there any code that can handle the case
> EncodedImage:_completeFrame being false?

Not sure about the current state of the decoders, but I can say that the new
jitter buffer is designed to only decode complete frames.

[commit-bot: I haz the power, 2016-10-27 13:23:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-27 13:23:31]

Try jobs failed on following builders:
  win_x64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_dbg/builds/2513)

[nisse-webrtc, 2016-10-27 13:32:40]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2016-10-27 13:32:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-10-27 13:52:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-27 13:52:33]

Try jobs failed on following builders:
  win_x64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_dbg/builds/2520)

[nisse-webrtc, 2016-10-28 07:29:21]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2016-10-28 07:29:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-10-28 07:37:32]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-10-28 07:37:40]

Description was changed from

==========
Delete FrameObject::size member.

Replaced with a size() method, returning the corresponding attribute
(_length) of the underlying EncodedImage.

BUG=None
==========

to

==========
Delete FrameObject::size member.

Replaced with a size() method, returning the corresponding attribute
(_length) of the underlying EncodedImage.

BUG=None

Committed: https://crrev.com/37abf53116b693a24518f19ed34fd017355255c9
Cr-Commit-Position: refs/heads/master@{#14809}
==========

[commit-bot: I haz the power, 2016-10-28 07:37:41]

Patchset 1 (id:??) landed as
https://crrev.com/37abf53116b693a24518f19ed34fd017355255c9
Cr-Commit-Position: refs/heads/master@{#14809}