[Webkit-unassigned] [Bug 181990] [EME][GStreamer] Add support for the encrypted Caps in GStreamerUtilities

[bugzilla-daemon at webkit.org]

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

--- Comment #14 from Xabier Rodríguez Calvar <calvaris at igalia.com> ---
(In reply to Yacine Bandou from comment #12)
> > > Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp:70
> > > +        gint width = 0, height = 0;
> > 
> > gint -> int and variables need to be moved to just before where they are
> > used.
> >
> 
> why it needs to be moved ? it's a code style or it's an optimization?

It's coding style. We try to keep it like this as thoroughly as possible though it is true that in some cases, there's still code like this and even reviewers don't realized of this issues when reviewing. Anyway, all new code should respect the coding style and yes, this is part of it.

> > > Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp:76
> > > +        gst_structure_get_fraction(structure, "pixel-aspect-ratio", &pixelAspectRatioNumerator, &pixelAspectRatioDenominator);
> > 
> > Assign these two variables to 1 before calling this.
> 
> I prefer do it when "gst_structure_get_fraction" return false. 

Ok.

> > > Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp:169
> > > +const char* getMediaTypeFromCaps(GstCaps* caps)
> > 
> > getMediaTypeFromCaps -> capsMediaType and make it tame const caps
> > 
> > > Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp:176
> > > +    if (!structure) {
> > > +        GST_ERROR("caps are empty");
> > > +        return nullptr;
> > > +    }
> > 
> > Did you ever find any case where this error was hit? I doubt it, so I think
> > it is better to just ASSERT on this. If this is something that really can
> > happen sometimes, we should not just ERROR, it looks the kind of situation
> > that should be managed by the caller instead, so we could WARNING here.
> > 
> 
> Yes, the caps can be empty.
> I agree, I should use WARNING instead of ERROR.

Ok.

-- 
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/20180209/5900d350/attachment.html>