Bug 116042 – [GStreamer] [texmap] upload a video buffer into the video texture

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Víctor M. Jáquez L.]

In bug #86410, composited video support, using the TexturMapper, was added. But it doesn't handle when the video is handled by Accelerated Hardware decoders.

Is until GStreamer 1.2 when we have official support to upload a buffer into a texture without changing from GPU context to the CPU one, hence webkit would play fullHD videos.

[Víctor M. Jáquez L.]

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.

[WebKit Commit Bot]

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.

[EFL EWS Bot]

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

[Early Warning System Bot]

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

[EFL EWS Bot]

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

[Early Warning System Bot]

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

[Víctor M. Jáquez L.]

Created attachment 204047 [details]
Patch

[Víctor M. Jáquez L.]

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.

[Early Warning System Bot]

Comment on attachment 204047 [details]
Patch

Attachment 204047 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/777474

[EFL EWS Bot]

Comment on attachment 204047 [details]
Patch

Attachment 204047 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/793136

[Early Warning System Bot]

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

[EFL EWS Bot]

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

[Víctor M. Jáquez L.]

Yeah, if we go supporting the deprecated GstSurfaceMeta, I also need to modify the cmake scripts...

[kov's GTK+ EWS bot]

Comment on attachment 204047 [details]
Patch

Attachment 204047 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/793139

[Philippe Normand]

(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!

[Víctor M. Jáquez L.]

Created attachment 204060 [details]
Patch

[EFL EWS Bot]

Comment on attachment 204060 [details]
Patch

Attachment 204060 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/709943

[Early Warning System Bot]

Comment on attachment 204060 [details]
Patch

Attachment 204060 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/784156

[Early Warning System Bot]

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

[EFL EWS Bot]

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

[Víctor M. Jáquez L.]

Created attachment 204061 [details]
Patch

[Martin Robinson]

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.

[Martin Robinson]

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.

[Víctor M. Jáquez L.]

Created attachment 204151 [details]
Patch

[Víctor M. Jáquez L.]

This last version of the patch aims Martin's review. Thanks!

[Víctor M. Jáquez L.]

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

[Víctor M. Jáquez L.]

Created attachment 204424 [details]
Patch

[Martin Robinson]

Comment on attachment 204424 [details]
Patch

Looks good to me, but before landing would love to see a review by Philippe and/or slomo.

[Sebastian Dröge (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?

[Sebastian Dröge (slomo)]

(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.

[Philippe Normand]

Thanks for the review Sebastian :) Usually though only "official reviewers" are allowed to r+/r-. Anyway, props for the valuable comments on the patch.

[Víctor M. Jáquez L.]

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.

[Sebastian Dröge (slomo)]

(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

[Philippe Normand]

(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 :)

[Víctor M. Jáquez L.]

Created attachment 204835 [details]
Patch

[Martin Robinson]

Comment on attachment 204835 [details]
Patch

Looks good to me as long as it's okay for the GStreamer experts.

[WebKit Commit Bot]

Comment on attachment 204835 [details]
Patch

Clearing flags on attachment: 204835

Committed r151673: <http://trac.webkit.org/changeset/151673>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Sebastian Dröge (slomo)]

Yes looks good to me, a bit late comment but better late then never :)

[Zoltan Arvai]

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?

[Philippe Normand]

Victor is preparing a patch for this, it will be uploaded in this bug. Sorry for the breakage.

[Víctor M. Jáquez L.]

Created attachment 204901 [details]
Unreviewed, build fix after r151673

[Víctor M. Jáquez L.]

Created attachment 204902 [details]
Fix after r151673

[Philippe Normand]

Reopening

[WebKit Commit Bot]

Comment on attachment 204902 [details]
Fix after r151673

Clearing flags on attachment: 204902

Committed r151681: <http://trac.webkit.org/changeset/151681>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Brendan Long]

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").

[Brendan Long]

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

[Philippe Normand]

(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.

[Brendan Long]

(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.

[Víctor M. Jáquez L.]

(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.

[Sebastian Dröge (slomo)]

Can someone get a GStreamer debug log of this?

[Víctor M. Jáquez L.]

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.

[Sebastian Dröge (slomo)]

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

[Sebastian Dröge (slomo)]

(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...

[Sebastian Dröge (slomo)]

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...

[Sebastian Dröge (slomo)]

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...

[Víctor M. Jáquez L.]

(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.

[Víctor M. Jáquez L.]

(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!

[Brendan Long]

(In reply to comment #57)

Works for me too. Thanks!