Make MediaFormatReader::GetMozDebugReaderData() machine parsable

RESOLVED FIXED in Firefox 69

Status

()

enhancement
P2
normal
RESOLVED FIXED
2 months ago
7 days ago

People

(Reporter: tarek, Assigned: tarek)

Tracking

57 Branch
mozilla69
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox68 wontfix, firefox69 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

2 months ago

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.

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.

Type: defect → enhancement
Priority: -- → P2
Assignee

Updated

2 months ago
Depends on: 1543032
Assignee

Updated

2 months ago
Assignee: jyavenard → tarek
Assignee

Comment 3

2 months ago

consumer (besides media/dom/test/marionette)

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

(with mjf as reviewer)

Assignee

Comment 4

2 months ago

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
Assignee

Comment 5

2 months ago

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..

Attachment #9058949 - Attachment description: Bug 1542674 - Make MediaFormatReader::GetMozDebugReaderData() machine parsable r?padenot → Bug 1542674 - Make Media debug info machine parsable r?padenot
Attachment #9058949 - Attachment description: Bug 1542674 - Make Media debug info machine parsable r?padenot → Bug 1542674 - Make Media debug info machine parsable r?padenot,smaug

Comment 8

Last month
Pushed by tziade@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c386ebfd9c6b
Make Media debug info machine parsable r=padenot,smaug

Comment 9

Last month
bugherder
Status: NEW → RESOLVED
Closed: Last month
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Assignee

Comment 10

Last month

Backing out, see Bug 1549697

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee

Updated

Last month
Depends on: 1549697

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

Comment 12

Last month
Backout by shindli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/882bba44f789
Backed out changeset c386ebfd9c6b as per tarek's request on IRC
Attachment #9058949 - Attachment description: Bug 1542674 - Make Media debug info machine parsable r?padenot,smaug → Bug 1542674 - Make Media debug info machine parsable r?padenot,smaug,jya
Assignee

Updated

Last month
Blocks: 1551147
Assignee

Updated

28 days ago
No longer blocks: 1551147
Depends on: 1551147

Comment 16

22 days ago
Pushed by tziade@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a064b18a1a57
Make Media debug info machine parsable r=padenot,smaug,jya

Comment 17

22 days ago
bugherder
Status: REOPENED → RESOLVED
Closed: Last month22 days ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

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

Flags: needinfo?(tarek)
Assignee

Comment 19

15 days ago

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

Flags: needinfo?(tarek)
You need to log in before you can comment on or make changes to this bug.