The attached patch adds initial experimental support for webm in WebKitGTK+ with VP8, VP9, Vorbis and Opus formats, as long as suitable plugins are installed in GStreamer.
Created attachment 321540 [details] Patch
Comment on attachment 321540 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=321540&action=review > Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp:143 > + else > + ASSERT_NOT_REACHED(); This is the only concern I have. I suppose AppendPipeline wouldn't get created if the type doesn't match one of the two conditions?
Indeed. The process is stopped much earlier in the call stack, in MediaSource::addSourceBuffer()
Comment on attachment 321540 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=321540&action=review >> Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp:143 >> + ASSERT_NOT_REACHED(); > > This is the only concern I have. I suppose AppendPipeline wouldn't get created if the type doesn't match one of the two conditions? A test here would be interesting if there is none. Feel free to submit it to the spec tests if there is none about this yet. If it is not accepted because of whatever, I think it still pays off to keep it as WebKit test. I guess the test would consist of creating a SourceBuffer with crap as type, try appending some allowed format and try to play it, expecting an error. If the test fails already without calling this constructor, then we are good to go with the ASSERT_NOT_REACHED. If not, I agree with Žan here. I think the proper thing to do here would be to setting the AppendPipeline to Invalid state. Then I see another problem coming and it is that apparently all state transitions from Invalid are "ok" and I think it shouldn't be like that. Only Invalid -> Invalid should be allowed. Everything else should be ok = false. I guess then we would have some ASSERTs in Debug mode that we'd have to fix in cases where somebody appends something to a pipeline in Invalid state. Am I missing anything?
(In reply to Xabier Rodríguez Calvar from comment #4) > A test here would be interesting if there is none. There is already a test for that: imported/w3c/web-platform-tests/media-source/mediasource-addsourcebuffer.html If that did not work as expected it would crash but that's not the case.
Comment on attachment 321540 [details] Patch (In reply to Alicia Boya García from comment #5) > There is already a test for that: > imported/w3c/web-platform-tests/media-source/mediasource-addsourcebuffer.html > > If that did not work as expected it would crash but that's not the case. Good, you're landing then.
Comment on attachment 321540 [details] Patch Clearing flags on attachment: 321540 Committed r222550: <http://trac.webkit.org/changeset/222550>
All reviewed patches have been landed. Closing bug.