| Summary: | [GStreamer] Encoder probing support for the registry scanner | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Philippe Normand <pnormand> | ||||
| Component: | Platform | Assignee: | Philippe Normand <pnormand> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | calvaris, cgarcia, eric.carlson, ews-watchlist, glenn, gustavo, jer.noble, menard, philipj, sergio, vjaquez, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Philippe Normand
2020-10-15 04:18:59 PDT
Created attachment 411427 [details]
Patch
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? 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 :) Committed r268576: <https://trac.webkit.org/changeset/268576> 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... (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 :\ |