Summary: | [MediaStream API] HTMLMediaElement should be able to use MediaStream as source | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Carlson <eric.carlson> | ||||||||
Component: | WebCore Misc. | Assignee: | Nick Diego Yamane (diegoyam) <nick.diego> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cdumez, commit-queue, dino, esprehn+autocc, glenn, gtk-ews, gyuyoung.kim, hta, jer.noble, kaisers, kondapallykalyan, lauro.neto, nick.diego, rakuco, sergio, thiago.lacerda, tommyw, xan.lopez | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 124856, 124860, 124896 | ||||||||||
Bug Blocks: | 124288 | ||||||||||
Attachments: |
|
Description
Eric Carlson
2013-09-25 22:05:17 PDT
Created attachment 217891 [details]
Patch
Comment on attachment 217891 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=217891&action=review > Source/WebCore/html/HTMLMediaElement.cpp:324 > +#if ENABLE(MEDIA_STREAM) Shouldn't this be guarded with ENABLE(VIDEO) too? > Source/WebCore/html/HTMLMediaElement.cpp:753 > +#if ENABLE(MEDIA_STREAM) Ditto. > Source/WebCore/html/HTMLMediaElement.h:147 > +#if ENABLE(MEDIA_STREAM) Ditto. > Source/WebCore/html/HTMLMediaElement.h:808 > +#if ENABLE(MEDIA_STREAM) Ditto. Comment on attachment 217891 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=217891&action=review >> Source/WebCore/html/HTMLMediaElement.cpp:324 >> +#if ENABLE(MEDIA_STREAM) > > Shouldn't this be guarded with ENABLE(VIDEO) too? Everything in this file is already guarded with ENABLE(VIDEO) >> Source/WebCore/html/HTMLMediaElement.h:147 >> +#if ENABLE(MEDIA_STREAM) > > Ditto. Ditto. Created attachment 217896 [details]
Added Xcode stuff
Comment on attachment 217891 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=217891&action=review > Source/WebCore/Modules/mediastream/HTMLMediaElementMediaStream.cpp:38 > + ASSERT(mediaElement); If this can never be null, can you change the function to take a HTMLMediaElement&. > Source/WebCore/Modules/mediastream/HTMLMediaElementMediaStream.cpp:40 > + isNull = !mediaStream; This is probably not your doing, but passing null out of band here seems wasteful, since we have an inband signal already (the value of the pointer). > Source/WebCore/Modules/mediastream/HTMLMediaElementMediaStream.cpp:46 > + ASSERT(mediaElement); If this can never be null, can you change the function to take a HTMLMediaElement&. > Source/WebCore/Modules/mediastream/HTMLMediaElementMediaStream.h:40 > + static PassRefPtr<MediaStream> srcObject(HTMLMediaElement*, bool& isNull); This should not return a PassRefPtr. Just good old MediaStream*. > Source/WebCore/html/HTMLMediaElement.h:148 > + PassRefPtr<MediaStream> srcObject() const { return m_mediaStreamSrcObject; } This should not return a PassRefPtr. Just good old MediaStream*. Comment on attachment 217891 [details] Patch Attachment 217891 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/37468030 (In reply to comment #5) > (From update of attachment 217891 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=217891&action=review > > > Source/WebCore/Modules/mediastream/HTMLMediaElementMediaStream.cpp:38 > > + ASSERT(mediaElement); > > If this can never be null, can you change the function to take a HTMLMediaElement&. I suspect that's not possible, because this function is called by the auto-generated binding code, which in this case expects to pass a HTMLMediaElement* as the first parameter. > > > Source/WebCore/Modules/mediastream/HTMLMediaElementMediaStream.cpp:40 > > + isNull = !mediaStream; > > This is probably not your doing, but passing null out of band here seems wasteful, since we have an inband signal already (the value of the pointer). > 'isNull' parameter is also parte of the interface expected by auto-generated code. I'm not sure if it's necessary to set it according to the pointer value... > > Source/WebCore/Modules/mediastream/HTMLMediaElementMediaStream.cpp:46 > > + ASSERT(mediaElement); > > If this can never be null, can you change the function to take a HTMLMediaElement&. > > > Source/WebCore/Modules/mediastream/HTMLMediaElementMediaStream.h:40 > > + static PassRefPtr<MediaStream> srcObject(HTMLMediaElement*, bool& isNull); > > This should not return a PassRefPtr. Just good old MediaStream*. > Ok, done. > > Source/WebCore/html/HTMLMediaElement.h:148 > > + PassRefPtr<MediaStream> srcObject() const { return m_mediaStreamSrcObject; } > > This should not return a PassRefPtr. Just good old MediaStream*. ok Created attachment 217914 [details]
Proposed Patch
Fixing GTK build issues
Comment on attachment 217914 [details] Proposed Patch Clearing flags on attachment: 217914 Committed r159797: <http://trac.webkit.org/changeset/159797> All reviewed patches have been landed. Closing bug. |