Bug 125156

Summary: [MSE][Mac] Report the intrinsic size of the media element
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: MediaAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, glenn
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch eric.carlson: review+

Jer Noble
Reported 2013-12-03 08:50:37 PST
[MSE][Mac] Report the intrinsic size of the media element from MediaPlayerPrivateMediaSourceAVFObjC::naturalSize().
Attachments
Patch (21.53 KB, patch)
2014-01-08 23:32 PST, Jer Noble 		eric.carlson: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Jer Noble
Comment 1 2014-01-08 23:32:25 PST
Created attachment 220699 [details] Patch
Eric Carlson
Comment 2 2014-01-09 07:30:06 PST
Comment on attachment 220699 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220699&action=review A test case would be useful. > Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:331 > + auto videoTrack = VideoTrackPrivateMediaSourceAVFObjC::create(track, this); This seems like one if the cases where "auto" is not helpful because someone reading the code can't know what style of ref-ptr VideoTrackPrivateMediaSourceAVFObjC::create returns. > Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:338 > + auto audioTrack = AudioTrackPrivateMediaSourceAVFObjC::create(track, this); Ditto.
Jer Noble
Comment 3 2014-01-09 16:25:50 PST
(In reply to comment #2) > (From update of attachment 220699 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=220699&action=review > > A test case would be useful. Unfortunately, this isn't really testable at the moment; we could add a Mock test, but that wouldn't test this change. > > Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:331 > > + auto videoTrack = VideoTrackPrivateMediaSourceAVFObjC::create(track, this); > > This seems like one if the cases where "auto" is not helpful because someone reading the code can't know what style of ref-ptr VideoTrackPrivateMediaSourceAVFObjC::create returns. I wonder if "RefPtr<auto>" would be valid syntax? > > Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:338 > > + auto audioTrack = AudioTrackPrivateMediaSourceAVFObjC::create(track, this); > > Ditto. Ditto.
Jer Noble
Comment 4 2014-01-09 16:29:11 PST
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 220699 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=220699&action=review > > > > A test case would be useful. > > Unfortunately, this isn't really testable at the moment; we could add a Mock test, but that wouldn't test this change. > > > > Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:331 > > > + auto videoTrack = VideoTrackPrivateMediaSourceAVFObjC::create(track, this); > > > > This seems like one if the cases where "auto" is not helpful because someone reading the code can't know what style of ref-ptr VideoTrackPrivateMediaSourceAVFObjC::create returns. > > I wonder if "RefPtr<auto>" would be valid syntax? > Nope. "error: 'auto' not allowed in template argument". I'll switch these back to non-auto types.
Jer Noble
Comment 5 2014-01-09 17:45:51 PST
Committed r161612: <http://trac.webkit.org/changeset/161612>
Darin Adler
Comment 6 2014-01-10 12:44:52 PST
Comment on attachment 220699 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220699&action=review > Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:648 > + for (auto videoTrack : m_videoTracks) { Here auto is causing reference count churn. You could use auto& or const auto& to avoid that.
Note You need to log in before you can comment on or make changes to this bug.