[webkit-reviews] review granted: [Bug 195948] [GStreamer] Switch back to webkitwebsrc for adaptive streaming fragments downloading : [Attachment 365172] Patch

[bugzilla-daemon at webkit.org]

Xabier Rodríguez Calvar <calvaris at igalia.com> has granted  review:
Bug 195948: [GStreamer] Switch back to webkitwebsrc for adaptive streaming
fragments downloading
https://bugs.webkit.org/show_bug.cgi?id=195948

Attachment 365172: Patch

https://bugs.webkit.org/attachment.cgi?id=365172&action=review




--- Comment #4 from Xabier Rodríguez Calvar <calvaris at igalia.com> ---
Comment on attachment 365172
  --> https://bugs.webkit.org/attachment.cgi?id=365172
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=365172&action=review

>>>
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
346
>>> +	 }
>> 
>> This code is ok but unnecessary here.
>> 
>> GstContext is meant to the set on pipelines so that it is the pipeline that
dispatches the context to the elements when they are created and added to it.
>> 
>> The pipeline cannot exist without the player private,  the private creates
the pipeline and the player is (or should be) set to the private prior to that.
So when the pipeline is created, we can set the context to it immediately (fire
and forget!). Once the elements are created and added to it, the context should
be immediately available.
>> 
>> Summing up, IMHO you should neither listen to this message nor send it (read
my comment below) and just set the context to the pipeline when the pipeline is
created.
> 
> This code is needed because secondary webkitwebsrc elements created by
adaptivedemux/uridownloader have no relationship whatsoever with the first
webkitwebsrc that was used to download the manifest. Anyway, I tried what you
suggest, set the context on the whole pipeline when it was created, and
secondary webkitwebsrc element now crashes because of the RELEASE_ASSERT I
added in webKitWebSrcStart(), which means the element context is not set early
enough.

I understand, then we're cool regarding the messages, queries and context.
Please fix the other minor things and we're good to go