1324926 - [EME] Improve logging in EME/GMP modules

Status: RESOLVED FIXED

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

Description

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

While debugging a Linux/Netflix issue, it seemed handy to improve logging of EME/GMP.

Comment 1

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

Attachment: Bug 1324926 - Log Gecko version and buildID in GMP log.

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

Comment 2

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

Attachment: Bug 1324926 - Log keyId as hex when decoding video via GMP.

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

Comment 3

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

Attachment: Bug 1324926 - Convert other EME/GMP byte logging from Base64 to Hex.

This is marginly more readable.

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

Comment 4

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

Attachment: Bug 1324926 - Log expiration in terms of hours remaining in MediaKeySession.SetExpiration().

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

Comment 5

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

Comment on attachment 8820444 [details]
Bug 1324926 - Log Gecko version and buildID in GMP log.

https://reviewboard.mozilla.org/r/99940/#review100418

Comment 6

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

Comment on attachment 8820445 [details]
Bug 1324926 - Log keyId as hex when decoding video via GMP.

https://reviewboard.mozilla.org/r/99942/#review100420

Comment 7

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

Comment on attachment 8820446 [details]
Bug 1324926 - Convert other EME/GMP byte logging from Base64 to Hex.

https://reviewboard.mozilla.org/r/99944/#review100424

Comment 8

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

Comment on attachment 8820447 [details]
Bug 1324926 - Log expiration in terms of hours remaining in MediaKeySession.SetExpiration().

https://reviewboard.mozilla.org/r/99946/#review100426

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b0a59896aa01
Log Gecko version and buildID in GMP log. r=gerald
https://hg.mozilla.org/integration/autoland/rev/1694d0248f1a
Log keyId as hex when decoding video via GMP. r=gerald
https://hg.mozilla.org/integration/autoland/rev/3a5fc19f492b
Convert other EME/GMP byte logging from Base64 to Hex. r=gerald
https://hg.mozilla.org/integration/autoland/rev/e57acea9a4ab
Log expiration in terms of hours remaining in MediaKeySession.SetExpiration(). r=gerald

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back these out for Windows build failures like https://treeherder.mozilla.org/logviewer.html#?job_id=8213846&repo=autoland

https://hg.mozilla.org/integration/autoland/rev/58d40c0333cf

Comment 11

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

Comment on attachment 8820444 [details]
Bug 1324926 - Log Gecko version and buildID in GMP log.

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

Comment 12

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

Comment on attachment 8820445 [details]
Bug 1324926 - Log keyId as hex when decoding video via GMP.

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

Comment 13

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

Comment on attachment 8820446 [details]
Bug 1324926 - Convert other EME/GMP byte logging from Base64 to Hex.

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

Comment 14

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

Comment on attachment 8820447 [details]
Bug 1324926 - Log expiration in terms of hours remaining in MediaKeySession.SetExpiration().

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

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4b7650c9b073
Log Gecko version and buildID in GMP log. r=gerald
https://hg.mozilla.org/integration/autoland/rev/c7e0e99464b4
Log keyId as hex when decoding video via GMP. r=gerald
https://hg.mozilla.org/integration/autoland/rev/1fa993c1b9e7
Convert other EME/GMP byte logging from Base64 to Hex. r=gerald
https://hg.mozilla.org/integration/autoland/rev/158945ebbebc
Log expiration in terms of hours remaining in MediaKeySession.SetExpiration(). r=gerald

Comment 16

[James Cheng[:JamesCheng]]

Comment on attachment 8820445 [details]
Bug 1324926 - Log keyId as hex when decoding video via GMP.

https://reviewboard.mozilla.org/r/99942/#review100508

::: dom/media/gmp/GMPUtils.cpp:69
(Diff revision 2)
>    }
>    return base64;
>  }
>  
> +nsCString
> +ToHexString(const uint8_t * aBytes, uint32_t aLength)

just find a extra white space uint8_t * aBytes,

Comment 17

[James Cheng[:JamesCheng]]

Comment on attachment 8820445 [details]
Bug 1324926 - Log keyId as hex when decoding video via GMP.

https://reviewboard.mozilla.org/r/99942/#review100512

::: dom/media/gmp/GMPUtils.cpp:80
(Diff revision 2)
> +    'c', 'd', 'e', 'f'
> +  };
> +  nsCString str;
> +  for (uint32_t i = 0; i < aLength; i++) {
> +    char buf[3];
> +    buf[0] = hex[aBytes[i] & 0xf0 >> 4];

Hi Chris,
Have you run this code locally and the shown the correct result ?

For me, "& 0xf0" will be prompted into integer which means the length is 32bits.

After shifting 4 bits, it larger than 0xFF which may cause a stack overflow, right?

Comment 18

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

It works for me locally. I would assume that when assigning to a char, the value would be converted to an 8 bit type. If this was a memcpy, I'd be more worried about a write out-of-bounds.

Comment 19

[(Away)]

> > +    buf[0] = hex[aBytes[i] & 0xf0 >> 4];

Doesn't >> have higher precedence than &?

Comment 20

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

Oh dear, I missed that one. Thanks David.

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4b7650c9b073
https://hg.mozilla.org/mozilla-central/rev/c7e0e99464b4
https://hg.mozilla.org/mozilla-central/rev/1fa993c1b9e7
https://hg.mozilla.org/mozilla-central/rev/158945ebbebc