Skip to content

Add JSON serialize functionality - #25

Closed
edominia wants to merge 1 commit into
thesofproject:topic/gst-topofrom
edominia:serialize-json
Closed

Add JSON serialize functionality#25
edominia wants to merge 1 commit into
thesofproject:topic/gst-topofrom
edominia:serialize-json

Conversation

@edominia

@edominiaedominia commented Dec 10, 2019

Copy link
Copy Markdown

Signed-off-by: Emilia Dominiak emilia.dominiak@intel.com

mmaka1
mmaka1 requested changes Dec 10, 2019
View reviewed changes
Comment threadgef-integration.target/gef-integration.target Outdated
Comment threadorg.sofproject.alsa.topo/META-INF/MANIFEST.MF Outdated
Comment threadorg.sofproject.gst.topo/src/org/sofproject/gst/json/JsonCustomOptionPane.java Outdated
Comment threadorg.sofproject.gst.topo/src/org/sofproject/gst/topo/model/GstTopoNode.java Outdated
jajanusz
jajanusz reviewed Dec 12, 2019
View reviewed changes
Comment threadorg.sofproject.gst.topo/src/org/sofproject/gst/json/JsonCustomOptionPane.java Outdated
jajanusz
jajanusz suggested changes Dec 12, 2019
View reviewed changes

@jajanuszjajanusz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting issues - I'd go with default eclipse conventions as probably that will be what most of plugin developers will use.
So you probably can solve of mentioned problems with ctrl shif f & ctrl shift o in eclipse.

Comment threadorg.sofproject.gst.topo/src/org/sofproject/gst/json/JsonCustomOptionPane.java Outdated
Comment threadorg.sofproject.gst.topo/src/org/sofproject/gst/json/JsonCustomOptionPane.java Outdated
Comment threadorg.sofproject.gst.topo/src/org/sofproject/gst/json/JsonCustomOptionPane.java Outdated
Comment threadorg.sofproject.gst.topo/src/org/sofproject/gst/topo/model/GstTopoGraph.java Outdated
jajanusz
jajanusz reviewed Dec 17, 2019
View reviewed changes
Comment threadorg.sofproject.alsa.topo/src/org/sofproject/alsa/topo/model/AlsaTopoGraph.java Outdated
jajanusz
jajanusz reviewed Dec 17, 2019
View reviewed changes
Comment threadorg.sofproject.alsa.topo/src/org/sofproject/alsa/topo/model/AlsaTopoGraph.java Outdated
@edominiaedominia mentioned this pull request Dec 20, 2019
Closed
jajanusz
jajanusz suggested changes Jan 16, 2020
View reviewed changes
Comment threadgef-integration.target/gef-integration.target Outdated
Comment threadorg.sofproject.gst.topo/.classpath
@lgirdwood

lgirdwood commented Jan 16, 2020

Copy link
Copy Markdown
Member

@edominia please also use a more descriptive commit message. You need to explain what, why and how for each patch.

@edominia
edominiaforce-pushed the serialize-json branch 3 times, most recently from 400c344 to 4d84c46CompareJanuary 22, 2020 13:55
@jajanusz

jajanusz commented Jan 29, 2020

Copy link
Copy Markdown

Regarding jars that this PR adds f.e. - org.sofproject.gst.topo/lib/jackson-annotations-2.10.1.jar
@mmaka1 Is it ok to have these binaries here, are there no licensing issues?

mmaka1
mmaka1 requested changes Feb 5, 2020
View reviewed changes
Comment threadorg.sofproject.gst.topo/src/org/sofproject/gst/json/JsonCustomOptionPane.java
gridLayout.verticalSpacing = 8;
shell.setLayout(gridLayout);

new Label(shell, SWT.NULL).setText("Name:");

@mmaka1mmaka1Feb 5, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name, description, and other properties collected here should be properties accessible via the ITopoGraph interface, displayed and editable in the Properties view. So that the serialize-to-json operation would not require any user interaction.

Comment threadorg.sofproject.gst.topo/src/org/sofproject/gst/json/JsonCustomOptionPane.java
Comment threadorg.sofproject.topo.ui/META-INF/MANIFEST.MF
Comment threadorg.sofproject.topo.ui/src/org/sofproject/topo/ui/editor/policies/TopoEditorOnClickHandler.java
Comment threadorg.sofproject.alsa.topo/src/org/sofproject/alsa/topo/model/AlsaTopoGraph.java Outdated
Comment threadorg.sofproject.alsa.topo/src/org/sofproject/alsa/topo/model/AlsaTopoGraph.java Outdated
Comment threadorg.sofproject.alsa.topo/src/org/sofproject/alsa/topo/model/AlsaTopoGraph.java Outdated
try {
ByteArrayOutputStream os = new ByteArrayOutputStream();
Writer writer = new BufferedWriter(new OutputStreamWriter(os));
tlvs.serialize(writer);

@mmaka1mmaka1Feb 5, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not duplicate the code of ITopoGraph.serialize(). Atm me may just refactor the latter to return a String and move target file content writing to topo.ui.

Comment threadorg.sofproject.gst.topo/build.properties
.,\
plugin.xml
plugin.xml,\
lib/jackson-annotations-2.10.1.jar,\

@mmaka1mmaka1Feb 5, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not import external binaries to the plug-in repository. Add a dependency to an existing Eclipse plug-in(s) to the manifest (and adjust the documentation if any of them is not a part of core bundle, see how the dependency on GEF is handled).

@edominia

edominia commented Feb 21, 2020

Copy link
Copy Markdown
Author

Regarding jars that this PR adds f.e. - org.sofproject.gst.topo/lib/jackson-annotations-2.10.1.jar
@mmaka1 Is it ok to have these binaries here, are there no licensing issues?

Jackson is on Apache license so I think it's ok.

@edominia
Add JSON serialize functionality
6ffdf8e
There was only possibility to serialize topology to gstreamer format.
Added JSON serialize functionality. JSON format is is compatible with
VAS.
Signed-off-by: Emilia Dominiak <emilia.dominiak@intel.com>
@edominiaedominia mentioned this pull request Mar 12, 2020
Open
@edominia

edominia commented Apr 17, 2020

Copy link
Copy Markdown
Author

Merge three pull requests into one #43

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@paulstelian97paulstelian97paulstelian97 left review comments
@mmaka1mmaka1mmaka1 requested changes
+1 more reviewer
@jajanuszjajanuszjajanusz requested changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@edominia@lgirdwood@jajanusz@paulstelian97@mmaka1