ASSERTION FAILED: !m_adoptionIsRequired 0x00007fffed2980a8 in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:321 321 *(int *)(uintptr_t)0xbbadbeef = 0; (gdb) bt #0 0x00007fffed2980a8 in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:321 #1 0x00007ffff272740e in WTF::RefCountedBase::ref (this=0x7fff52867e78) at ../../Source/WTF/wtf/RefCounted.h:45 #2 0x00007ffff3eeabdb in WTF::refIfNotNull<WebCore::SourceBufferPrivateGStreamer> (ptr=0x7fff52867e70) at ../../Source/WTF/wtf/PassRefPtr.h:36 #3 0x00007ffff3eea93f in WTF::RefPtr<WebCore::SourceBufferPrivateGStreamer>::RefPtr (this=0x7fffffffc1e0, ptr=0x7fff52867e70) at ../../Source/WTF/wtf/RefPtr.h:43 #4 0x00007ffff3eea235 in WebCore::MediaSourceGStreamer::addSourceBuffer (this=0x7fff5283cc80, contentType=..., sourceBufferPrivate=...) at ../../Source/WebCore/platform/graphics/gstreamer/MediaSourceGStreamer.cpp:66 #5 0x00007ffff2c206d8 in WebCore::MediaSource::createSourceBufferPrivate (this=0x7fff527c4c60, type=..., ec=@0x7fffffffc39c: 0) at ../../Source/WebCore/Modules/mediasource/MediaSource.cpp:850 #6 0x00007ffff2c1f534 in WebCore::MediaSource::addSourceBuffer (this=0x7fff527c4c60, type="audio/mp4; codecs=\"mp4a.40.2\"", ec=@0x7fffffffc39c: 0) at ../../Source/WebCore/Modules/mediasource/MediaSource.cpp:541 #7 0x00007ffff3fd07e7 in WebCore::jsMediaSourcePrototypeFunctionAddSourceBuffer (exec=0x7fffffffc420) at DerivedSources/WebCore/JSMediaSource.cpp:304 #8 0x00007fff9b2e80b4 in ?? () #9 0x00007fffffffc490 in ?? () #10 0x00007fffed2403a7 in llint_entry () from /home/phil/WebKit/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18 Backtrace stopped: frame did not save the PC
Sebastian fixed in his patch for Bug 140078, I'll upload the fix based on his patch here.
Created attachment 244045 [details] patch
Created attachment 244046 [details] patch
Seems good to me obviously ;) Thanks for extracting that part from my huge patch!
Created attachment 244054 [details] patch
Comment on attachment 244054 [details] patch oops this is incomplete.
Created attachment 244057 [details] patch
Attachment 244057 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/gstreamer/MediaSourceGStreamer.cpp:40: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 244058 [details] patch
Comment on attachment 244058 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=244058&action=review > Source/WebCore/platform/graphics/gstreamer/MediaSourceGStreamer.cpp:65 > + for (auto it = m_sourceBuffers.begin(), end = m_sourceBuffers.end(); it != end; ++it) > + (*it)->clearMediaSource(); This could be for (auto& sourceBuffer : m_sourceBuffers) sourceBuffer.clearMediaSource(); > Source/WebCore/platform/graphics/gstreamer/MediaSourceGStreamer.cpp:79 > + return m_client->addSourceBuffer(m_sourceBuffers.last(), contentType); You could use sourceBufferPrivate.get() here instead of calling last() again. > Source/WebCore/platform/graphics/gstreamer/MediaSourceGStreamer.cpp:87 > + size_t index = m_sourceBuffers.find(buffer); > + m_sourceBuffers.remove(index); Shouldn't you also call clearMediaSource() here? Since it's removed from the vector here, it won't be called for this source bufer in the destructor. Since the order doesn't really matter, and we are always adding a new SourceBufferPrivateGStreamer, I wonder whether we could use a HashSet instead, and here you would just take the buffer can call clearMediaSource (if it's actually needed) > Source/WebCore/platform/graphics/gstreamer/MediaSourceGStreamer.h:50 > + MediaSourceClientGStreamer* client() const { return m_client.get(); } Since m_client can't be null, you could return a reference instead.
Comment on attachment 244058 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=244058&action=review > Source/WebCore/platform/graphics/gstreamer/SourceBufferPrivateGStreamer.cpp:68 > + SourceBufferPrivateClient::AppendResult result = m_mediaSource->client()->append(this, data, length); Not in the scope of this bug, but MediaSourceClientGStreamer::append() doesn't have to operate on a PassRefPtr<SourceBufferPrivate> -- a simple reference to the SourceBufferPrivate object would do. > Source/WebCore/platform/graphics/gstreamer/SourceBufferPrivateGStreamer.cpp:85 > + m_mediaSource->client()->removedFromMediaSource(this); Same here, MediaSourceClientGStreamer::removedFromMediaSource() could operate with a reference to the SourceBufferPrivate object.
(In reply to comment #11) > Comment on attachment 244058 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=244058&action=review > > > Source/WebCore/platform/graphics/gstreamer/SourceBufferPrivateGStreamer.cpp:68 > > + SourceBufferPrivateClient::AppendResult result = m_mediaSource->client()->append(this, data, length); > > Not in the scope of this bug, but MediaSourceClientGStreamer::append() > doesn't have to operate on a PassRefPtr<SourceBufferPrivate> -- a simple > reference to the SourceBufferPrivate object would do. > > > Source/WebCore/platform/graphics/gstreamer/SourceBufferPrivateGStreamer.cpp:85 > > + m_mediaSource->client()->removedFromMediaSource(this); > > Same here, MediaSourceClientGStreamer::removedFromMediaSource() could > operate with a reference to the SourceBufferPrivate object. Right, I think I'll fix this in a separate patch. Thanks for pointing it out!
Created attachment 244262 [details] patch
Comment on attachment 244262 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=244262&action=review > Source/WebCore/platform/graphics/gstreamer/MediaSourceGStreamer.cpp:79 > + RefPtr<SourceBufferPrivateGStreamer> buffer = SourceBufferPrivateGStreamer::create(this); > + m_sourceBuffers.add(buffer.get()); > + sourceBufferPrivate = buffer; I'm confused about the ownership now. In the previous patch you kept a reference in the Vector, why are you using raw pointers now? Is removeSourceBuffer always called before the SourceBufferPrivate object is deleted? > Source/WebCore/platform/graphics/gstreamer/SourceBufferPrivateGStreamer.cpp:86 > + m_mediaSource->removeSourceBuffer(this); > + m_mediaSource->client().removedFromMediaSource(this); hmm, now that I see this. It looks a bit weird to me that MediaSourceGStreamer::addSourceBuffer() calls MediaSourceClientGStreamer::addSourceBuffer() but removedFromMediaSource is called here instead of in MediaSourceGStreamer::removeSourceBuffer(). Also why does MediaSourceClientGStreamer::removedFromMediaSource() receive a PassRefPtr?
This likely isn't going to apply to the reworked MSE backend.