Bug 84058

Summary: Move AudioDestinationChromium FIFO class to its own class.
Product: WebKit Reporter: Raymond Toy <rtoy>
Component: New BugsAssignee: Raymond Toy <rtoy>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, feature-media-reviews, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Raymond Toy
Reported 2012-04-16 11:49:26 PDT
Move AudioDestinationChromium FIFO class to its own class.
Attachments
Patch (21.96 KB, patch)
2012-04-16 11:52 PDT, Raymond Toy 		no flags
DetailsFormatted DiffDiff
Patch (22.68 KB, patch)
2012-04-16 13:57 PDT, Raymond Toy 		no flags
DetailsFormatted DiffDiff
Patch (22.49 KB, patch)
2012-04-17 10:09 PDT, Raymond Toy 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Raymond Toy
Comment 1 2012-04-16 11:52:33 PDT
Created attachment 137372 [details] Patch
Chris Rogers
Comment 2 2012-04-16 13:00:38 PDT
Comment on attachment 137372 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=137372&action=review > Source/WebCore/WebCore.gypi:3177 > + 'platform/audio/AudioPullFIFO.h', nit: alpha order > Source/WebCore/platform/audio/AudioPullFIFO.h:38 > +// fulfill a request. It's probably a good idea to update this comment to be a little more general. I think the first part: "A FIFO (First In First Out) buffer to handle mismatches in" and the last part: "This FIFO will ask the provider for more data when necessary to fulfill a request." Are good, but the middle part can be made more general -- and also maybe add the concept of "pull" in there (as opposed to push) > Source/WebKit/chromium/src/AudioDestinationChromium.h:34 > +#include "AudioPullFIFO.h" Can you forward declare this class in the header file, and move this #include to the .cpp?
Raymond Toy
Comment 3 2012-04-16 13:57:46 PDT
Created attachment 137396 [details] Patch
Raymond Toy
Comment 4 2012-04-16 13:58:52 PDT
Comment on attachment 137372 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=137372&action=review >> Source/WebCore/WebCore.gypi:3177 >> + 'platform/audio/AudioPullFIFO.h', > > nit: alpha order Fixed. >> Source/WebCore/platform/audio/AudioPullFIFO.h:38 >> +// fulfill a request. > > It's probably a good idea to update this comment to be a little more general. I think the first part: > > "A FIFO (First In First Out) buffer to handle mismatches in" > > and the last part: > "This FIFO will ask the provider for more data when necessary to fulfill a request." > > Are good, but the middle part can be made more general -- and also maybe add the concept of "pull" in there (as opposed to push) Updated. >> Source/WebKit/chromium/src/AudioDestinationChromium.h:34 >> +#include "AudioPullFIFO.h" > > Can you forward declare this class in the header file, and move this #include to the .cpp? Done.
Raymond Toy
Comment 5 2012-04-17 10:09:53 PDT
Created attachment 137554 [details] Patch
Chris Rogers
Comment 6 2012-04-23 17:02:53 PDT
Comment on attachment 137554 [details] Patch Thanks Ray!
WebKit Review Bot
Comment 7 2012-04-23 17:39:35 PDT
Comment on attachment 137554 [details] Patch Clearing flags on attachment: 137554 Committed r114970: <http://trac.webkit.org/changeset/114970>
WebKit Review Bot
Comment 8 2012-04-23 17:39:40 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.