Bug 217750 – [GStreamer] Encoder probing support for the registry scanner

• Component: Platform
• Status: RESOLVED FIXED

[Philippe Normand]

This would be useful for the mediacapabilities spec implementation. Currently we probe only for decoders.

[Philippe Normand]

Created attachment 411427 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 411427 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/GStreamerRegistryScanner.cpp:175
> +        Optional<HashSet<String, ASCIICaseInsensitiveHash>> mimeTypeSet;
> +        Optional<HashMap<AtomString, bool>> codecMap;

Why do you need to make this Optional if you're not checking if the value is there?

> Source/WebCore/platform/graphics/gstreamer/GStreamerRegistryScanner.h:47
> +    HashSet<String, ASCIICaseInsensitiveHash>& mimeTypeSet(Configuration);

Can't you return a const here?

[Philippe Normand]

Comment on attachment 411427 [details]
Patch

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

>> Source/WebCore/platform/graphics/gstreamer/GStreamerRegistryScanner.cpp:175
>> +        Optional<HashMap<AtomString, bool>> codecMap;
> 
> Why do you need to make this Optional if you're not checking if the value is there?

No need indeed!

>> Source/WebCore/platform/graphics/gstreamer/GStreamerRegistryScanner.h:47
>> +    HashSet<String, ASCIICaseInsensitiveHash>& mimeTypeSet(Configuration);
> 
> Can't you return a const here?

Yes :)

[Philippe Normand]

Committed r268576: <https://trac.webkit.org/changeset/268576>

[Radar WebKit Bug Importer]

<rdar://problem/70370485>

[Philippe Normand]

Comment on attachment 411427 [details]
Patch

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

>>> Source/WebCore/platform/graphics/gstreamer/GStreamerRegistryScanner.cpp:175
>>> +        Optional<HashMap<AtomString, bool>> codecMap;
>> 
>> Why do you need to make this Optional if you're not checking if the value is there?
> 
> No need indeed!

Ah I remember now... I used Optional<> because it keeps an internal reference, which is needed here, because the HashMap and HashSet are modified below...

[Carlos Alberto Lopez Perez]

(In reply to Philippe Normand from comment #4)
> Committed r268576: <https://trac.webkit.org/changeset/268576>

This has caused 35 new failures on layout tests for GTK:

  imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/mime-types/canPlayType.html [ Failure ]
  media/audio-controls-rendering.html [ Failure ]
  media/audio-playback-restriction-removed-muted.html [ Failure ]
  media/audio-playback-volume-changes-with-restrictions-and-user-gestures.html [ Failure ]
  media/media-can-play-flac-audio.html [ Failure ]
  media/media-can-play-wav-audio.html [ Failure ]
  media/restricted-audio-playback-with-document-gesture.html [ Failure ]
  media/video-controls-visible-audio-only.html [ Failure ]
  userscripts/user-script-audio-document.html [ Failure ]
  userscripts/user-script-video-document.html [ Failure ]
  imported/w3c/web-platform-tests/css/css-transitions/parsing/transition-timing-function-computed.html [ Crash ]
  imported/w3c/web-platform-tests/html/canvas/offscreen/path-objects/2d.path.arc.selfintersect.1.html [ Crash ]
  http/tests/media/video-buffered-range-contains-currentTime.html [ Timeout ]
  http/tests/security/webaudio-render-remote-audio-allowed-crossorigin-redirect.html [ Timeout ]
  http/tests/security/webaudio-render-remote-audio-allowed-crossorigin.html [ Timeout ]
  http/tests/security/webaudio-render-remote-audio-blocked-no-crossorigin-redirect.html [ Timeout ]
  http/tests/security/webaudio-render-remote-audio-blocked-no-crossorigin.html [ Timeout ]
  media/audio-background-playback-playlist.html [ Timeout ]
  media/audio-constructor-preload.html [ Timeout ]
  media/audio-constructor-src.html [ Timeout ]
  media/audio-constructor.html [ Timeout ]
  media/audio-controls-timeline-in-media-document.html [ Timeout ]
  media/audio-data-url.html [ Timeout ]
  media/audio-delete-while-slider-thumb-clicked.html [ Timeout ]
  media/audio-play-event.html [ Timeout ]
  media/controls-should-not-trigger-isolates-blending.html [ Timeout ]
  media/media-continues-playing-after-replace-source.html [ Timeout ]
  media/media-document-audio-controls-visible.html [ Timeout ]
  media/media-document-audio-size.html [ Timeout ]
  media/media-higher-prio-audio-stream.html [ Timeout ]
  media/media-load-event.html [ Timeout ]
  media/media-volume-slider-rendered-below.html [ Timeout ]
  media/restricted-audio-playback-with-multiple-settimeouts.html [ Timeout ]
  media/video-defaultmuted.html [ Timeout ]
  media/video-rtl.html [ Timeout ]


Check previous run (for r268575) -> https://build.webkit.org/builders/GTK-Linux-64-bit-Release-Tests/builds/16457
Check run for this commit (r268576) -> https://build.webkit.org/builders/GTK-Linux-64-bit-Release-Tests/builds/16458

Results of the tests here: https://build.webkit.org/results/GTK-Linux-64-bit-Release-Tests/r268576%20(16458)/results.html


This doesn't seem to have caused issue on the WPE bots because those tests are skipped for WPE :\

[Philippe Normand]

https://trac.webkit.org/r268586