[webkit-reviews] review granted: [Bug 210341] [GStreamer][MSE] Implement the "sequence" mode in SourceBuffer for the GStreamer ports : [Attachment 436307] Patch

[bugzilla-daemon at webkit.org]

Alicia Boya García <aboya at igalia.com> has granted Enrique Ocaña
<eocanha at igalia.com>'s request for review:
Bug 210341: [GStreamer][MSE] Implement the "sequence" mode in SourceBuffer for
the GStreamer ports
https://bugs.webkit.org/show_bug.cgi?id=210341

Attachment 436307: Patch

https://bugs.webkit.org/attachment.cgi?id=436307&action=review




--- Comment #17 from Alicia Boya García <aboya at igalia.com> ---
Comment on attachment 436307
  --> https://bugs.webkit.org/attachment.cgi?id=436307
Patch

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

> Source/WebCore/ChangeLog:12
> +	   A parser element is needed to process the data (mpegaudioparse for
mpeg layer 1,

MPEG Audio Version 1. MPEG version 1 includes layer 1, layer 2 and layer 3, the
last one most commonly known as MP3.

> Source/WebCore/ChangeLog:13
> +	   aacparse for mpeg layer 3 and 4). The existing
createOptionalParserForFormat()

MPEG Audio Versions 2 and 4, both commonly referred to as AAC. There is no
concept of layers in MPEG Audio Versions 2 and 4, instead profiles are used.

Yes, MPEG nomenclature is quite confusing.

> Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.cpp:199
> +void MediaSampleGStreamer::setTimestamps(const MediaTime& presentationTime,
const MediaTime& decodeTime)
> +{
> +    m_pts = presentationTime;
> +    m_dts = decodeTime;
> +}

You're updating the timestamps in WebKit but not in GStreamer, which means they
will played at the wrong time in the playback pipeline.

> Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp:182
> +		   GRefPtr<GstCaps> caps = gst_pad_get_current_caps(pad);
> +		   if (!caps)
> +		       return GST_PAD_PROBE_DROP;

In the hypothetical case that a segment is sent before caps, it would be lost,
which could confuse downstream. Maybe it makes more sense to drop any buffers
before caps are received, but not anything else.