Bug 60205 - Media Stream API: integrate the Track List objects into the existing code
Summary: Media Stream API: integrate the Track List objects into the existing code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Tommy Widenflycht
URL:
Keywords:
Depends on: 60177 60184
Blocks: 56459 61069
  Show dependency treegraph
 
Reported: 2011-05-04 13:16 PDT by Leandro Graciá Gil
Modified: 2011-06-08 06:10 PDT (History)
4 users (show)

See Also:


Attachments
Patch (28.38 KB, patch)
2011-05-31 05:02 PDT, Tommy Widenflycht
no flags Details | Formatted Diff | Diff
Patch (28.43 KB, patch)
2011-06-08 04:45 PDT, Tommy Widenflycht
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Leandro Graciá Gil 2011-05-04 13:16:23 PDT
Add compile-guarded modifications to the Track List objects to make them fully compatible with their use in the Media Stream API. Update also the controllers to provide these lists on stream generation.
Comment 1 Tommy Widenflycht 2011-05-31 05:02:24 PDT
Created attachment 95421 [details]
Patch
Comment 2 Tony Gentilcore 2011-06-07 06:42:54 PDT
Comment on attachment 95421 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=95421&action=review

Just a nit and a question, everything else looks good to me.

> Source/WebCore/dom/ExclusiveTrackList.cpp:73
> +void ExclusiveTrackList::trackFailed(unsigned long index)

It isn't obvious to me why mediaStreamFrameController doesn't need to be notified.

> Source/WebCore/dom/TrackList.cpp:167
>      // For the HTML Media Element: https://bugs.webkit.org/show_bug.cgi?id=61127

This comment has a redundant "for the HTML Media Element" now.
Comment 3 Leandro Graciá Gil 2011-06-07 06:51:26 PDT
Comment on attachment 95421 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=95421&action=review

>> Source/WebCore/dom/ExclusiveTrackList.cpp:73
>> +void ExclusiveTrackList::trackFailed(unsigned long index)
> 
> It isn't obvious to me why mediaStreamFrameController doesn't need to be notified.

It is. In fact the MediaStreamFrameController is the one calling this method. Just look for MediaStreamFrameController::audioTrackFailed and MediaStreamFrameController::videoTrackFailed. Is that what you mean?
Comment 4 Tony Gentilcore 2011-06-07 07:45:25 PDT
> It is. In fact the MediaStreamFrameController is the one calling this method. Just look for MediaStreamFrameController::audioTrackFailed and MediaStreamFrameController::videoTrackFailed. Is that what you mean?

Thanks for the explanation. Looks good.
Comment 5 Tommy Widenflycht 2011-06-08 04:43:07 PDT
Comment on attachment 95421 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=95421&action=review

>> Source/WebCore/dom/TrackList.cpp:167
>>      // For the HTML Media Element: https://bugs.webkit.org/show_bug.cgi?id=61127
> 
> This comment has a redundant "for the HTML Media Element" now.

Removed.
Comment 6 Tommy Widenflycht 2011-06-08 04:45:15 PDT
Created attachment 96405 [details]
Patch
Comment 7 WebKit Review Bot 2011-06-08 06:08:23 PDT
Comment on attachment 96405 [details]
Patch

Clearing flags on attachment: 96405

Committed r88341: <http://trac.webkit.org/changeset/88341>
Comment 8 WebKit Review Bot 2011-06-08 06:08:27 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Tommy Widenflycht 2011-06-08 06:10:46 PDT
Changed platform to All/All