[Webkit-unassigned] [Bug 178160] [GStreamer][MSE] Trim space between codecs

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=178160

--- Comment #21 from Alicia Boya García <aboya at igalia.com> ---
Comment on attachment 324667
  --> https://bugs.webkit.org/attachment.cgi?id=324667
Patch

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

>> Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp:602
>> +        if (!MediaPlayerPrivateGStreamerMSE::supportsCodecs({ originalMediaType })) {
> 
> If you keep the original signature + the overload, you don't need this.

Can a media type have two codecs somehow?

>> Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp:844
>> +        codec = codec.stripWhiteSpace();
> 
> I think this won't be needed if you take const ContentType& here because that logic seems to be in that class.

yep, I forgot to remove that.

>> Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.h:86
>> +    static bool supportsCodecs(const Vector<String>& codecs);
> 
> I think this function signature should left unchanged and create an overloaded that takes a const ContentType&. The original function should create the ContentType from a String and pass it to the overloaded function.

I'd rather not unless proven necessary. Overloads are confusing (at least compared to not having them) for people reading the code, so they should provide a clear benefit in exchange.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20171026/cafaeb2c/attachment.html>