Add rotation to EncodedImage and make sure it is passed through encoders.

Add rotation to EncodedImage and make sure it is passed through encoders.
This fix a potential race where the rotation information of a sent frame does not match the encoded frame.

BUG=webrtc:5783
TEST= Run ApprtcDemo on IOs and Android with and without capture to texture and both VP8 and H264.
R=magjed@webrtc.org, pbos@webrtc.org, tkchin@webrtc.org
TBR=tkchin_webrtc // For IOS changes.

Committed: https://crrev.com/ba7dc723b0922b9b46c2051dc90f6c127a93fbfb
Cr-Commit-Position: refs/heads/master@{#12426}

[perkj_webrtc, 2016-04-14 11:23:42]

Description was changed from

==========
Add rotation to EncodedImage and make sure it is passed through mobile encoders.

BUG=
==========

to

==========
Add rotation to EncodedImage and make sure it is passed through mobile encoders.

BUG=webrtc::5783
==========

[perkj_webrtc, 2016-04-14 11:50:18]

Description was changed from

==========
Add rotation to EncodedImage and make sure it is passed through mobile encoders.

BUG=webrtc::5783
==========

to

==========
Add rotation to EncodedImage and make sure it is passed through mobile encoders.

BUG=webrtc::5783
TEST= Run ApprtcDemo on IOs and Android with and without capture to texture and
both VP8 and H264.
==========

[perkj_webrtc, 2016-04-14 14:17:25]

perkj@webrtc.org changed reviewers:
+ magjed@webrtc.org, pbos@webrtc.org, tkchin@webrtc.org

[perkj_webrtc, 2016-04-14 14:17:26]

Can you please review:

pbos- all but Android and Ios
magjed - Android
tkchin - IOS.

[pbos-webrtc, 2016-04-14 14:29:57]

lgtm

https://codereview.webrtc.org/1886113003/diff/40001/webrtc/api/java/jni/andro...
File webrtc/api/java/jni/androidmediaencoder_jni.cc (right):

https://codereview.webrtc.org/1886113003/diff/40001/webrtc/api/java/jni/andro...
webrtc/api/java/jni/androidmediaencoder_jni.cc:15: #include <algorithm>
Not needed, right?

[pbos-webrtc, 2016-04-14 14:30:09]

Description was changed from

==========
Add rotation to EncodedImage and make sure it is passed through mobile encoders.

BUG=webrtc::5783
TEST= Run ApprtcDemo on IOs and Android with and without capture to texture and
both VP8 and H264.
==========

to

==========
Add rotation to EncodedImage and make sure it is passed through mobile encoders.

BUG=webrtc:5783
TEST= Run ApprtcDemo on IOs and Android with and without capture to texture and
both VP8 and H264.
==========

[pbos-webrtc, 2016-04-14 14:30:29]

This doesn't only affect mobile encoders, please update the CL description to
reflect that.

[perkj_webrtc, 2016-04-15 08:18:16]

Description was changed from

==========
Add rotation to EncodedImage and make sure it is passed through mobile encoders.

BUG=webrtc:5783
TEST= Run ApprtcDemo on IOs and Android with and without capture to texture and
both VP8 and H264.
==========

to

==========
Add rotation to EncodedImage and make sure it is passed through encoders.
This fix a potential race where the rotation information of a sent frame does
not match the encoded frame.

BUG=webrtc:5783
TEST= Run ApprtcDemo on IOs and Android with and without capture to texture and
both VP8 and H264.
==========

[magjed_webrtc, 2016-04-15 09:43:30]

https://codereview.webrtc.org/1886113003/diff/40001/webrtc/api/java/jni/andro...
File webrtc/api/java/jni/androidmediaencoder_jni.cc (right):

https://codereview.webrtc.org/1886113003/diff/40001/webrtc/api/java/jni/andro...
webrtc/api/java/jni/androidmediaencoder_jni.cc:227: int frames_in_queue_;  //
Number of frames in encoder queue.
This looks identical to input_frame_infos_.size(). Can you remove this variable?

https://codereview.webrtc.org/1886113003/diff/40001/webrtc/api/java/jni/andro...
webrtc/api/java/jni/androidmediaencoder_jni.cc:248: std::vector<int64_t>
frame_rtc_times_ms_;  // Time when video frame is sent to
Can you add a rtc_time member in InputFrameInfo and merge frame_rtc_times_ms_
with input_frame_infos_?

https://codereview.webrtc.org/1886113003/diff/40001/webrtc/api/java/jni/andro...
webrtc/api/java/jni/androidmediaencoder_jni.cc:250: int32_t output_timestamp_;  
              // Last output frame timestamp from
Can you replace |output_timestamp_|, |output_render_time_ms|, and
|output_render_time_ms_| with 'InputFrameInfo last_output_| instead? Down in
DeliverPendingOutputs(), you can write:
last_output_ = input_frame_infos_.front();
input_frame_infos_.pop_front();

https://codereview.webrtc.org/1886113003/diff/40001/webrtc/api/java/jni/andro...
webrtc/api/java/jni/androidmediaencoder_jni.cc:754: 
nit: remove empty line

https://codereview.webrtc.org/1886113003/diff/40001/webrtc/api/java/jni/andro...
webrtc/api/java/jni/androidmediaencoder_jni.cc:755:
input_frame_infos_.push_back(InputFrameInfo(input_frame.timestamp(),
emplace_back is allowed now according to http://chromium-cpp.appspot.com/, so I
think you should use that instead of push_back here.

https://codereview.webrtc.org/1886113003/diff/40001/webrtc/api/java/jni/andro...
webrtc/api/java/jni/androidmediaencoder_jni.cc:963: if (frames_in_queue_ > 0) {
Can you CHECK_GT(frames_in_queue_, 0) here? It looks it should never happen that
we have no metadata for a frame.

[perkj_webrtc, 2016-04-15 12:37:39]

https://codereview.webrtc.org/1886113003/diff/40001/webrtc/api/java/jni/andro...
File webrtc/api/java/jni/androidmediaencoder_jni.cc (right):

https://codereview.webrtc.org/1886113003/diff/40001/webrtc/api/java/jni/andro...
webrtc/api/java/jni/androidmediaencoder_jni.cc:15: #include <algorithm>
On 2016/04/14 14:29:57, pbos-webrtc wrote:
> Not needed, right?

git cl lint say its needed.

https://codereview.webrtc.org/1886113003/diff/40001/webrtc/api/java/jni/andro...
webrtc/api/java/jni/androidmediaencoder_jni.cc:227: int frames_in_queue_;  //
Number of frames in encoder queue.
On 2016/04/15 09:43:30, magjed_webrtc wrote:
> This looks identical to input_frame_infos_.size(). Can you remove this
variable?

Done.

https://codereview.webrtc.org/1886113003/diff/40001/webrtc/api/java/jni/andro...
webrtc/api/java/jni/androidmediaencoder_jni.cc:248: std::vector<int64_t>
frame_rtc_times_ms_;  // Time when video frame is sent to
On 2016/04/15 09:43:30, magjed_webrtc wrote:
> Can you add a rtc_time member in InputFrameInfo and merge frame_rtc_times_ms_
> with input_frame_infos_?

Done.

https://codereview.webrtc.org/1886113003/diff/40001/webrtc/api/java/jni/andro...
webrtc/api/java/jni/androidmediaencoder_jni.cc:250: int32_t output_timestamp_;  
              // Last output frame timestamp from
On 2016/04/15 09:43:30, magjed_webrtc wrote:
> Can you replace |output_timestamp_|, |output_render_time_ms|, and
> |output_render_time_ms_| with 'InputFrameInfo last_output_| instead? Down in
> DeliverPendingOutputs(), you can write:
> last_output_ = input_frame_infos_.front();
> input_frame_infos_.pop_front();

I could but that would mean a slight beaviour difference that I am not sure I am
comfortable with. ie- two callbacks for the same input frame. Separate cl?

https://codereview.webrtc.org/1886113003/diff/40001/webrtc/api/java/jni/andro...
webrtc/api/java/jni/androidmediaencoder_jni.cc:754: 
On 2016/04/15 09:43:30, magjed_webrtc wrote:
> nit: remove empty line

Done.

https://codereview.webrtc.org/1886113003/diff/40001/webrtc/api/java/jni/andro...
webrtc/api/java/jni/androidmediaencoder_jni.cc:755:
input_frame_infos_.push_back(InputFrameInfo(input_frame.timestamp(),
On 2016/04/15 09:43:30, magjed_webrtc wrote:
> emplace_back is allowed now according to http://chromium-cpp.appspot.com/, so
I
> think you should use that instead of push_back here.

Done.

https://codereview.webrtc.org/1886113003/diff/40001/webrtc/api/java/jni/andro...
webrtc/api/java/jni/androidmediaencoder_jni.cc:963: if (frames_in_queue_ > 0) {
On 2016/04/15 09:43:30, magjed_webrtc wrote:
> Can you CHECK_GT(frames_in_queue_, 0) here? It looks it should never happen
that
> we have no metadata for a frame.

as discussed- I don't want to in this cl. The comment above make me think that
this might be something that may happen with H264 for the first frame when
configuration parameters are created. But this all look a bit weird.

[magjed_webrtc, 2016-04-18 10:01:49]

lgtm

[perkj_webrtc, 2016-04-18 13:40:52]

Description was changed from

==========
Add rotation to EncodedImage and make sure it is passed through encoders.
This fix a potential race where the rotation information of a sent frame does
not match the encoded frame.

BUG=webrtc:5783
TEST= Run ApprtcDemo on IOs and Android with and without capture to texture and
both VP8 and H264.
==========

to

==========
Add rotation to EncodedImage and make sure it is passed through encoders.
This fix a potential race where the rotation information of a sent frame does
not match the encoded frame.

BUG=webrtc:5783
TEST= Run ApprtcDemo on IOs and Android with and without capture to texture and
both VP8 and H264.

TBR=tkchin_webrtc // For IOS changes.
==========

[perkj_webrtc, 2016-04-18 13:41:07]

The CQ bit was checked by perkj@webrtc.org

[perkj_webrtc, 2016-04-18 13:41:08]

The patchset sent to the CQ was uploaded after l-g-t-m from pbos@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1886113003/#ps60001 (title:
"Addressed magjeds comments")

[commit-bot: I haz the power, 2016-04-18 13:41:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1886113003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1886113003/60001

[commit-bot: I haz the power, 2016-04-18 13:42:20]

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

[commit-bot: I haz the power, 2016-04-18 13:42:21]

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

[tkchin_webrtc, 2016-04-18 17:05:09]

lgtm % bots
thanks

[perkj_webrtc, 2016-04-19 08:12:29]

The CQ bit was checked by perkj@webrtc.org

[perkj_webrtc, 2016-04-19 08:12:30]

The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org,
pbos@webrtc.org, tkchin@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1886113003/#ps80001 (title:
"Rebased")

[commit-bot: I haz the power, 2016-04-19 08:12:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1886113003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1886113003/80001

[commit-bot: I haz the power, 2016-04-19 08:19:29]

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

[commit-bot: I haz the power, 2016-04-19 08:19:30]

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

[perkj_webrtc, 2016-04-19 12:41:15]

The CQ bit was checked by perkj@webrtc.org

[perkj_webrtc, 2016-04-19 12:41:15]

The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org,
pbos@webrtc.org, tkchin@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1886113003/#ps100001 (title:
"Fix merge mistake.")

[commit-bot: I haz the power, 2016-04-19 12:41:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1886113003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1886113003/100001

[perkj_webrtc, 2016-04-19 13:01:37]

Description was changed from

==========
Add rotation to EncodedImage and make sure it is passed through encoders.
This fix a potential race where the rotation information of a sent frame does
not match the encoded frame.

BUG=webrtc:5783
TEST= Run ApprtcDemo on IOs and Android with and without capture to texture and
both VP8 and H264.

TBR=tkchin_webrtc // For IOS changes.
==========

to

==========
Add rotation to EncodedImage and make sure it is passed through encoders.
This fix a potential race where the rotation information of a sent frame does
not match the encoded frame.

BUG=webrtc:5783
TEST= Run ApprtcDemo on IOs and Android with and without capture to texture and
both VP8 and H264.
R=magjed@webrtc.org, pbos@webrtc.org, tkchin@webrtc.org
TBR=tkchin_webrtc // For IOS changes.

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

[perkj_webrtc, 2016-04-19 13:01:39]

Committed patchset #6 (id:100001) manually as
ba7dc723b0922b9b46c2051dc90f6c127a93fbfb (presubmit successful).

[commit-bot: I haz the power, 2016-04-19 13:01:39]

Description was changed from

==========
Add rotation to EncodedImage and make sure it is passed through encoders.
This fix a potential race where the rotation information of a sent frame does
not match the encoded frame.

BUG=webrtc:5783
TEST= Run ApprtcDemo on IOs and Android with and without capture to texture and
both VP8 and H264.
R=magjed@webrtc.org, pbos@webrtc.org, tkchin@webrtc.org
TBR=tkchin_webrtc // For IOS changes.

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

to

==========
Add rotation to EncodedImage and make sure it is passed through encoders.
This fix a potential race where the rotation information of a sent frame does
not match the encoded frame.

BUG=webrtc:5783
TEST= Run ApprtcDemo on IOs and Android with and without capture to texture and
both VP8 and H264.
R=magjed@webrtc.org, pbos@webrtc.org, tkchin@webrtc.org
TBR=tkchin_webrtc // For IOS changes.

Committed: https://crrev.com/ba7dc723b0922b9b46c2051dc90f6c127a93fbfb
Cr-Commit-Position: refs/heads/master@{#12426}
==========

[commit-bot: I haz the power, 2016-04-19 13:01:41]

Patchset 6 (id:??) landed as
https://crrev.com/ba7dc723b0922b9b46c2051dc90f6c127a93fbfb
Cr-Commit-Position: refs/heads/master@{#12426}