1426486 - Make GetInputStream()->AsSourceStream() invariant

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, enhancement)

Description

[Jean-Yves Avenard [:jya]]

I can never return null

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1426486 - P1. Make GetInputStream()->AsSourceStream() invariant.

It can never be nullptr, strongly assert that this is the case and remove unecessary tests.

Review commit: https://reviewboard.mozilla.org/r/208844/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/208844/

Comment 2

[Andreas Pehrson [:pehrsons]]

Comment on attachment 8938130 [details]
Bug 1426486 - P1. Make GetInputStream()->AsSourceStream() invariant.

https://reviewboard.mozilla.org/r/208844/#review214786

::: media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp:2406
(Diff revision 1)
>      if (delta > 0) {
>        VideoSegment segment;
>        IntSize size = image ? image->GetSize() : IntSize(mWidth, mHeight);
>        segment.AppendFrame(image.forget(), delta, size, mPrincipalHandle);
>        // Handle track not actually added yet or removed/finished
> -      if (!mTrack->GetInputStream()->AsSourceStream()->AppendToTrack(
> +      if (!mSource->AppendToTrack(mTrack->GetInputTrackId(), &segment)) {

Looks weird to do mTrack->GetInputTrackId() here when we have mTrackId.

Could you do the same to mTrackId as you just did to mSource? Thanks.

Comment 3

[Jean-Yves Avenard [:jya]]

Comment on attachment 8938130 [details]
Bug 1426486 - P1. Make GetInputStream()->AsSourceStream() invariant.

https://reviewboard.mozilla.org/r/208844/#review214786

> Looks weird to do mTrack->GetInputTrackId() here when we have mTrackId.
> 
> Could you do the same to mTrackId as you just did to mSource? Thanks.

this appears out of scope, also most of the use of mTrackId appears unused for the video MediaPipeline so I'll remove it in a P2

Comment 4

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1426486 - P2. Make mTrackId a base member.

Also remove unused code.
mTrackId / mTrackIdexternal were protected/private members and only ever written.

Review commit: https://reviewboard.mozilla.org/r/209108/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/209108/

Comment 5

[Jean-Yves Avenard [:jya]]

Comment on attachment 8938130 [details]
Bug 1426486 - P1. Make GetInputStream()->AsSourceStream() invariant.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/208844/diff/1-2/

Comment 6

[Byron Campen [:bwc]]

Comment on attachment 8938408 [details]
Bug 1426486 - P2. Make mTrackId a base member.

https://reviewboard.mozilla.org/r/209108/#review214876

Comment 7

[Jean-Yves Avenard [:jya]]

Comment on attachment 8938408 [details]
Bug 1426486 - P2. Make mTrackId a base member.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/209108/diff/1-2/

Comment 8

[Jean-Yves Avenard [:jya]]

thanks for the reviews....

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5475206095b7
P1. Make GetInputStream()->AsSourceStream() invariant. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/85b81d55503a
P2. Make mTrackId a base member. r=bwc

Comment 10

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/5475206095b7
https://hg.mozilla.org/mozilla-central/rev/85b81d55503a