848994 - [Decoder Doctor] Detect codec problems and tell front-end to notify user to install "Windows Media Feature Pack" if required for WMF playback

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P2)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

Some versions of Windows need to have a "Media Feature Pack" installed in order for Windows Media Foundation to be present. These are usually the "N and KN" versions of Enterprise and Server variants of Windows.

We should try to detect when this is the case, and prompt the user to install the media feature pack.

See also: bug 848562, bug 847879.

Comment 2

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

We still need this after we implanted OpenH264 video codec in bug #948160 or after bug #1057646 will be finalized?

More info what Media Feature Packs for Windows N and KN versions contains:
Win 8.1 - http://support.microsoft.com/kb/2929699
Win 8 - http://support.microsoft.com/kb/2703761
Win 7 - http://support.microsoft.com/kb/968211

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Yes. OpenH264 does not do H.264 Main or High profile, and it does not support the audio codec commonly streamed alongside H.264; AAC. So even with OpenH264 we can't play MP4 videos.

Comment 5

[Chris Peterson [:cpeterson]]

For our MVP, we will only check for the Windows N/KN editions that are missing codecs *and* do not have Flash installed (as a fallback).

Comment 6

[Chris Peterson [:cpeterson]]

cpearce has volunteered to work on the Platform bits of the "Decoder Doctor" feature.

Comment 7

[Chris Peterson [:cpeterson]]

Gerald said he is working on Decoder Doctor for Windows.

Comment 8

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

This WMF-specific task depends on generic infrastructure work done in bug 1248507.

Comment 9

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: 848994-WMF-failure.patch

Detect and report when WMF is not found.
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6f3c0cdd1eb989cf3316ceb0783719b2a74d2b9

Comment 10

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8740761 [details] [diff] [review]
848994-WMF-failure.patch

Boris, another quick review please (after bug 1248507), just for an added enum value in the webidl at the bottom of the patch.
Thank you.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8740761 [details] [diff] [review]
848994-WMF-failure.patch

r=me

Comment 12

[Chris Peterson [:cpeterson]]

Comment on attachment 8740761 [details] [diff] [review]
848994-WMF-failure.patch

Review of attachment 8740761 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/locales/en-US/chrome/dom/dom.properties
@@ +107,5 @@
>  MediaLoadUnsupportedMimeType=HTTP "Content-Type" of "%1$S" is not supported. Load of media resource %2$S failed.
>  # LOCALIZATION NOTE: %S is the URL of the media resource which failed to load because of error in decoding.
>  MediaLoadDecodeError=Media resource %S could not be decoded.
> +MediaWFMNeeded=To play video, you may need to install Microsofts Media Feature Pack.
> +MediaWMFRecommended=To improve video quality, you may need to install Microsofts Media Feature Pack.

I think you have inadvertently inserted a UTF-16 0x0092 smart quote instead of UTF-8. None of the other strings in this file appear to use smart quotes. Do we have a UX style guide that has anything to say on quotes?

Comment 13

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

