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

No, it's not. ContentType::codecs() already parses them properly.


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

Okay, I'm fine with that.

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