[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 #22 from Xabier Rodríguez Calvar <calvaris at igalia.com> ---
(In reply to Alicia Boya García from comment #21)
> >> 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?

I don't think so.

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

In this patch it was probably still necessary if you're only splitting by the ,. The result it would probably contain white spaces. Though this is irrelevant for changes we need to do.

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

Considering what you mention in your first comment that applies to the second as well, maybe the approach should be creating a supportsContentType and a supportsCodec that checks only one codec. That way you could use the first in the last case and the second in the first two cases.

-- 
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/21c6df7c/attachment.html>