1542674 - Make MediaFormatReader::GetMozDebugReaderData() machine parsable

Status: RESOLVED FIXED

(Core :: Audio/Video, enhancement, P2)

Description

[Tarek Ziadé (:tarek)]

Let's make the output machine parseable (my preference would be JSON) so the code using it is future-proof when we add some info there.

Comment 1

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Marking as P2 based on being assigned. Please place in backlog if this is not something we're planning on working on in the next few releases.

Comment 2

[Tarek Ziadé (:tarek)]

Attachment: Bug 1542674 - Make Media debug info machine parsable r?padenot,smaug,jya

work in progress

Comment 3

[Tarek Ziadé (:tarek)]

consumer (besides media/dom/test/marionette)

https://github.com/mjfroman/media-devtools-panel-react

(with mjf as reviewer)

Comment 4

[Tarek Ziadé (:tarek)]

The current design makes it hard to build a clean and simple JSON output. As suggested by Paul I will redesign the patch from scratch by separating the data and the presentation.

This is what I propose:

• each RequestDebugInfo / GetDebugInfo method will return a specific structure containing the class debug information.
• I will add a dom/media/DebugInfo.[h|cpp] module that will group all the structures to have a single place to change
  and overview them in the code base.
• there will be a simple function in that module to generate a JSON object we can pass to the JS layer. That function will
  use the structures it collected via GetDebugInfo() calls

Comment 5

[Tarek Ziadé (:tarek)]

unfortunately using JSONWriter is a bad idea because the string produced is not what we ultimately want to send via webidl.
I am refactoring the patch so all structures are webidl dictionaries instead of my MediaDebugInfo class.

This way we can pass it through JS without any trouble..

Comment 6

[Tarek Ziadé (:tarek)]

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

Comment 7

[Tarek Ziadé (:tarek)]

New try jobs

https://treeherder.mozilla.org/#/jobs?repo=try&revision=af40f633650a84b2cbc89765a033b0f588fb7a47
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f95c3a69435aad72cb503f34d47733b2359cf78a

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c386ebfd9c6b
Make Media debug info machine parsable r=padenot,smaug

Comment 9

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/c386ebfd9c6b

Comment 10

[Tarek Ziadé (:tarek)]

Backing out, see Bug 1549697

Comment 11

[Stefan Hindli [:stefan_hindli]]

Backed out changeset c386ebfd9c6b (Bug 1542674) as per tarek's request on IRC

Comment 12

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/882bba44f789
Backed out changeset c386ebfd9c6b as per tarek's request on IRC

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://bugherder.mozilla.org/?cset=882bba44f789d3189564d44134a452daf2aad3ed&tree=mozilla-central

Comment 14

[Tarek Ziadé (:tarek)]

New try runs

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f0a09f4aaccdd992dd2d69dfb29e643dee84351f
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2c3df5a46984004cef5b5ecf7c71c6d8f96abe67

Comment 15

[Tarek Ziadé (:tarek)]

new try runs after the beta tag

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b944ede16ce23bc750601fbccd89f378cbb0d3c2
https://treeherder.mozilla.org/#/jobs?repo=try&revision=35809cc1449b78179af322d98f4b8402d87f121f

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a064b18a1a57
Make Media debug info machine parsable r=padenot,smaug,jya

Comment 17

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/a064b18a1a57

Comment 18

[Boris Zbarsky [:bzbarsky]]

Shouldn't all the mozDumpDebugInfo() callers in tests get updated?

Comment 19

[Tarek Ziadé (:tarek)]

That's right, thanks for noticing and sorry for the miss. I added bug 1556637 to fix this issue.