Bug 217750 - [GStreamer] Encoder probing support for the registry scanner
Summary: [GStreamer] Encoder probing support for the registry scanner
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philippe Normand
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-10-15 04:18 PDT by Philippe Normand
Modified: 2020-10-16 06:23 PDT (History)
12 users (show)

See Also:


Attachments
Patch (59.81 KB, patch)
2020-10-15 04:34 PDT, Philippe Normand
calvaris: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2020-10-15 04:18:59 PDT
This would be useful for the mediacapabilities spec implementation. Currently we probe only for decoders.
Comment 1 Philippe Normand 2020-10-15 04:34:10 PDT
Created attachment 411427 [details]
Patch
Comment 2 Xabier Rodríguez Calvar 2020-10-16 01:14:04 PDT
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 3 Philippe Normand 2020-10-16 01:47:55 PDT
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 :)
Comment 4 Philippe Normand 2020-10-16 01:53:00 PDT
Committed r268576: <https://trac.webkit.org/changeset/268576>
Comment 5 Radar WebKit Bug Importer 2020-10-16 01:53:18 PDT
<rdar://problem/70370485>
Comment 6 Philippe Normand 2020-10-16 05:30:37 PDT
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...
Comment 7 Carlos Alberto Lopez Perez 2020-10-16 05:40:11 PDT
(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 :\
Comment 8 Philippe Normand 2020-10-16 06:23:51 PDT
https://trac.webkit.org/r268586