(In reply to Chris Peterson [:cpeterson] from comment #12)
> > +MediaWMFRecommended=To improve video quality, you may need to install Microsofts Media Feature Pack.
> I think you have inadvertently inserted a UTF-16 0x0092 smart quote instead
> of UTF-8. None of the other strings in this file appear to use smart quotes.
> Do we have a UX style guide that has anything to say on quotes?

Good catch! I copied the strings from a word-processor document, that must be why. I'll fix that of course.

Comment 14

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=19aa0219ff8ae907d241b32068f9fb9207d2b3d6

Comment 15

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8740761 [details] [diff] [review]
848994-WMF-failure.patch

Review of attachment 8740761 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/locales/en-US/chrome/dom/dom.properties
@@ +107,5 @@
>  MediaLoadUnsupportedMimeType=HTTP "Content-Type" of "%1$S" is not supported. Load of media resource %2$S failed.
>  # LOCALIZATION NOTE: %S is the URL of the media resource which failed to load because of error in decoding.
>  MediaLoadDecodeError=Media resource %S could not be decoded.
> +MediaWFMNeeded=To play video, you may need to install Microsofts Media Feature Pack.
> +MediaWMFRecommended=To improve video quality, you may need to install Microsofts Media Feature Pack.

Since bug 1259847 we should be using curly quotes in our strings.

Also, your string name for MediaWFMNeeded is wrong. It should be MediaWMFNeeded.

Comment 16

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

(In reply to Jared Wein [:jaws] from comment #15)
Fixed & fixed. Thank you for checking.

Comment 17

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: MozReview Request: Bug 848994 - p1. Refactor Decoder Doctor - r=cpearce

Some refactoring, clean-ups, etc.

The main change is that the can-play status is passed when diagnostics are
finally recorded. This will help when introducing different types of
diagnostics in future patches (e.g., Key System issues).

Review commit: https://reviewboard.mozilla.org/r/48145/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48145/

Comment 18

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: MozReview Request: Bug 848994 - p2. Detect when WMF is not found - r=cpearce

Review commit: https://reviewboard.mozilla.org/r/48147/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48147/

Comment 19

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: MozReview Request: Bug 848994 - p3. Check MediaKeySystem requests - r?cpearce

Media Key System access requests are now recorded with their success/failure,
as well as accompanying issues of importance.
In this bug we focus on the Widevine-with-no-WMF case.

Review commit: https://reviewboard.mozilla.org/r/48149/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48149/

Comment 20

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: MozReview Request: Bug 848994 - p4. Record GMP diagnostics - r?cpearce

Record diagnostics information about whether the GMP CDM failed to load
(though currently impossible!), and which GMP is used in the current media
format check.

Review commit: https://reviewboard.mozilla.org/r/48151/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48151/

Comment 21

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: MozReview Request: Bug 848994 - p5. Check Silverlight presence - r?cpearce

Review commit: https://reviewboard.mozilla.org/r/48153/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48153/

Comment 22

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: MozReview Request: Bug 848994 - p6. Analyze Windows issues - r?cpearce

Analyze the diagnostics information gathered so far, and dispatch
notifications as appropriate.
The most important case in this bug is when Widevine is requested but WMF and
Silverlight are missing.
The generic case of WMF missing (when needed to play) is there too.

the following patch will add filtering to allow/prevent targeted notifications.

Review commit: https://reviewboard.mozilla.org/r/48155/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48155/

Comment 23

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: MozReview Request: Bug 848994 - p7. Filter front-end notifications - r?cpearce

Pref "media.decoder-doctor.notifications-allowed" defines which notifications
may be forwarded to the front-end (based on the web-console message ID for
that situation). This allows fine-grained filtering.

The current default is to only notify the user about Widevine requests when WMF
and Silverlight are missing (because Widevine does not include an AAC decoder).

Review commit: https://reviewboard.mozilla.org/r/48157/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/48157/

Comment 24

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8743878 [details]
MozReview Request: Bug 848994 - p1. Refactor Decoder Doctor - r=cpearce

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48145/diff/1-2/

Comment 25

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8743879 [details]
MozReview Request: Bug 848994 - p2. Detect when WMF is not found - r=cpearce

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48147/diff/1-2/

Comment 26

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8743880 [details]
MozReview Request: Bug 848994 - p3. Check MediaKeySystem requests - r?cpearce

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48149/diff/1-2/

Comment 27

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8743881 [details]
MozReview Request: Bug 848994 - p4. Record GMP diagnostics - r?cpearce

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48151/diff/1-2/

Comment 28

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8743882 [details]
MozReview Request: Bug 848994 - p5. Check Silverlight presence - r?cpearce

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48153/diff/1-2/

Comment 29

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8743883 [details]
MozReview Request: Bug 848994 - p6. Analyze Windows issues - r?cpearce

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48155/diff/1-2/

Comment 30

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8743884 [details]
MozReview Request: Bug 848994 - p7. Filter front-end notifications - r?cpearce

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48157/diff/1-2/

Comment 31

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8743878 [details]
MozReview Request: Bug 848994 - p1. Refactor Decoder Doctor - r=cpearce

https://reviewboard.mozilla.org/r/48145/#review44945

Comment 32

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8743879 [details]
MozReview Request: Bug 848994 - p2. Detect when WMF is not found - r=cpearce

https://reviewboard.mozilla.org/r/48147/#review44949

Comment 33

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8743880 [details]
MozReview Request: Bug 848994 - p3. Check MediaKeySystem requests - r?cpearce

https://reviewboard.mozilla.org/r/48149/#review44953

::: dom/media/DecoderDoctorDiagnostics.cpp:327
(Diff revision 2)
>          }
>          unplayableFormats += diag.mDecoderDoctorDiagnostics.Format();
>        }
> +    } else if (!diag.mDecoderDoctorDiagnostics.KeySystem().IsEmpty()) {
> +      if (!diag.mDecoderDoctorDiagnostics.IsKeySystemSupported()) {
> +        if (!unsupportedKeySystems.IsEmpty()) {

Optional: I feel like a helper that appends ", " to a string if the string isn't empty may make this code cleaner.

::: dom/media/DecoderDoctorDiagnostics.cpp:482
(Diff revision 2)
> +
> +  mKeySystem = aKeySystem;
> +  mIsKeySystemSupported = aIsSupported;
> +
> +  // StoreDiagnostics should only be called once, after all data is available,
> +  // so it is safe to Move() from this object.

Forgive me if I'm missing something, but is there a way you can assert or enforce that StoreDiagnostics is only called once?

::: dom/media/eme/MediaKeySystemAccessManager.cpp:108
(Diff revision 2)
>                                            aKeySystem,
>                                            MediaKeySystemStatus::Api_disabled);
>      aPromise->MaybeReject(NS_ERROR_DOM_NOT_SUPPORTED_ERR,
>                            NS_LITERAL_CSTRING("EME has been preffed off"));
> +    diagnostics.StoreMediaKeySystemAccess(mWindow->GetExtantDoc(),
> +                                          aKeySystem, false, __func__);

Do we need to store diagnostics in the cases where EME is unsupported for reasons other than missing codecs?

There's already events fired and UI for handling the cases where EME is unsupported for non-codec reasons.

Comment 34

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8743881 [details]
MozReview Request: Bug 848994 - p4. Record GMP diagnostics - r?cpearce

https://reviewboard.mozilla.org/r/48151/#review44961

::: dom/media/DecoderDoctorDiagnostics.cpp:501
(Diff revision 2)
>      }
>      if (mFFmpegFailedToLoad) {
>        s += ", Linux platform decoder failed to load";
>      }
> +    if (mGMPFailedToStartup) {
> +      s += ", GMP failed to startup";

This does *not* record whether the GMP failed to startup. This records whether the GMP *PDM* failed to startup.

So this string should reflect that. And yes, as you mentioned in the commit message, the GMP PDM can't currently fail to load.

GMP's failing to load typically manifests as a MediaKeys::Init() promise rejecting with "Call to GetGMPDecryptor() failed early", see CDMProxy::gmp_InitGetGMPDecryptor(). At least in the EME case.

::: dom/media/DecoderDoctorDiagnostics.cpp:502
(Diff revision 2)
>      if (mFFmpegFailedToLoad) {
>        s += ", Linux platform decoder failed to load";
>      }
> +    if (mGMPFailedToStartup) {
> +      s += ", GMP failed to startup";
> +    }

} else if (...) {

That is, '}' on the same line as "else".

Or at least be consistent with the rest of the file.

Comment 35

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8743882 [details]
MozReview Request: Bug 848994 - p5. Check Silverlight presence - r?cpearce

https://reviewboard.mozilla.org/r/48153/#review44963

::: dom/media/DecoderDoctorDiagnostics.cpp:309
(Diff revision 2)
> +{
> +  RefPtr<nsPluginHost> pluginHost = nsPluginHost::GetInst();
> +  if (!pluginHost) {
> +    return eNoSilverlight;
> +  }
> +  nsTArray<nsCOMPtr<nsIInternalPluginTag>> plugins;

It's remarkably easier than I expected to figure out what plugins we have!  :)

Comment 36

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://reviewboard.mozilla.org/r/48153/#review44965

::: dom/media/DecoderDoctorDiagnostics.cpp:305
(Diff revision 2)
> +  eSilverlightEnabled
> +};
> +static SilverlightPresence
> +CheckSilverlight()
> +{
> +  RefPtr<nsPluginHost> pluginHost = nsPluginHost::GetInst();

At a glance nsPluginHost doesn't look like it's threadsafe. So you probably should be asserting that this function only gets called on the main thread?

Comment 37

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8743883 [details]
MozReview Request: Bug 848994 - p6. Analyze Windows issues - r?cpearce

https://reviewboard.mozilla.org/r/48155/#review44969

::: dom/locales/en-US/chrome/dom/dom.properties:110
(Diff revision 2)
>  MediaLoadSourceMediaNotMatched=Specified "media" attribute of "%1$S" does not match the environment. Load of media resource %2$S failed.
>  # LOCALIZATION NOTE: %1$S is the MIME type HTTP header being sent by the web server, %2$S is the URL of the media resource which failed to load.
>  MediaLoadUnsupportedMimeType=HTTP "Content-Type" of "%1$S" is not supported. Load of media resource %2$S failed.
>  # LOCALIZATION NOTE: %S is the URL of the media resource which failed to load because of error in decoding.
>  MediaLoadDecodeError=Media resource %S could not be decoded.
> +MediaWidevineNoWMFNoSilverlight=Trying to play Widevine with no MWF nor Silverlight, see https://support.mozilla.org/kb/fix-video-audio-problems-firefox-windows

s/MWF/Windows Media Foundation/

Or at least, s/MWF/WMF/

;)


I think the goal here should be to get people to install the Media Feature Pack, so how about we don't mention Silverlight in the string?

We check for Silverlight, because we assume the site will fallback to Silverlight if Widevine EME doesn't work. If both Widevine EME and Silverlight don't work, the site won't work; it's a dead end. We want to drive people to getting Widevine EME working, *not* Silverlight.

::: dom/media/DecoderDoctorDiagnostics.cpp:324
(Diff revision 2)
>    }
>  
>    return eNoSilverlight;
>  }
>  
> +static void AppendToStringList(nsAString& list, const nsAString& item)

