Summary: | [GStreamer] create playbin in ::load(), not in player constructor | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Víctor M. Jáquez L. <vjaquez> | ||||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | eric.carlson, feature-media-reviews, gustavo, menard, mrobinson, philn, pnormand, webkit-ews, webkit.review.bot, xan.lopez | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 31155 | ||||||||||||
Attachments: |
|
Description
Víctor M. Jáquez L.
2012-12-03 08:27:28 PST
Created attachment 179587 [details]
Patch
Comment on attachment 179587 [details] Patch Attachment 179587 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/15366013 Comment on attachment 179587 [details] Patch Attachment 179587 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/15362036 Comment on attachment 179587 [details] Patch Attachment 179587 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/15317988 Comment on attachment 179587 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=179587&action=review Looks good I guess, I'd like to test it before landing. Are the tests still passing? including http/tests/media ? > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1774 > + enableDownloadBuffering(); s/enable/set I guess Created attachment 179589 [details]
Patch
(In reply to comment #5) > (From update of attachment 179587 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=179587&action=review > > Looks good I guess, I'd like to test it before landing. Are the tests still passing? including http/tests/media ? > > > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1774 > > + enableDownloadBuffering(); > > s/enable/set I guess Yep All the tests passed ok $ Tools/Scripts/run-webkit-tests --gtk http/test/media Using port 'gtk' Test configuration: <, x86, release> Placing test results in /home/vjaquez/WebKit/WebKitBuild/Release/layout-test-results Baseline search path: gtk -> generic Using Release build Pixel tests disabled Regular timeout: 6000, slow test timeout: 30000 Command line: /home/vjaquez/WebKit/WebKitBuild/Release/Programs/DumpRenderTree - Found 39 tests; running 21, skipping 18. Running 1 DumpRenderTree over 1 shard. All 21 tests ran as expected. Comment on attachment 179589 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=179589&action=review > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:-1753 > - ASSERT(m_playBin); I think it'd make sense to move this ASSERT in ::load, before setting the url of playbin. Created attachment 179601 [details]
Patch
Comment on attachment 179601 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=179601&action=review Some more feedback, I'd still like to test this patch as well :) > Source/WebCore/ChangeLog:8 > + This patch does the instantiation of the playbin pipeline into the WebCore::MediaPlayerPrivateGStreamer::load() method, so we could construct the pipeline take in count other possible options, such as pitch. Sorry for the late feedback on this, can you please wrap this line so it's not ultra wide huge? :) Also rephrase the second part, such as "so the pipeline layout can reflect other predefined settings such as audio pitch preservation in variable playback rate scenarios" ? Created attachment 179645 [details]
Patch
Comment on attachment 179645 [details] Patch Clearing flags on attachment: 179645 Committed r137838: <http://trac.webkit.org/changeset/137838> All reviewed patches have been landed. Closing bug. |