Bug 162893 - [GStreamer] GStreamer Media Description
Summary: [GStreamer] GStreamer Media Description
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Enrique Ocaña
URL:
Keywords:
Depends on:
Blocks: 157314
  Show dependency treegraph
 
Reported: 2016-10-04 03:16 PDT by Enrique Ocaña
Modified: 2016-10-26 01:41 PDT (History)
1 user (show)

See Also:


Attachments
Patch (6.25 KB, patch)
2016-10-04 03:21 PDT, Enrique Ocaña
no flags Details | Formatted Diff | Diff
Patch (6.19 KB, patch)
2016-10-16 12:03 PDT, Enrique Ocaña
no flags Details | Formatted Diff | Diff
Patch (6.13 KB, patch)
2016-10-20 13:54 PDT, Enrique Ocaña
no flags Details | Formatted Diff | Diff
Patch (6.09 KB, patch)
2016-10-21 14:23 PDT, Enrique Ocaña
no flags Details | Formatted Diff | Diff
Patch (6.14 KB, patch)
2016-10-26 01:18 PDT, Enrique Ocaña
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.