Ah, there's the helper I suggested earlier. Great minds think alike!

Comment 38

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8743884 [details]
MozReview Request: Bug 848994 - p7. Filter front-end notifications - r?cpearce

https://reviewboard.mozilla.org/r/48157/#review44977

::: dom/media/DecoderDoctorDiagnostics.cpp:311
(Diff revision 2)
> +  if (filter.EqualsLiteral("-")) {
> +    // Allow nothing.
> +  } else if (filter.EqualsLiteral("*")) {
> +    allowed = true;
> +  } else for (uint32_t start = 0; start < filter.Length(); ) {
> +    int32_t comma = filter.FindChar(',', start);

You could use SplitAt() from GMPUtils.h here to make this a simpler loop. If you do that, you can move SplitAt() into VideoUtils.h I guess, as it would no longer be a GMP specific utility.

::: modules/libpref/init/all.js:357
(Diff revision 2)
>  pref("media.apple.mp4.enabled", true);
>  #endif
> +
> +// Filter what triggers user notifications.
> +// See DecoderDoctorDocumentWatcher::ReportAnalysis for details.
> +pref("media.decoder-doctor.notifications-allowed", "");

I'd stick 'MediaWidevineNoWMFNoSilverlight' here (and not in the code), so the default is obvious to the place where the behaviour is configured; the pref's value.

Comment 39

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

https://reviewboard.mozilla.org/r/48149/#review44953

> Optional: I feel like a helper that appends ", " to a string if the string isn't empty may make this code cleaner.

As you've noticed later, it's happening in part 6.

> Forgive me if I'm missing something, but is there a way you can assert or enforce that StoreDiagnostics is only called once?

Nothing to forgive, as I did not check -- I thought the normal usage pattern (where creation and storage happen at the same spot) would naturally prevent misuse.
However, better be safe than sorry, so I'm adding a diagnostics type that will both help with store-only-once assertions, and as a bonus will make it easier to determine what's in the diagnostics struct during analysis.

> Do we need to store diagnostics in the cases where EME is unsupported for reasons other than missing codecs?
> 
> There's already events fired and UI for handling the cases where EME is unsupported for non-codec reasons.

As discussed elsewhere, gathering this information may still be useful, and it could end up in a more-verbose web-console messages.
For sure, it will be important to avoid displaying two UX notification drop-down bars!
This won't happen in this bug (as the only MediaKeySystem-related information used is targetted at a situation where there is currently no UX), so I propose that we let that through, but make note of it in follow-up bugs that will deal with these situations with existing UX.

Comment 40

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

https://reviewboard.mozilla.org/r/48151/#review44961

> This does *not* record whether the GMP failed to startup. This records whether the GMP *PDM* failed to startup.
> 
> So this string should reflect that. And yes, as you mentioned in the commit message, the GMP PDM can't currently fail to load.
> 
> GMP's failing to load typically manifests as a MediaKeys::Init() promise rejecting with "Call to GetGMPDecryptor() failed early", see CDMProxy::gmp_InitGetGMPDecryptor(). At least in the EME case.

Changed variable names and description strings to say "GMP PDM". Yes we'll have to catch actual GMP failures elsewhere.
As discussed we don't actually need this part for this specific bug just yet, but it will be useful later on.

Comment 41

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

https://reviewboard.mozilla.org/r/48153/#review44965

> At a glance nsPluginHost doesn't look like it's threadsafe. So you probably should be asserting that this function only gets called on the main thread?

Everything in DD is on the main thread. I'll add an assert for peace of mind.

Comment 42

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

https://reviewboard.mozilla.org/r/48155/#review44969

> s/MWF/Windows Media Foundation/
> 
> Or at least, s/MWF/WMF/
> 
> ;)
> 
> 
> I think the goal here should be to get people to install the Media Feature Pack, so how about we don't mention Silverlight in the string?
> 
> We check for Silverlight, because we assume the site will fallback to Silverlight if Widevine EME doesn't work. If both Widevine EME and Silverlight don't work, the site won't work; it's a dead end. We want to drive people to getting Widevine EME working, *not* Silverlight.

I'm really having trouble with this "WMF" acronym! I'll expand it in the string.

As discussed, this string will only be shown in the web console, which will mostly be seen by more knowledgeable people, and the given SUMO link is all about WMF.
But to reduce emphasis on Silverlight, I'll change its mention to "(nor Silverlight fallback)".

Comment 43

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8743878 [details]
MozReview Request: Bug 848994 - p1. Refactor Decoder Doctor - r=cpearce

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48145/diff/2-3/

Comment 44

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8743879 [details]
MozReview Request: Bug 848994 - p2. Detect when WMF is not found - r=cpearce

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48147/diff/2-3/

Comment 45

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8743880 [details]
MozReview Request: Bug 848994 - p3. Check MediaKeySystem requests - r?cpearce

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48149/diff/2-3/

Comment 46

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8743881 [details]
MozReview Request: Bug 848994 - p4. Record GMP diagnostics - r?cpearce

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48151/diff/2-3/

Comment 47

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8743882 [details]
MozReview Request: Bug 848994 - p5. Check Silverlight presence - r?cpearce

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48153/diff/2-3/

Comment 48

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8743883 [details]
MozReview Request: Bug 848994 - p6. Analyze Windows issues - r?cpearce

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48155/diff/2-3/

Comment 49

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8743884 [details]
MozReview Request: Bug 848994 - p7. Filter front-end notifications - r?cpearce

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/48157/diff/2-3/

