Bug 222108 – [GStreamer] Error instead of asserting on the player in the source

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Xabier Rodríguez Calvar]

[GStreamer] Error instead of asserting on the player in the source

[Xabier Rodríguez Calvar]

Created attachment 420833 [details]
Patch

[Philippe Normand]

Comment on attachment 420833 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        The release assert can be hit in some cases in smoothstreaming
> +        with already invalid pipelines so better to just error out instead
> +        of assert on release.

I don't disagree about this patch, but I would like to know more about these invalid pipelines :)

[Alicia Boya García]

Do you have a test vector that reproduces the failed assertion?

[Xabier Rodríguez Calvar]

It's a DRM test that plays, ends, loops, the DRM systems fails and then sometimes you get the error.

[Alicia Boya García]

Comment on attachment 420833 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:484
> +        GST_ERROR_OBJECT(src, "player should not be null");

I would word it better: "Couldn't obtain WebKitWebSrcPlayerContext, which is necessary to make network requests."

> Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:485
> +        ASSERT_NOT_REACHED();

I would not assert here. While reaching this case is undesirable, we're already handling the error by returning GST_FLOW_ERROR. The cause of not being able to get a player is most likely a preceding error, so crashing here doesn't help gather valuable information. Also I don't think Release and Debug should get different treatment in this case.

[Alicia Boya García]

Informal r+ with these changes

[Xabier Rodríguez Calvar]

Created attachment 421933 [details]
Patch

[EWS]

Committed r273731: <https://commits.webkit.org/r273731>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 421933 [details].

[Radar WebKit Bug Importer]

<rdar://problem/74930203>