Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pdu support #43

Open
jsallay opened this issue Feb 16, 2022 · 6 comments
Open

Add pdu support #43

jsallay opened this issue Feb 16, 2022 · 6 comments

Comments

@jsallay
Copy link
Collaborator

jsallay commented Feb 16, 2022

We would like to have some form of support for pdus. In the short term, there are a few questions that need to be answered to put this in. The new pmts provide more flexibility and stronger typing.

  1. What exactly is a pdu? The short answer is a dictionary and a vector of data. There was a question on the mailing list a little while back about have a dictionary and a string. The question really is, should a pdu be a (dict, vector) or a (dict, pmt)?
  2. Should there be one pdu for any type of vector or one per type of vector (i.e. is there a pdu<int>, pdu<float>, etc or just a pdu.) This has implications on the functions that one writes to process messages. If my function has a signature void proc(pmtf::pdu<float> f then I know that the data vector that I receive will be of the correct type.
  3. Should there be any limits on the dictionary portion of the pdu?

In the long term, I think there are some important questions to answer about how metadata is handled in gnuradio. There is a desire to make it easy to write one processing function that works for streaming or packet based processing. I do not believe that there is an easy way to do this given the differences between tags and pdus. They just handle metadata differently. I'm not necessarily proposing that we figure out the perfect solution right now. I would like to know if there are things that can be done as I am implementing pdus that push us in the right direction. We are also going to want to add support for tags, so this could be of value there too.

I would argue that with tags and pdus, we have 3 kinds of metadata.

  1. Metadata that applies to a single sample (tags)
  2. Metadata that applies forever or until superceeded. (tags)
  3. Metadata that applies to a fixed duration of data. (pdus)

There are many possible solutions:

  1. Make tags more like pdus. The work function would receive a pdu instead of a buffer. There would be no need to call functions like get_tags_in_window.
  2. Make pdus more like tags. Add in functions such as get_tags_in_window that work on pdus.
  3. Support both in both places. The idea would be that we expand pdus and tags to support the 3 metadata cases above in a consistent manner.

I would like to hear thoughts on the matter.
@mormj @jacobagilbert

@mormj
Copy link
Contributor

mormj commented Feb 16, 2022

Thank you for bringing this up!

Should there be one pdu for any type of vector or one per type of vector (i.e. is there a pdu, pdu, etc or just a pdu.) This has implications on the functions that one writes to process messages. If my function has a signature void proc(pmtf::pdu f then I know that the data vector that I receive will be of the correct type.

This is an interesting question - I know there are some PDU handlers that will query the type of vector or use the metadata to tell it how to decode the vector. If we used strong typing of the vector, we would lose that ability. I don't have a good use case, but one of the nice things about the PDU datatype is its ability to be self descriptive

Make tags more like pdus. The work function would receive a pdu instead of a buffer. There would be no need to call functions like get_tags_in_window.

I like this idea - it is loosely like this now in that the tags are associated with the buffer passed into the work function rather than in gnuradio where the tags are attached to the scheduler and have to be searched. But it could use some work to make it less attached to the buffer object.

@jacobagilbert
Copy link

What exactly is a pdu?

I may be biased given my familiarity, but i see no reason we should deviate from [dict, vector]. I believe allowing any type object in the cdr would make wider use of PDUs more complicated. As data is distilled it can easily be added to metadata fields, and any binary information (including strings) can readily be represented as U8 bytes.

Should there be one pdu for any type of vector or one per type of vector

PDU vector type checking is something that is not well handled now, and needs some improvement. That said, strongly typing PDUs based on their vector-type is probably not what I would recommend, there are many cases where a block might want to handle many types of PDU and requiring a specific port for each would be somewhat burdensome. A referencefor making templatized message handlers would be very nice for straightforward multi-type PDU blocks though.

Should there be any limits on the dictionary portion of the pdu?

I do not think the library should structure limits on this, though the effort as a whole would benefit from some best practices on keynaming.

Regarding tags/PDUs: one other important difference is that PDUs are inherently processed asynchronously. Per the discussion here gnuradio/newsched#190 I think it would be somewhat reasonable to make tags an analog for PDU metadata (and thus must be a dict). There is a strong analog between these two types of data, though you bring up a couple possible interpretations of tags that might be good to clarify. Maybe we should add some additional structure to tags (maybe an optional uint64 tag_range field that defines the scope of the tag:

0: indefinitely (if omitted this is implied)
1: this item
n: for n item starting with the tagged item

This is conceptually back/fwd compatible (default = 0), and would enable better defined coupling between PDUs and streams. I am also thinking of this from a SigMF perspective, and there are some nice analogs there too (captures fields would have explicit scope).

@mormj
Copy link
Contributor

mormj commented Feb 16, 2022

Also, if a PDU is strictly a dict, vector, a tag doesn't (or shouldn't) need to have a vector component. So, just a PMT dict for tags for maximal flexibility seems reasonable. It would have 3 or 4 fixed fields (offset, key, value)

@jsallay
Copy link
Collaborator Author

jsallay commented Feb 16, 2022

I like the idea of augmenting a tag to be able to represent a range. I think it might be good as well to think about a good way to represent a tag within a pdu. For example, I have a burst and want to say that something happens at a specific sample.
Perhaps, we make a tag a pmt type and then a pdu could contain tags in the dictionary.

@jsallay
Copy link
Collaborator Author

jsallay commented Feb 16, 2022

Also, I would strongly support getting rid of the car and cdr naming and replace it with something more meaningful, such as meta and data.

@jacobagilbert
Copy link

Removal of car/cdr is good and we could use your suggestion for PDUs though something more generic for any pair type object would also be helpful. Not really sure how the new PMT lib addresses general pairs....but something to consider.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants