Closed Bug 1542674 Opened 1 year ago Closed 1 year ago

Make MediaFormatReader::GetMozDebugReaderData() machine parsable

Categories

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

57 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox68 --- wontfix
firefox69 --- fixed

People

(Reporter: tarek, Assigned: tarek)

References

Details

Attachments

(1 file)

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
Depends on: 1543032
Assignee: jyavenard → tarek

consumer (besides media/dom/test/marionette)

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

(with mjf as reviewer)

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

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
Pushed by tziade@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c386ebfd9c6b
Make Media debug info machine parsable r=padenot,smaug
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Backing out, see Bug 1549697

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Depends on: 1549697

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

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
Blocks: 1551147
No longer blocks: 1551147
Depends on: 1551147
Pushed by tziade@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a064b18a1a57
Make Media debug info machine parsable r=padenot,smaug,jya
Status: REOPENED → RESOLVED
Closed: 1 year ago1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

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

Flags: needinfo?(tarek)

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.