Re: [Discuss-gnuradio] comments on stream tags and metadata storage

[Marcus Müller]

Hi Peter,
I'm on very limited time and internet, so let me just answer things
in-text, shortening where contributing to readability.
On 25.07.2014 23:37, Peter A. Bigot wrote:
> I've added five issues to cover the topics from my original email and
> followups.
>
> http://gnuradio.org/redmine/issues/698 proposes that the key of a
> stream tag be a namespaced identifier to avoid conflicts between
> individually-developed components.
That's a good idea, and actually, standarizing the way stream tags and
messages should be named and structured was one of the points of the
last dev call, and is thus work to be done.
>
> The other four are listed inline below with additional responses to
> Marcus:
>
> On 07/25/2014 08:46 AM, Marcus Müller wrote:
>> < issue at hand: inserting tags only seems save from within work, but
>> this is documented nowhere>
>>
>> sooo let me just whip up a few comments:
>> (1) That's a documentation issue, isn't it?
>
> I don't believe so.
>
> http://gnuradio.org/redmine/issues/699 expands on point (1), that
> add_item_tag() can only be safely called from within gr::block::work().
And I have to contradict Sean and you :)
Inserting tags *is* safe at any time. You get logical errors when you
don't take care that you don't attach tags to your stream that belong in
ranges that might already have been processed. This is logical; I do
think it would be a good idea to explicitely state it in the documentation.
However, there's no reason not to insert tags anywhere else, just make
sure that nitems_written() is bigger than your inserted tags' offsets.

>
>
>>   Anyway, I'm not quite sure
>> you're right; the insertion of tags is mutexed IIRC, and the
>> get_tags_in_range() functionality, too, so once the user got his vector
>> of tags, that won't change anymore. There's the possibility that he
>> *misses* some of the tags for the range that get inserted after he
>> got_tags_in_range(), but that's only fair -- it's quite intuitive not to
>> insert tags after you've handed off the samples to which they would
>> belong to downstream tags.
>
> I wasn't concerned about tags across multiple calls to
> get_tags_in_range(), but for tags that are added to a stream after one
> call to work() and before the end of the next call to work().
I'm not sure I understand you correctly.
If (general_)work gets called, the samples that can be processed within
that call to work have left the upstream block's work function already.
If that upstream block decides that he wants to insert tags to samples
he produced in a prior call already, my comment to (1) and basic "how
the hell is this going to look causal to the block downstream" applies!
>
>
>> (3) I don't see the relation to the discussion, as you said ;) but that
>> sounds like a bug, so if you opened up a new thread or filed a bug at
>> gnuradio.org, that would be awesome :)
>
> http://gnuradio.org/redmine/issues/700 records point (3), that GRC
> parameter callbacks can be invoked multiple times as a result of a
> single user action due to the architecture of GUI and other components.
Thanks! Sounds hard to fix, though, if you say it's due to architecture.
>
>> (4) I'm fairly certain the buffers use a deque to store tags, not a map
>> of any kind. So maybe I'm misunderstanding you, or you misread code
>> somewhere?
>> I think what you're describing might be a bug in metadata_filesink, so
>> that might need some attention! see (3).
>
> Yes, the lead-in example was specific to the behavior of
> file_metadata_sink, but the fundamental requirement goes across the
> entire infrastructure.  I'm hoping to see support for making that
> requirement stick.
It is *NOT* a requirement (so far). So far it is the rule that everyone
that expects multiple tags will do the sorting. And this makes sense --
computationally. I don't think we should "break" correctness of existing
blocks by taking away that requirement from the tag readers just to
shift it to the tag producers. So I strongly disagree with you here, sorry.


>
> http://gnuradio.org/redmine/issues/701 expands on points (2) and (4),
> that the infrastructure should promise to maintain all tags inserted
> by blocks in their original order (for any sample offset) and should
> document the situations where tags may be discarded by the
> infrastructure.
>
>
Also, I don't understand the bullet points at the end of your ticket #701.
* preserve order of insertion: GR does this, so how is this an issue?
* preserve incoming tag order: I'm not quite sure I get your point, but
if I do: GR does this, so how is this an issue?
* never discard a tag at the block that inserted it: Why would someone
discard a tag he inserted it? GR doesn't discard tags, so how is this an
issue?
* clearly document duplicate elimination process: duplicates are *not*
detected, so how is this an issue?

Seriously, tag insertion is *this* 2 lines of code
(gnuradio-runtime/lib/buffer_detail.cc):
  void
  buffer::add_item_tag(const tag_t &tag)
  {
    gr::thread::scoped_lock guard(*mutex());
    d_item_tags.push_back(tag);
  }

So how did you come to the conclusion that there might be race
conditions, necessity to call it only from work, elimination and
reordering problems?
All of the problems you raise are logical problems that can -- in my
private opinion -- best be solved by what should be called a contract,
ie. "do not shoot yourself in the knee by inserting tags to item numbers
that you already processed, and do not read tags from samples you don't
have yet. If you need your tags sorted, sort 'em", which basically boils
down to "um, you now, don't try to circumvent causality by using tags,
and by the way, don't assume they're sorted", which, for the signal
processing guys can further be reduced to "tags aren't per se sorted.
Usual considerations apply."

As a serious attempt to get something like a private conclusion to this
discussion, I'd like to point out that so far tag propagation works ok;
the performance of get_tags_in_range leaves a lot of room for concepts
that perform better, but that should be applied under the hood; if there
are two tags at the same item: someone put them there, let's keep it
like that. If tags are in a certain order: we should keep it like that
(which metadata filesink seems to break, so thanks for pointing that
out!). If you insert tags to items that have been processed in the past:
don't assume someone else will ever look at them. If you're expecting
tags to be in a specific order and don't know who inserted them: sort
em. In most cases, this is a non-issue because tags will usually be
inserted to mark something in a stream, and therefore get the
item-chronological order automatically.

>> (5)>(5) All stream tags are placed in the extras block,
>> sorry, can't follow you there. Extras block?
>
> The extras block is described in the section "Extras Information" at:
> http://gnuradio.org/doc/doxygen/page_metadata.html
>
This is all about the on-disk storage format of the metadata file sink,
so it's not really related to the way tags are handled. Maybe we're
misunderstanding each other, but there is a section in the file format
for extras.

> http://gnuradio.org/redmine/issues/702 records points (2) and (5),
> that the existing file_meta_source/sink corrupt (IMO) the metadata.
>
Thanks for that bug report!

> Again I may not have made it clear that I'm not intending to raise
> these simply as complaints.  I have the necessary software skills to
> fix them, and they generally include an outline of the approach I'd
> propose.  At this time I won't make any promises, but I'd consider
> contributing the necessary effort if I can get enough
> feedback/sanity-checks/interface validation/similar assistance from
> you folks to be confident that it'd be seen as a worthwhile enhancement.
:)
Greetings,
Marcus