1194576 - [EME] Add more logging to GMP parent actors.

Status: RESOLVED FIXED

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

Description

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

It would be helpful if we had more NSPR logging about what's going on with GMPs parent actors. That may help debug some of the errors that I can repro intermittently.

Comment 1

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

Attachment: Patch: Add more logging to GMP*Parent actors

I focused on the parent actors that we care about with regards to EME.

Comment 2

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

Comment on attachment 8647894 [details] [diff] [review]
Patch: Add more logging to GMP*Parent actors

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

::: dom/media/gmp/GMPDecryptorParent.cpp
@@ +63,5 @@
>                                    const nsCString& aInitDataType,
>                                    const nsTArray<uint8_t>& aInitData,
>                                    GMPSessionType aSessionType)
>  {
> +  LOGD(("GMPDecryptorParent[%p]::CreateSession(token=%d, promiseId=%d, aInitData='%s')",

Both |%d|'s should be |%u|'s for the uint32_t's.
Same with all new LOGD's below.

@@ +340,5 @@
>  
>  bool
>  GMPDecryptorParent::RecvSetCaps(const uint64_t& aCaps)
>  {
> +  LOGD(("GMPDecryptorParent[%p]::RecvSetCaps(caps=0x%x)", this, aCaps));

%llx, I think?

::: dom/media/gmp/GMPVideoDecoderParent.cpp
@@ +287,5 @@
>      return false;
>    }
>  
>    if (!GMPVideoi420FrameImpl::CheckFrameData(aDecodedFrame)) {
> +    LOG(LogLevel::Error, ("%s: Decoded frame corrupt, ignoring"));

Remove '%s'.

Comment 3

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9cca751c9c7c

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/54d0fc994606

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/54d0fc994606

Comment 6

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

Comment on attachment 8647894 [details] [diff] [review]
Patch: Add more logging to GMP*Parent actors

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: Without this patch, it will be harder to uplift other critical patches as they'll need more complicated rebases, and it harder to debug EME failures in the field in beta builds. This patch adds more NSPR logging, and we've found it very useful to ask people to turn this on to figure out why they've had failures.
[Describe test coverage new/current, TreeHerder]: EME has lots of mochitests
[Risks and why]: Low; just adds logging.
[String/UUID change made/needed]: None.

Beta Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e657c22d317

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8647894 [details] [diff] [review]
Patch: Add more logging to GMP*Parent actors

Taking it to simplify your life, should be in 42 beta 3.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1e432808c085