FrameObject inherit from VCMEncodedFrame.

FrameObject inherit from VCMEncodedFrame.

Let the FrameObject class inherit from VCMEncodedFrame since the rest of the
decoding pipeline use VCMEncodedFrame.

BUG=webrtc:5514
R=stefan@webrtc.org

Committed: https://crrev.com/552866c4020adb65a0de569c4f0ed2c6960314b4
Cr-Commit-Position: refs/heads/master@{#13380}

[philipel, 2016-06-29 14:36:33]

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

[stefan-webrtc, 2016-06-29 16:43:24]

I have a different proposal to avoid the inheritance. Please consider if it's
feasible.

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

https://codereview.webrtc.org/2110543005/diff/1/webrtc/modules/video_coding/f...
webrtc/modules/video_coding/frame_object.cc:51: timestamp = packet->timestamp;
Could you add a comment before each block of assignments mentioning what
subclass the member assigned to belongs to, just to make it easier to read given
the multiple inheritances?

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

https://codereview.webrtc.org/2110543005/diff/1/webrtc/modules/video_coding/f...
webrtc/modules/video_coding/frame_object.h:21: class FrameObject : public
webrtc::VCMEncodedFrame {
What do you think of the option of adding a constructor to VCMEncodedFrame
instead which takes a FrameObject calls GetBitstream() on it to get a copy of
the bitstream, and initializes its members based on the public members of the
FrameObject? That way we don't need this inheritance.

Some of the things needed to fully initialize the VCMEncodedFrame isn't
available in the FrameObject, but either they may not be needed at all after the
jitter buffer, or they probably have to be added to FrameObject later anyway to
replace VCMEnocdedFrame, right?

[philipel, 2016-06-30 09:34:15]

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

https://codereview.webrtc.org/2110543005/diff/1/webrtc/modules/video_coding/f...
webrtc/modules/video_coding/frame_object.cc:51: timestamp = packet->timestamp;
On 2016/06/29 16:43:23, stefan-webrtc (holmer) wrote:
> Could you add a comment before each block of assignments mentioning what
> subclass the member assigned to belongs to, just to make it easier to read
given
> the multiple inheritances?

Done.

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

https://codereview.webrtc.org/2110543005/diff/1/webrtc/modules/video_coding/f...
webrtc/modules/video_coding/frame_object.h:21: class FrameObject : public
webrtc::VCMEncodedFrame {
On 2016/06/29 16:43:24, stefan-webrtc (holmer) wrote:
> What do you think of the option of adding a constructor to VCMEncodedFrame
> instead which takes a FrameObject calls GetBitstream() on it to get a copy of
> the bitstream, and initializes its members based on the public members of the
> FrameObject? That way we don't need this inheritance.

Sounds reasonable but I don't think it makes much of a difference in regards to
the long-term goal (replacing VCMEncodedFrame with FrameObject). I think both
will be equally complex and require the same amount of work.
 
> Some of the things needed to fully initialize the VCMEncodedFrame isn't
> available in the FrameObject, but either they may not be needed at all after
the
> jitter buffer, or they probably have to be added to FrameObject later anyway
to
> replace VCMEnocdedFrame, right?

That's right, we don't need everything to be initialized after the jitter buffer
and things that are in the VCMEncodedFrame will have to be moved to the
FrameObject class as the VCMEncodedFrame is gradually remvoed.

[philipel, 2016-07-04 11:43:14]

ping

[stefan-webrtc, 2016-07-04 11:50:37]

lgtm

[philipel, 2016-07-04 12:08:33]

The CQ bit was checked by philipel@webrtc.org

[commit-bot: I haz the power, 2016-07-04 12:08: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-07-04 14:09:22]

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

[commit-bot: I haz the power, 2016-07-04 14:09:23]

Try jobs failed on following builders:
  android_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[philipel, 2016-07-04 14:19:15]

Description was changed from

==========
FrameObject inherit from VCMEncodedFrame.

Let the FrameObject class inherit from VCMEncodedFrame since the rest of the
decoding pipeline use VCMEncodedFrame.

BUG=webrtc:5514
==========

to

==========
FrameObject inherit from VCMEncodedFrame.

Let the FrameObject class inherit from VCMEncodedFrame since the rest of the
decoding pipeline use VCMEncodedFrame.

BUG=webrtc:5514
R=stefan@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/552866c4020adb65a0de569c4...
==========

[commit-bot: I haz the power, 2016-07-04 14:19:15]

Description was changed from

==========
FrameObject inherit from VCMEncodedFrame.

Let the FrameObject class inherit from VCMEncodedFrame since the rest of the
decoding pipeline use VCMEncodedFrame.

BUG=webrtc:5514
R=stefan@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/552866c4020adb65a0de569c4...
==========

to

==========
FrameObject inherit from VCMEncodedFrame.

Let the FrameObject class inherit from VCMEncodedFrame since the rest of the
decoding pipeline use VCMEncodedFrame.

BUG=webrtc:5514
R=stefan@webrtc.org

Committed: https://crrev.com/552866c4020adb65a0de569c4f0ed2c6960314b4
Cr-Commit-Position: refs/heads/master@{#13380}
==========

[commit-bot: I haz the power, 2016-07-04 14:19:16]

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

[philipel, 2016-07-04 14:19:16]

Committed patchset #2 (id:20001) manually as
552866c4020adb65a0de569c4f0ed2c6960314b4 (presubmit successful).