Bug 229145 – [GStreamer][EME] Try to parse XML init datas that could possibly come from MPD manifests

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

[Xabier Rodríguez Calvar]

[GStreamer][EME] Try to parse XML init datas that could possibly come from MPD manifests

[Xabier Rodríguez Calvar]

Created attachment 435610 [details]
Patch

[Philippe Normand]

Comment on attachment 435610 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/eme/GStreamerEMEUtilities.cpp:81
> +    if (g_markup_parse_context_parse(markupParseContext.get(), payload->dataAsCharPtr(), payload->size(), &error.outPtr()) && userData.pssh)

g_warning() mentioning the error message if this fails?

[Alicia Boya García]

Comment on attachment 435610 [details]
Patch

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

LGTM.

>> Source/WebCore/platform/graphics/gstreamer/eme/GStreamerEMEUtilities.cpp:81
>> +    if (g_markup_parse_context_parse(markupParseContext.get(), payload->dataAsCharPtr(), payload->size(), &error.outPtr()) && userData.pssh)
> 
> g_warning() mentioning the error message if this fails?

It is expected to fail in the case it's not an XML, right? I would emit a warning when an XML is found but no pssh was parsed though.

[Philippe Normand]

Comment on attachment 435610 [details]
Patch

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

>>> Source/WebCore/platform/graphics/gstreamer/eme/GStreamerEMEUtilities.cpp:81
>>> +    if (g_markup_parse_context_parse(markupParseContext.get(), payload->dataAsCharPtr(), payload->size(), &error.outPtr()) && userData.pssh)
>> 
>> g_warning() mentioning the error message if this fails?
> 
> It is expected to fail in the case it's not an XML, right? I would emit a warning when an XML is found but no pssh was parsed though.

Then you can omit the GError, use nullptr. If we pass a GError here and the call fails, we should warn about it.

[Xabier Rodríguez Calvar]

Created attachment 435674 [details]
Patch

[Xabier Rodríguez Calvar]

Created attachment 435683 [details]
Patch

[EWS]

Alicia Boya García found in /Volumes/Data/worker/Commit-Queue/build/Source/WTF/ChangeLog does not appear to be a valid reviewer according to contributors.json.
/Volumes/Data/worker/Commit-Queue/build/Source/WTF/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

[Xabier Rodríguez Calvar]

Created attachment 435751 [details]
Patch for landing

[EWS]

Committed r281183 (240628@main): <https://commits.webkit.org/240628@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/82066595>

[Michael Catanzaro]

(In reply to EWS from comment #7)
> Alicia Boya García

Huh, this must be caused by the í in your Bugzilla name. Alica, you could probably either:

 * rename yourself in contributors.json from Garcia to García
 * rename yourself in Bugzilla from García to Garcia

[Alicia Boya García]

(In reply to Michael Catanzaro from comment #11)
> (In reply to EWS from comment #7)
> > Alicia Boya García
> 
> Huh, this must be caused by the í in your Bugzilla name. Alica, you could
> probably either:
> 
>  * rename yourself in contributors.json from Garcia to García
>  * rename yourself in Bugzilla from García to Garcia

This is not the first patch I review. This has occurred because when calvaris send a revision of the patch with the "Reviewed by Alicia" string (since a previous patch already got r+), he used the tilde, which doesn't match contributors.json (or previous strings for that matter).

There is no reason to change my entry in contributors.json at this point, especially given how WebKit's Bugzilla still struggles with non-ASCII text, just amending the "Reviewed by" string as he did is enough.