1266644 - Rename StreamBuffers to StreamTracks.

Status: RESOLVED FIXED

(Core :: Audio/Video: MediaStreamGraph, defect, P2)

Description

[Chia-hung Tai [:ctai]]

Once Bug 1201363 landed, there will no video buffer in MSG. Rename Bug 1201363 first.

Comment 1

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: bug-1266644-1.diff

Carry r+ from pehrsons. See https://bugzilla.mozilla.org/show_bug.cgi?id=1201363#c87

Comment 2

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: bug-1266644-2.diff

Comment 3

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Simple refactor.
(In reply to Chia-hung Tai [:ctai :ctai_mozilla :cht] from comment #2)
> Created attachment 8744171 [details] [diff] [review]
> bug-1266644-2.diff

Comment 4

[Andreas Pehrson [:pehrsons]]

Comment on attachment 8744171 [details] [diff] [review]
bug-1266644-2.diff

Review of attachment 8744171 [details] [diff] [review]:
-----------------------------------------------------------------

Please divide these into smaller commits before landing. Feel free to carry forward my r+, modulo the inline comment below.

::: dom/media/MediaStreamGraphImpl.h
@@ +92,5 @@
>   * be able to friend it.
>   *
> + * There can be multiple MediaStreamGraph per process: one per AudioChannel.
> + * Additionaly, each OfflineAudioContext object creates its own MediaStreamGraph
> + * object too.

padenot should take a look at this.

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8744170 [details] [diff] [review]
bug-1266644-1.diff

Review of attachment 8744170 [details] [diff] [review]:
-----------------------------------------------------------------

r+ but PLEASE instead of hg remove and hg add for StreamBuffer* -> StreamTrack*, use hg rename for each file.  This keeps history across the rename.

Comment 6

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Thanks for the reminder, but I did use hg rename for StreamBuffer* -> StreamTrack*.
roc already reminded me in his review(https://bugzilla.mozilla.org/show_bug.cgi?id=1201363#c56).
And I did that in previous patch. See https://reviewboard.mozilla.org/r/42457/diff/1#index_header 
Also in the link of the detail of the attached patch, it shows 

diff --git a/dom/media/StreamBuffer.cpp b/StreamTracks.cpp
rename from dom/media/StreamBuffer.cpp
rename to StreamTracks.cpp
diff --git a/dom/media/StreamBuffer.h b/StreamTracks.h
rename from dom/media/StreamBuffer.h
rename to StreamTracks.h

See https://bugzilla.mozilla.org/attachment.cgi?id=8744170&action=edit

I think that might be caused by the diff and review page of bugzilla won't reflect that but mozreview did.
Anyway, thanks for your review. :)

(In reply to Randell Jesup [:jesup] from comment #5)
> Comment on attachment 8744170 [details] [diff] [review]
> bug-1266644-1.diff
> 
> Review of attachment 8744170 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+ but PLEASE instead of hg remove and hg add for StreamBuffer* ->
> StreamTrack*, use hg rename for each file.  This keeps history across the
> rename.

Comment 7

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Try result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c7d36f091567&selectedJob=19842387

Comment 8

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: bug-1266644-1.diff

Carry r+

Comment 9

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: bug-1266644-2.diff

Carry r+.

Comment 10

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: bug-1266644-3.diff

Carry r+.

Comment 11

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: bug-1266644-4.diff

Carry r+.

Comment 12

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: bug-1266644-5.diff

Carry r+.

Comment 13

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: bug-1266644-6.diff

Carry r+.

Comment 14

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: bug-1266644-7.diff

Carry r+.

Comment 15

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Comment on attachment 8744802 [details] [diff] [review]
bug-1266644-5.diff

Hi, Paul,
pehrsons suggest me that you might review these comments in comment 5. Thanks.

Comment 16

[Paul Adenot (:padenot)]

Comment on attachment 8744802 [details] [diff] [review]
bug-1266644-5.diff

Review of attachment 8744802 [details] [diff] [review]:
-----------------------------------------------------------------

Yep, thanks for fixing this comment !

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fd16a818e34e
https://hg.mozilla.org/integration/mozilla-inbound/rev/eb0d98b53876
https://hg.mozilla.org/integration/mozilla-inbound/rev/41f21e3f7cbd
https://hg.mozilla.org/integration/mozilla-inbound/rev/5bbea4dc90c0
https://hg.mozilla.org/integration/mozilla-inbound/rev/8cdb01638397
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ccb82588607
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f43cfbd45ca

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/fd16a818e34e
https://hg.mozilla.org/mozilla-central/rev/eb0d98b53876
https://hg.mozilla.org/mozilla-central/rev/41f21e3f7cbd
https://hg.mozilla.org/mozilla-central/rev/5bbea4dc90c0
https://hg.mozilla.org/mozilla-central/rev/8cdb01638397
https://hg.mozilla.org/mozilla-central/rev/0ccb82588607
https://hg.mozilla.org/mozilla-central/rev/5f43cfbd45ca

Comment 19

[Cameron McCormack (:heycam)]

ctai, did you mean to move StreamTracks.h/cpp into the root directory?

Comment 20

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Oh, no. May bad.
I would like to move to /dom/media/StreamTracks.cpp
Thanks for pointing out.


(In reply to Cameron McCormack (:heycam) from comment #19)
> ctai, did you mean to move StreamTracks.h/cpp into the root directory?

Comment 21

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: bug1266644-correct history-1.diff

Correct the hg history. Thanks for JW's help.

Comment 22

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: bug1266644-correct history part2.diff

Comment 23

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7f873753780e
https://hg.mozilla.org/integration/mozilla-inbound/rev/58ea1b949927

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7f873753780e
https://hg.mozilla.org/mozilla-central/rev/58ea1b949927