Comment 50

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

https://reviewboard.mozilla.org/r/48157/#review44977

> You could use SplitAt() from GMPUtils.h here to make this a simpler loop. If you do that, you can move SplitAt() into VideoUtils.h I guess, as it would no longer be a GMP specific utility.

If you don't mind, I'd like to skip this (non-issue) change right now, and come back to it in a follow-up.
I would like to use SplitAt, and in fact I would argue it should be made even more available across the project. I can't believe it's not already needed and/or reinvented in multiple places!

Comment 51

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd03882583511870115f0b4e158c6cee33238a39

Comment 52

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c2c86d1fa8a1
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf1b1f91f522
https://hg.mozilla.org/integration/mozilla-inbound/rev/826d477a117b
https://hg.mozilla.org/integration/mozilla-inbound/rev/1c4133b49948
https://hg.mozilla.org/integration/mozilla-inbound/rev/0843709b5d55
https://hg.mozilla.org/integration/mozilla-inbound/rev/4f09b3b6b8e9
https://hg.mozilla.org/integration/mozilla-inbound/rev/e07d236d6566

Comment 53

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c2c86d1fa8a1
https://hg.mozilla.org/mozilla-central/rev/cf1b1f91f522
https://hg.mozilla.org/mozilla-central/rev/826d477a117b
https://hg.mozilla.org/mozilla-central/rev/1c4133b49948
https://hg.mozilla.org/mozilla-central/rev/0843709b5d55
https://hg.mozilla.org/mozilla-central/rev/4f09b3b6b8e9
https://hg.mozilla.org/mozilla-central/rev/e07d236d6566