adding spatial extension #150
Conversation
28230b1
to
069abea
Compare
6a7a357
to
3638281
Compare
|
Updated this as the first pass had multidimensional arrays which can be problematic for parsing (e.g.: in libsigmf). Also removed the obnoxious multi-type bearing object which is similarly problematic for parsing. |
bd9c43a
to
adeafb5
Compare
Signed-off-by: Jacob Gilbert <[email protected]>
Added a couple catures fields to support in-band calibration and to enable collection of coherent but uncalibrated data Signed-off-by: Jacob Gilbert <[email protected]>
adeafb5
to
8467e2b
Compare
extensions/spatial.sigmf-ext.md
Outdated
| |name|required|type|units|description| | ||
| |----|--------|----|-----|-----------| | ||
| |`num_elements`|true|int|N/A|Defines the number of phase centers / channels collected in the Collection or multichannel Dataset.| | ||
| |`channel_num`|true|int|N/A|The channel number, shares index with `element_geometry`.| |
There was a problem hiding this comment.
Now, this field would not apply to multi-channel .sigmf-data files, agreed? I suggest a JSON object in a collection be the one and only place that num_elements and channel_num be indicated.
There was a problem hiding this comment.
Now, this field would not apply to multi-channel .sigmf-data files, agreed?
yep, this doesn't make much sense for multichannel datasets.
Some place needs to map each file to aperture arrays though, and in our work we have found that contextual information is of significant value to the individual files, so this is included here.
multichannel datasets were sort of shoehorned in to this when they came into existence (they were never really a roadmapped feature, they just sort of came about). hrmm will need to think more. same issue with conditional requirement as-is.
There was a problem hiding this comment.
I have added clarification specifically for the case of multi-channel datasets. Even with multichannel datasets there is no guarantee that all of the data required for the spatial extension is contained in one file (e.g.: Two four channel radios that are synchronized and aligned with each other providing 4 phase-coherent channels in two separate 4 channel files), so this is only redundant in the specific (not recommended) case where all data is within one sigmf file.
There was a problem hiding this comment.
If you went the Collections-based approach in my previous comment, would the need for this be OBE and simplify the structure? Honest question.
There was a problem hiding this comment.
See my other response to the line above, the following excerpt holds true here as well I think.
ultimately I think that the cost of a single redundant field is negligible especially considering it contains contextually useful information even in a standalone sense.
extensions/spatial.sigmf-ext.md
Outdated
| |name|required|type|description| | ||
| |----|--------|----|-----------| | ||
| |`signal_azimuth`|false|double|Azimuth in degrees east of north associated with the specific annotation.| | ||
| |`signal_bearing`|false|`bearing`|Bearing associated with the specific annotation.| |
There was a problem hiding this comment.
For the case of a collection, which of the num_elements .sigmf-meta files should signal_{azimuth, bearing} go in?? Please don't say "all of them" :-D
Surely this doesn't belong in the .sigmf-collection file ...
Some things are just so much easier in multi-channel recordings ...
There was a problem hiding this comment.
this is left to implementation. in general when using this, we reduce the multichannel data to a separate sigmf-meta file (or no data file at all).
extensions/spatial.sigmf-ext.md
Outdated
| |name|required|type|description| | ||
| |----|--------|----|-----------| | ||
| |`signal_azimuth`|false|double|Azimuth in degrees east of north associated with the specific annotation.| | ||
| |`signal_bearing`|false|`bearing`|Bearing associated with the specific annotation.| |
There was a problem hiding this comment.
this is left to implementation. in general when using this, we reduce the multichannel data to a separate sigmf-meta file (or no data file at all).
extensions/spatial.sigmf-ext.md
Outdated
| |name|required|type|units|description| | ||
| |----|--------|----|-----|-----------| | ||
| |`num_elements`|true|int|N/A|Defines the number of phase centers / channels collected in the Collection or multichannel Dataset.| | ||
| |`channel_num`|true|int|N/A|The channel number, shares index with `element_geometry`.| |
There was a problem hiding this comment.
Now, this field would not apply to multi-channel .sigmf-data files, agreed?
yep, this doesn't make much sense for multichannel datasets.
Some place needs to map each file to aperture arrays though, and in our work we have found that contextual information is of significant value to the individual files, so this is included here.
multichannel datasets were sort of shoehorned in to this when they came into existence (they were never really a roadmapped feature, they just sort of came about). hrmm will need to think more. same issue with conditional requirement as-is.
|
I should also probably mention that this is a formerly private extension that has been in active, stable use for some time. The proposal being made in this PR is not really "lets discuss this on a conceptual level", but rather "is this of sufficient common value to the world to upstream". Some tweaks can possibly be supported, specifically for currently unused features (multichannel datasets for example), but breaking changes will not be made. |
Hmm -- unless 1) it forever remains a private extensions (which I think is allowed, although surely not what you were planning), 2) the public version of this which incorporates input from the community be a fork with a new name, or 3) a backwards-incompatible version 2.0 of the extension is the first one publicly released. Again, all of these have significant drawbacks to your work, but they are hypothetically plausible, no? Like, you did want feedback, right? |
Oops I now see that I did not read your comment nearly as carefully as I should have. Let me say instead that I do feel that the extension is valuable, and hopefully will be eventually accepted as a standard extension. |
|
Yes, I do want feedback, and value what you provided. What will happen to that feedback is dependent on whether it breaks existing usage though, just want to be upfront about that. I can certainly continue to keep and maintain this privately (as I do with other extensions), but figured I would put this forward since a number of people have brought up discussion about this over the past few years. |
both multidimensional arrays and overloaded types (`bearing`) object are obnoxious for parsing so these are removed. language surrounding multichannel objects has also been updated. Signed-off-by: Jacob Gilbert <[email protected]>
8467e2b
to
804583a
Compare
Signed-off-by: Jacob Gilbert <[email protected]>
804583a
to
2a381dd
Compare
Ok, thanks for your input - the changes ended up being fairly subtle but you caught some important stuff with the collections vs multichannel datasets that would have made for awkward implementations. |
dd2a785
to
fee80a3
Compare
Signed-off-by: Jacob Gilbert <[email protected]>
31e583a
to
a246873
Compare
a246873
to
20ed862
Compare
There was a problem hiding this comment.
Overall, I like the updated design, @jacobagilbert. I found a couple of small items that I think will be easy to fix and/or add. LGTM once those comments are addressed 👍🏼
20ed862
to
0071d3a
Compare
Signed-off-by: Jacob Gilbert <[email protected]>
de7f240
to
32c620f
Compare
|
All discussion resolved, long overdue... merging. |
adding spatial extension Signed-off-by: Jacob Gilbert <[email protected]>
#147 cannot be reopened so I am recreating this here...
This extension is intended to augment the antenna and multirecording extensions by providing a canonical system for managing phase coherent metadata for spatial RF and audio applications. Currently this is primarily focused on how to associate line-of-bearing information with annotations or reference datasets. We felt this was something that was of sufficient general utility that upstreaming was a good idea.
Due to complicated coordinate reference systems, this extension includes a figure; not sure how we feel about that....but it is quite helpful for keeping the interaction between the cartesian definition of the antenna array geometry and the spherical nature of bearing/direction data.
There is a complicated and very large trade space of how this could have been implemented, what was chosen is heavily biased toward real-world meaningful information as used in navigation, astronomy, mapping, etc, which is the primary reason why degrees were chosen for units and AZ/EL is used to contain this information as opposed to the mathematical representation of phi/theta.
If someone is aware of an appropriate ISO/RFC for the common definition of 'azimuth' and 'elevation' that would be good to know; a quick search resulted in mostly paywalled ISO specs (ISO 19101-1, 19111, and 6709 seem like they may but paywalled).
You can view the markdown file and image directly on my branch here:
https://github.com/jacobagilbert/SigMF/blob/add_spatial_ext/extensions/spatial.sigmf-ext.md