Bug 162893

Summary: [GStreamer] GStreamer Media Description
Product: WebKit Reporter: Enrique Ocaña <eocanha>
Component: PlatformAssignee: Enrique Ocaña <eocanha>
Status: RESOLVED FIXED    
Severity: Normal CC: calvaris
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 157314    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Enrique Ocaña 2016-10-04 03:16:35 PDT
Implement the MediaDescription interface for the GStreamer platform.
Comment 1 Enrique Ocaña 2016-10-04 03:21:59 PDT
Created attachment 290586 [details]
Patch
Comment 2 Enrique Ocaña 2016-10-04 07:06:08 PDT
Comment on attachment 290586 [details]
Patch

Wait until all the patches in 157314 are ready.
Comment 3 Enrique Ocaña 2016-10-16 12:03:38 PDT
Created attachment 291758 [details]
Patch
Comment 4 Zan Dobersek 2016-10-18 01:12:08 PDT
Comment on attachment 291758 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/mse/GStreamerMediaDescription.cpp:49
> +    AtomicString simpleCodecName(codecName);
> +
> +    return simpleCodecName;

Just return `codecName`, it should be converted into AtomicString implicitly.

> Source/WebCore/platform/graphics/gstreamer/mse/GStreamerMediaDescription.h:34
> +private:
> +    GRefPtr<GstCaps> m_caps;

The public section is always first here. This should be moved below where currently the constructor is.

> Source/WebCore/platform/graphics/gstreamer/mse/GStreamerMediaDescription.h:39
> +    static PassRefPtr<GStreamerMediaDescription> create(GstCaps* caps)
> +    {
> +        return adoptRef(new GStreamerMediaDescription(caps));
> +    }

This should return Ref<GStreamerMediaDescription>.

> Source/WebCore/platform/graphics/gstreamer/mse/GStreamerMediaDescription.h:41
> +    virtual ~GStreamerMediaDescription() { }

Default the constructor:

    virtual ~GStreamerMediaDescription() = default;

It should probably be placed in the implementation file.

> Source/WebCore/platform/graphics/gstreamer/mse/GStreamerMediaDescription.h:53
> +    GStreamerMediaDescription(GstCaps* caps)
> +        : MediaDescription()
> +        , m_caps(caps)
> +    {
> +    }

This too could be moved into the implementation file. I don't think there's much profit in defining it in the header and having it inlined by the compiler.
Comment 5 Enrique Ocaña 2016-10-20 13:54:07 PDT
Created attachment 292249 [details]
Patch
Comment 6 Enrique Ocaña 2016-10-21 14:23:03 PDT
Created attachment 292404 [details]
Patch
Comment 7 Enrique Ocaña 2016-10-26 01:18:52 PDT
Created attachment 292889 [details]
Patch
Comment 8 Enrique Ocaña 2016-10-26 01:41:23 PDT
Comment on attachment 292889 [details]
Patch

Clearing flags on attachment: 292889

Committed r207876: <http://trac.webkit.org/changeset/207876>
Comment 9 Enrique Ocaña 2016-10-26 01:41:31 PDT
All reviewed patches have been landed.  Closing bug.