Summary: | [GStreamer] [texmap] upload a video buffer into the video texture | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Víctor M. Jáquez L. <vjaquez> | ||||||||||||||||||||||
Component: | Media | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||
Severity: | Normal | CC: | b.long, commit-queue, eflews.bot, ehdgms, eric.carlson, glenn, gtk-ews, gustavo, gyuyoung.kim, hk, jer.noble, menard, mrobinson, pnormand, rego+ews, slomo, webkit-ews, xan.lopez, zan, zarvai | ||||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||
Bug Depends on: | 86410 | ||||||||||||||||||||||||
Bug Blocks: | |||||||||||||||||||||||||
Attachments: |
|
Description
Víctor M. Jáquez L.
2013-05-13 09:52:05 PDT
Created attachment 201579 [details]
upload the buffer with GstSurface{Meta/Convert} (GStreamer 1.0 -deprecated-)
This patch enables the purpose of this bug, but uses deprecated GStreamer API.
Attachment 201579 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp', u'Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h', u'Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp', u'Source/autotools/FindDependencies.m4']" exit_code: 1
Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:43: Alphabetical sorting problem. [build/include_order] [4]
Total errors found: 1 in 4 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 201579 [details] upload the buffer with GstSurface{Meta/Convert} (GStreamer 1.0 -deprecated-) Attachment 201579 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/404791 Comment on attachment 201579 [details] upload the buffer with GstSurface{Meta/Convert} (GStreamer 1.0 -deprecated-) Attachment 201579 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/375833 Comment on attachment 201579 [details] upload the buffer with GstSurface{Meta/Convert} (GStreamer 1.0 -deprecated-) Attachment 201579 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/461779 Comment on attachment 201579 [details] upload the buffer with GstSurface{Meta/Convert} (GStreamer 1.0 -deprecated-) Attachment 201579 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/451832 Created attachment 204047 [details]
Patch
I've two questions regarding this patch: 1) Shall I maintain the deprecated/unofficial GstSurfaceMeta/GstSurfaceConvert API? If we remove it, the code will be simpler and more easy to read. 2) Should I add the support got GstCapsFeatures? I guess we could do this after this patch get landed. Comment on attachment 204047 [details] Patch Attachment 204047 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/777474 Comment on attachment 204047 [details] Patch Attachment 204047 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/793136 Comment on attachment 204047 [details] Patch Attachment 204047 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/777473 Comment on attachment 204047 [details] Patch Attachment 204047 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/774532 Yeah, if we go supporting the deprecated GstSurfaceMeta, I also need to modify the cmake scripts... Comment on attachment 204047 [details] Patch Attachment 204047 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/793139 (In reply to comment #8) > I've two questions regarding this patch: > > 1) Shall I maintain the deprecated/unofficial GstSurfaceMeta/GstSurfaceConvert API? > > If we remove it, the code will be simpler and more easy to read. > Yes let's remove it. We don't want to maintain deprecated API usage :) > 2) Should I add the support got GstCapsFeatures? > > I guess we could do this after this patch get landed. Yes sounds like a good idea! Created attachment 204060 [details]
Patch
Comment on attachment 204060 [details] Patch Attachment 204060 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/709943 Comment on attachment 204060 [details] Patch Attachment 204060 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/784156 Comment on attachment 204060 [details] Patch Attachment 204060 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/709944 Comment on attachment 204060 [details] Patch Attachment 204060 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/779180 Created attachment 204061 [details]
Patch
Comment on attachment 204061 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204061&action=review Is the purpose of this patch to more quickly prepare the texture in the case that the video is already in GPU memory? > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:356 > + if (meta) { > + const BitmapTextureGL* textureGL = static_cast<const BitmapTextureGL*>(m_texture.get()); > + guint ids[4] = { textureGL->id(), 0, 0, 0 }; > + > + ret = gst_video_gl_texture_upload_meta_upload(meta, ids); What's happening behind the scenes here? Is it just using the current OpenGL context? If not I wonder about the cost of context switching. In WebKit we do early returns, so you should write: if (GstVideoGLTextureUploadMeta* meta = gst_buffer_get_video_gl_texture_upload_meta(buffer)) { if (gst_video_gl_texture_upload_meta_upload(meta, ids)) { client()->setPlatformLayerNeedsDisplay(); return; } } The nice benefit being you don't have to touch the code below it. > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:362 > + { // copy the buffer into the texture Please use full sentences with periods and capital letters in comments. Comment on attachment 204061 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204061&action=review Is the purpose of this patch to more quickly prepare the texture in the case that the video is already in GPU memory? > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:356 > + if (meta) { > + const BitmapTextureGL* textureGL = static_cast<const BitmapTextureGL*>(m_texture.get()); > + guint ids[4] = { textureGL->id(), 0, 0, 0 }; > + > + ret = gst_video_gl_texture_upload_meta_upload(meta, ids); What's happening behind the scenes here? Is it just using the current OpenGL context? If not I wonder about the cost of context switching. In WebKit we do early returns, so you should write: if (GstVideoGLTextureUploadMeta* meta = gst_buffer_get_video_gl_texture_upload_meta(buffer)) { if (gst_video_gl_texture_upload_meta_upload(meta, ids)) { client()->setPlatformLayerNeedsDisplay(); return; } } The nice benefit being you don't have to touch the code below it. > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:362 > + { // copy the buffer into the texture Please use full sentences with periods and capital letters in comments. Created attachment 204151 [details]
Patch
This last version of the patch aims Martin's review. Thanks! Comment on attachment 204061 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204061&action=review >>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:356 >>> + ret = gst_video_gl_texture_upload_meta_upload(meta, ids); >> >> What's happening behind the scenes here? Is it just using the current OpenGL context? If not I wonder about the cost of context switching. >> >> In WebKit we do early returns, so you should write: >> >> if (GstVideoGLTextureUploadMeta* meta = gst_buffer_get_video_gl_texture_upload_meta(buffer)) { >> if (gst_video_gl_texture_upload_meta_upload(meta, ids)) { >> client()->setPlatformLayerNeedsDisplay(); >> return; >> } >> } >> >> The nice benefit being you don't have to touch the code below it. > > What's happening behind the scenes here? Is it just using the current OpenGL context? If not I wonder about the cost of context switching. > > In WebKit we do early returns, so you should write: > > if (GstVideoGLTextureUploadMeta* meta = gst_buffer_get_video_gl_texture_upload_meta(buffer)) { > if (gst_video_gl_texture_upload_meta_upload(meta, ids)) { > client()->setPlatformLayerNeedsDisplay(); > return; > } > } > > The nice benefit being you don't have to touch the code below it. Martin, Yes, it is using the current OpenGL context. Behind the scenes were a calling a this function vaCopySurfaceGLX: http://www.sourcecodebrowser.com/libva/1.0.4/va__glx_8h.html#ab3e822b68bbb2da0aadc25911ad42aae Created attachment 204424 [details]
Patch
Comment on attachment 204424 [details]
Patch
Looks good to me, but before landing would love to see a review by Philippe and/or slomo.
Comment on attachment 204424 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204424&action=review > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:353 > + if (gst_video_gl_texture_upload_meta_upload(meta, ids)) { This looks all good > Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:44 > +#define GST_SURFACE_CAPS "video/x-surface, opengl=true; " Erm... no. There should not be any video/x-surface caps in 1.0 anywhere. Maybe you mean "video/x-raw(meta:GstVideoGLTextureMapper)" here? (In reply to comment #29) > > Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:44 > > +#define GST_SURFACE_CAPS "video/x-surface, opengl=true; " > > Erm... no. There should not be any video/x-surface caps in 1.0 anywhere. Maybe you mean "video/x-raw(meta:GstVideoGLTextureMapper)" here? And for that you should use GST_VIDEO_CAPS_MAKE_WITH_FEATURES() and also specify that you want BGRA/BGRx. Thanks for the review Sebastian :) Usually though only "official reviewers" are allowed to r+/r-. Anyway, props for the valuable comments on the patch. Comment on attachment 204424 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204424&action=review >> Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:44 >> +#define GST_SURFACE_CAPS "video/x-surface, opengl=true; " > > Erm... no. There should not be any video/x-surface caps in 1.0 anywhere. Maybe you mean "video/x-raw(meta:GstVideoGLTextureMapper)" here? Currently only gstreamer-vaapi can offer the GstVideoGLTextureMapper meta, but it won't offer x-raw buffers, only the custom caps video/x-surface: https://bugzilla.gnome.org/show_bug.cgi?id=687183#c22 I can add both, otherwise this patch will be useless for a while. (In reply to comment #31) > Thanks for the review Sebastian :) Usually though only "official reviewers" are allowed to r+/r-. Anyway, props for the valuable comments on the patch. Ah, I assumed that it would be ok because I apparently had permissions for that :) Sorry I hope that doesn't mean that random people can also cq+? ;) (In reply to comment #32) > (From update of attachment 204424 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=204424&action=review > > >> Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:44 > >> +#define GST_SURFACE_CAPS "video/x-surface, opengl=true; " > > > > Erm... no. There should not be any video/x-surface caps in 1.0 anywhere. Maybe you mean "video/x-raw(meta:GstVideoGLTextureMapper)" here? > > Currently only gstreamer-vaapi can offer the GstVideoGLTextureMapper meta, but it won't offer x-raw buffers, only the custom caps video/x-surface: > https://bugzilla.gnome.org/show_bug.cgi?id=687183#c22 > > I can add both, otherwise this patch will be useless for a while. Can you add video/x-raw to gstreamer-vaapi instead please? We really don't want caps such as video/x-surface anymore in 1.0 and I don't think it's a good idea to lead by bad example here (In reply to comment #33) > (In reply to comment #31) > > Thanks for the review Sebastian :) Usually though only "official reviewers" are allowed to r+/r-. Anyway, props for the valuable comments on the patch. > > Ah, I assumed that it would be ok because I apparently had permissions for that :) Sorry > I hope that doesn't mean that random people can also cq+? ;) > People with EditFlags permissions can set whatever flag they want but if the patch is landed with the commit-queue the bot with check: - cq+ was set by a commiter - r+ was set by a reviewer If a patch was r+'ed with nitpicks the patch author can reupload the patch with updated Reviewed by field and set the cq flag only :) Created attachment 204835 [details]
Patch
Comment on attachment 204835 [details]
Patch
Looks good to me as long as it's okay for the GStreamer experts.
Comment on attachment 204835 [details] Patch Clearing flags on attachment: 204835 Committed r151673: <http://trac.webkit.org/changeset/151673> All reviewed patches have been landed. Closing bug. Yes looks good to me, a bit late comment but better late then never :) It seems some compositing video tests failing after the patch. Video missing or test timed out on Qt WK1: http://build.webkit.org/results/Qt%20Linux%20Release/r151676%20%2860915%29/results.html Qt WK2: http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20WebKit2/r151674%20%2836237%29/results.html Maybe EFL also affected, but I'm not sure because there are more problems: http://build.webkit.org/results/EFL%20Linux%2064-bit%20Release%20WK2/r151676%20%289072%29/results.html On GTK bot media test timing out massivly after r151673: http://build.webkit.org/results/GTK%20Linux%2064-bit%20Debug%20WK1/r151673%20%282631%29/results.html Can you check it, please? Victor is preparing a patch for this, it will be uploaded in this bug. Sorry for the breakage. Created attachment 204901 [details] Unreviewed, build fix after r151673 Created attachment 204902 [details] Fix after r151673 Reopening Comment on attachment 204902 [details] Fix after r151673 Clearing flags on attachment: 204902 Committed r151681: <http://trac.webkit.org/changeset/151681> All reviewed patches have been landed. Closing bug. This patch causes certain videos to stop rendering for me using GStreamer master on Ubuntu 12.04 / Intel i7 graphics. Here's an example video: https://www.dropbox.com/s/x03ogh3epbd6lfq/example.mkv The audio plays but the video doesn't (and playbin never emits "video-changed"). I get this warning: 0:00:00.142290527 29837 0x10da800 WARN webkitmediaplayer /home/blong/workspace/webkit/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:726:handleMessage: Warning 6: No decoder available for type 'video/x-theora, width=(int)320, height=(int)240, framerate=(fraction)30/1, streamheader=(buffer)< 807468656f72610302010014000f0001400000f000000000001e0000000100000100000100000000c0c0, 817468656f72612b000000586970682e4f7267206c69627468656f726120312e312032303039303832322028546875736e656c64612900000000, 827468656f7261becd28f7b9cd6b18b5a9494a10739ce6318c5294a42108318c62108421084000000000000000000011f4e1642e5549b47612f570b4986b95a2a9409648a1d047d399b8d66430174b2552912c94462210078391c0d06431168b05628130944621100783a1c0c8602c1509848220f0683014fb9917d69555541412d2d19190504f0f0dcdcc8c8b4b4b4a0a0a08c8c8c787878786464646450505050503c3c3c3c3c3c3c28282828282828281414141414141402100b0a101828333d0c0c0e131a3a3c370e0d1018283945380e11161d3357503e1216253a446d674d182337405168715c31404e5767797865485c5f62706467631112182f6363636312151a4263636363181a3863636363632f42636363636363636363636363636363636363636363636363636363636363636363636363636310101014181c2028101014181c2028301014181c2028304014181c2028304040181c2028304040401c20283040404060202830404040608028304040406080803e2fcb7d00d3313be428de0ea89bdb96622dd471be03f7289225ee0d8c97d6aa91a665762ec4204e460f68ea64be730dad586c655295e7dbb87e49dc434ccaec5d88217c8a3c355a836325e3f3977312ba3da3c9384fbaaa420c57354fbaaa457634119ca6941ed1e75dc90e0656b31b7ce1fc5785e0979dbfe138ed30f68f3a9226301756b3329a835df0c9690a3fb40bd0a78d8e65d91a077283da3ceb94db6bbd82b829546045f127fc5a2d63907dc221dcdaff710b48d8662cad01ba72572287bb79d497aaa4a84799c274ad66c171bc508aec0d240e4f143ddbaefffb263796c1483d9e05a759dc7162b3187c065472346a26964daffea52dcec8e2c3e0f93826964da3c16f019f5740b2b022d31e4caffea52dc84d68f5e76036a22ff81ec7cbe84dcc2767d2db056b8134b26ba6280f32777aaa440126bf5ee5ea7134bb506e759b0896a1b8595869fe28f23bf3855494a02e8ceb26251afc2cac5e4e4d2ed23cedc398843b8f7360ff4b6a29404573862f5aca2cac7922fedbb909b6a3ecc4661d09ffd325b0d452811c22cacdd79f6a4f33f6d42225cd8742d327ba705cc7c48e7ba8aa916562f943ef7c06d44ffebd33c23114832e18a6e6d171de9bc3005f5acd440750f0dbe5d50a715adcf393b447a4aa26923fcd32309f30be7576043f9d3b8b6c573e8053534b438728c2695ed1279ae461be60d5cc37b49caec637011edfb83b88133da2593ceaa539993e816b0dc079d9b05137b2c913586f6f395e2b33072ea3d325c7fc7c554ad61007d7b1809f26e35ed174e611656b980f095b144dec91e85acdff0aa914be2379dfe4ee334160aee6ec0352636a3d96489a745ac9cc2637d42aa465c9d6709fe370708c26286dafb2e9dd8aed20dcbe7093c8b0f841529237785a1fb149e56b1c4e3975121b9b8660d286dafb34b95ff0a5c4e3166d11232426976d6e39bcefdf07d6acac144387fb8cba82a15521db404d2ed26b47370ce1be01f5a7e7a8e2eac2b3cc276e658f029112806d34bac9a42bfa383dcbc5a61df23eb1598b38ff204f737a82caea53b84f7118266dacba49942e0bfdf07428cf0ed6fc726317f00aa9391264c31f2037707b15a3af90bb9c46500db59b4973bd77eb2b155242b5ecfb90387d18f177eb2b3a2f93b7230c41a36d66ce5519442aa5b80cda36d72c65bbe8481c3c2f60a26fc4562b49d9d2d7b3a3e441529703d188db564d33a30bfbb0dd0ab5a9ee2345d0f25904dffdc83879aee9ef3139226d443033ac594aac8b9a1d3b09a593d8ff3ec2d0f887ce18de65b9f9da3d799c595ae347488134bb49ed1814d46705a2d5f01f38fdbce068a09095b4dafbe6170238bbe72acb2a52c59ba843e65852a8fdd4300b98f0deb2cb4bc095336d64d23cf93c7689ed1387d752a8e5906895336d64f637f783b44a33f0eb0e16d85703731867103e7fca9a85a8671d9ed0331a4cdb5964c9d5f5962ef0e0c70451ee3727fd77282a290321b6a99a672d0c70edd4559621f8f119ec97b8b46e8568bd40c9a666dae2c269fb40aec29a845f1d49c8fc97dba379c0aa91d5fc2730c791f5a10d1dfe80668836d52269748f2f4b2b2ef3714ee79c2df808b286e80e578482695b6ba671e0fddb1a0b2b277bd54297c31241b6a95a4d9e5442ddc3af43f308d15fe2bb04fcad5dda0a86f529803b5d1705bd3ef4425e48666daa569364c70ffc394ea235958dea5287808fa122f60c590db54cd3397ff601ca75d416fd5bc588bb1e52a9c1217a56036d4ca6699e3727b4428b9807af947eb2b2377d6e3ca948b7e91cfa4085656460bc99934cd81b6b8ddff510ed3a5a38dc4672174c5e60cb134cdb6b45bec8aa0291d7762b4a4fffc42e226fe0a47b09a51d404ec383fcbe1bdc62cf9acb02224256d36b1f7e5eb6a15525c65df81182695b6ba499a2a2ff83f75eb8618a0dcb6c5643cce2aa42324253499b6b9d17b963810f9f03b6517f68f7ae3713dd62b800518dffa7e605350b57780906669336d65cb0891f7615a17bd1c7db8dce8626498d336dacaa30497a3e54d44079f56584fdbdb8b783f8a11b89e4f9cc4aa920427bfda5958efa8dc0501b6ac9a678b0997cb778371fa75c0d336da9850cb1be692340727b2fd0955222796d6b0efae04091c4a42e4ebfef72aa4b2b22139f0db550c2f5c658b4cfd99e5b183af78a553eb41401b6a95a4d2671e3c90df7207eed832884fe58af714d481c3a84789f957602f8eef96a8302412b498db5cf2c626ef5dfe29a877f3ab601e91c5cacacf37217e5e918311b6b29334cf2b89e8e02cac8ba854d411be5c5e194cd33946dae3848e56d101e6ef47f3f7c046e523e9fd5548dfc8084a3b88e2c5617255e34cdb6a61332c7fb7a0de2f416bffbbe3c2cac12ae1962d3336d5330be7a3949d102385552528cdddf51101238b0adc9285d330bff1b6a658b4cfe50b67f0f61a8aa940462cac71bf64ae44ca2736d5ffc32c5a665d6b0bfaf489e490881e5aa6a086e73f939812099a666dacb963dd857b8511a2bfe70ebd075fa72406da994cd33958e17dd5298078e7e14456ee58e3d56771c229a887d2de7c202350909434cdb6a61332c6fbb70ffbe2c564ff3a882300cb16999b6a9985f74bfa7fb9595a9a843e406e89228b5eb875d8f4e4a169fcc1962d3336d5730be7ef741514bc71611ab7497e428a41bc6490c440381d063833c8b675f7267c6ba3fdb951dfa595bc95552378c9218880703a0c7067916cebee4cf8d747fb72a3bf4b2b792aaa46f192431100e07418e0cf22d9d7dc99f1ae8ff6e5477e9656f255548aa9384fd4008978266999b6b2e5231c21f16562de5f7a2eec8e8ff705b1c35297ae5fd02cae03cdc2e4860c465334ccdb596f9f83bf917407f3f3e52a96560fa88bd6f0a038242f4a34cdb6a4ccb1c2eed1ade82cace56be9fc237093b6fe90213a3f4a184ccb15e699b6d6ea22535149e440fa7a3b71cfa15d91c037169f8cb16999330bcdb5ba5ea486a10a27ab2ba55201bc8f00f932fba75a35d1fec63833ca5e5db851d46408519204a2eb585e4ca273be9ffd62b06dab9e0cb1699efef912aa4aa940462cac71bf64ae44ca2736d5ffc32c5a665d6b0bfaf489e49080 >'. (url=file:///home/blong/Dropbox/Public/example.mkv) But the video works fine if I do: gst-git gst-launch-1.0 playbin uri=file:///home/blong/Dropbox/Public/example.mkv (In reply to comment #48) > I get this warning: > > 0:00:00.142290527 29837 0x10da800 WARN webkitmediaplayer /home/blong/workspace/webkit/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:726:handleMessage: Warning 6: No decoder available for type 'video/x-theora, width=(int)320, height=(int)240, Do you have theoradec ? I wonder why it's being plugged for a mkv file. (In reply to comment #49) > Do you have theoradec ? I wonder why it's being plugged for a mkv file. Yes, I can gst-inspect-1.0 theoradec, and I can play that file with gst-launch-1.0 playbin uri=file:///path/to/file, plus it works if I build WebKit before these commits. The file was created with GStreamer using something like this: gst-launch-1.0 -e videotestsrc ! theoraenc ! matroskamux name=mux ! filesink location=example.mkv videotestsrc pattern=checkers-8 ! theoraenc ! mux. audiotestsrc ! vorbisenc ! mux. audiotestsrc wave=white-noise ! vorbisenc ! mux. The purpose is to have a file with multiple audio and video streams. So, the streams are theora. (In reply to comment #47) > This patch causes certain videos to stop rendering for me using GStreamer master on Ubuntu 12.04 / Intel i7 graphics. Here's an example video: > > https://www.dropbox.com/s/x03ogh3epbd6lfq/example.mkv > > The audio plays but the video doesn't (and playbin never emits "video-changed"). I observe the same behavior in my setup. As if the video pad never got negotiated. Can someone get a GStreamer debug log of this? Created attachment 205797 [details]
gstreamer log
It looks to me that the caps negotiation is not done correctly, since the webkit video sink never get _setcaps() called.
Uh, yes that looks like a really bad bug in playbin in 1.1.1/git master: ed video/x-raw(meta:GstVideoGLTextureUploadMeta), format=(string){ BGRx, BGRA }, width=(int)[ 1, 2147483647 ], height=(int)[ 1, 2147483647 ], framerate=(fraction)[ 0/1, 2147483647/1 ]; video/x-surface, opengl=(boolean)true; video/x-raw, format=(string){ BGRx, BGRA }, width=(int)[ 1, 2147483647 ], height=(int)[ 1, 2147483647 ], framerate=(fraction)[ 0/1, 2147483647/1 ] 0:00:02.849152483 12953 0xae4022c0 DEBUG playbin gstplaybin2.c:4302:autoplug_select_cb:<play> theoradec not compatible with the fixed sink (In reply to comment #54) > Uh, yes that looks like a really bad bug in playbin in 1.1.1/git master: > > ed video/x-raw(meta:GstVideoGLTextureUploadMeta), format=(string){ BGRx, BGRA }, width=(int)[ 1, 2147483647 ], height=(int)[ 1, 2147483647 ], framerate=(fraction)[ 0/1, 2147483647/1 ]; video/x-surface, opengl=(boolean)true; video/x-raw, format=(string){ BGRx, BGRA }, width=(int)[ 1, 2147483647 ], height=(int)[ 1, 2147483647 ], framerate=(fraction)[ 0/1, 2147483647/1 ] > 0:00:02.849152483 12953 0xae4022c0 DEBUG playbin gstplaybin2.c:4302:autoplug_select_cb:<play> theoradec not compatible with the fixed sink Also why does webkitvideosink still return video/x-surface? That should not be... This probably fixes it: https://bugs.webkit.org/show_bug.cgi?id=116042 However really make sure that video/x-surface disappears from webkitvideosink. That's not how GStreamer is supposed to be used... This probably fixes it: http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=f39d1dc3b48fad58668488ab985f10cca26478c7 However really make sure that video/x-surface disappears from webkitvideosink. That's not how GStreamer is supposed to be used... (In reply to comment #57) > This probably fixes it: http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=f39d1dc3b48fad58668488ab985f10cca26478c7 > > > However really make sure that video/x-surface disappears from webkitvideosink. That's not how GStreamer is supposed to be used... Yeah, I know, but I have it temporally for testing purposes. (In reply to comment #57) > This probably fixes it: http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=f39d1dc3b48fad58668488ab985f10cca26478c7 > I've verified the fix: it works, thanks slomo! (In reply to comment #57) Works for me too. Thanks! |