about feedback - rohc_feedback_flush()

Asked by Friedrich on 2013-08-12

Hi Didier,

I am opening a new thread for being easy to read, but actually, it follows our discussion about the standalone feedback.

-------------from Didier----------------
You can call rohc_feedback_flush() at any rate you want. The more you wait, the more feedback data you will get: sending every feedback chunk separately may have a cost (more link layer overhead for example). The more you wait, the more delay you add to the transmission of the feedback: negative feedbacks are only useful if they reach quickly the other endpoint. So, you need to do some tradeoffs according to your own use case ;-)
------------------------------------------

I was able to use rohc_feedback_flush() to send standalone feedback. But I had to call it all the time even maybe no feedback packets there. Right now, I want to implement a simple logic to check if there is feedback indeed:

If (there is a feedback packet in local compressor & no piggyback available)
{send standalone feedback by using rohc_feedback_flush()}
elseif (there is a feedback packet & there is reverse compressed-packet to send)
{send piggyback feedback by using rohc_decompress()}
end

My major concern is how to check if the feedback packet does exist. It is better not to change the library but to check it through API.

Thanks,
Friedrich

Question information

Language:
English Edit question
Status:
Solved
For:
rohc Edit question
Assignee:
No assignee Edit question
Solved by:
Friedrich
Solved:
2013-08-20
Last query:
2013-08-20
Last reply:
2013-08-14

> I want to implement a simple logic to check if there is feedback indeed:
>
> If (there is a feedback packet in local compressor & no piggyback available)
> {send standalone feedback by using rohc_feedback_flush()}
> elseif (there is a feedback packet & there is reverse compressed-packet to send)
> {send piggyback feedback by using rohc_decompress()}
> end
>
> My major concern is how to check if the feedback packet does exist. It is
> better not to change the library but to check it through API.

Your algorithm might simplified to:

   If (no piggyback available)
       {send standalone feedback by using rohc_feedback_flush()}
   elseif (there is reverse compressed-packet to send)
       {send piggyback feedback by using rohc_decompress()}
   end

So, it looks like you don't need a way to check if a not-yet-sent feedback packet is stored in the compressor ;-)

Joke aside, the algorithm you described is probably incomplete or simplified. If the full algo requires the information, we can extend the library API to add a new function. But first tell me what is your full algorithm so that we could design the function the best way.

Regards,
Didier

Friedrich (hitlbs) said : #2

Hi Didier,

Thanks for your response.

In your simplified algo, what method or function do we use to know "no piggyback available" or "there is reverse compressed-packet to send"?

Friedrich (hitlbs) said : #3

I don't have the full algo ready in mind yet, but the main thing is to have a balance between piggyback feedback (efficient) and standalone feedback (effective).

Check if there is a feedback packet to send,
     if true,
          Wait N cycles,
                if there is piggyback available during the N cycles, go with piggyback;
                if there is no piggyback available during the N cycles, send standalone feedback at N.
     if false,
         Do not call rohc_feedback_flush().

N is configurable.
Therefore, I need to know how check such conditions as
"if there is a feedback packet to send"
" if there is piggyback available"

Thanks,
Friedrich

Hi,

> In your simplified algo, what method or function do we use to know "no
> piggyback available" or "there is reverse compressed-packet to send"?

When your application calls the rohc_compress2() function.

> I don't have the full algo ready in mind yet, but the main thing is to
> have a balance between piggyback feedback (efficient) and standalone
> feedback (effective).
>
> Check if there is a feedback packet to send,
> if true,
> Wait N cycles,
> if there is piggyback available during the N cycles,
> go with piggyback; if there is no piggyback available during the N
> cycles, send standalone feedback at N. if false,
> Do not call rohc_feedback_flush().
>
> N is configurable.
> Therefore, I need to know how check such conditions as
> "if there is a feedback packet to send"
> " if there is piggyback available"

What do you think of the algo below?

  Every N cycles:
    // check if there is feedback data to send
    // (if yes, retrieve it altogether)
    (bytes_nr, bytes) := rohc_feedback_flush()
    // if there is feedback data, send it now
    if bytes_nr > 0:
      send(bytes, bytes_nr)

That's not exactly your algorithm: this one does not wait N cycles
after the feedback data was generated (feedback data generated at
cycle N-1 will be sent at cycle N for example), but it balances
piggybacking vs. feedback-only packets.

If you prefer your algorithm, then we could add a new API function
such as:

  /**
   * @brief Whether there is unsent feedback data available at compressor
   *
   * @param comp The ROHC compressor
   * @return true if there is at least 1 byte of unsent feedback data,
   * false if not
   *
   * @ingroup rohc_comp
   */
  bool rohc_feedback_avail(const struct rohc_comp *const comp)
    __attribute__((warn_unused_result));

Or:

  /**
   * @brief How many bytes of unsent feedback data are available at compressor?
   *
   * @param comp The ROHC compressor
   * @return The number of bytes of unsent feedback data,
   * 0 if no unsent feedback data is available
   *
   * @ingroup rohc_comp
   */
  size_t rohc_feedback_avail_bytes(const struct rohc_comp *const comp)
    __attribute__((warn_unused_result));

The second one seems to be a better choice: your algorithm could be
enhanced to take the amount of feedback data into account for example.

What do you think of?

Regards,
Didier

Friedrich (hitlbs) said : #5

Hi Didier,

Yes, the second one looks well since that API has more information.

I am still working on version 1.3.1 and version 1.5.1 , so could you please provide a method or API for that? I guess something that has the following flavor would work for me. Of course, I believe your real codes will be much more complicated with all error protections :-). I will put it into the rohc_comp.c file at my end. Thanks!

/**
 * @brief How many bytes of unsent feedback data are available at compressor?
   *
   * @param comp The ROHC compressor
   * @return The number of bytes of unsent feedback data,
   * 0 if no unsent feedback data is available
   *
   * @ingroup rohc_comp
   */

static int rohc_feedback_avail_bytes(struct rohc_comp *const comp));
{
 size_t feedback_length;

 if(comp->feedbacks_first_unlocked != comp->feedbacks_next)
 {
  feedback_length = comp->feedbacks[comp->feedbacks_first_unlocked].length;
 }
 else
 {
  feedback_length = 0;
 }

 rohc_debugf(2, "there are %zd byte(s) of feedback data", feedback_length);

 /* return the length of the feedback header/data,
  * or zero if no feedback */
 return feedback_length;
}
/////////////////////////////////////////////////////////////////////////////////////////////////////

Hi,

> Yes, the second one looks well since that API has more information.
>
> I am still working on version 1.3.1 and version 1.5.1 , so could you
> please provide a method or API for that? I guess something that has
> the following flavor would work for me. Of course, I believe your
> real codes will be much more complicated with all error
> protections :-). I will put it into the rohc_comp.c file at my end.

In addition to error checking missing, your code only takes the length
of the 1st unlocked feedback... if it even exists.

The feedbacks_first_unlocked index points on the first unlocked
feedback only if there is one. Otherwise it points on the location for
next feedback to come from decompressor.

Last thing, you need to add the length of the feedback header to the
length of data stored in the feedbacks[] array. They are not stored,
they are added dynamically when piggybacking or flushing.

I coded the version below for main dev branch (ie. future 1.7.0).
It should work for 1.5.x if you replace the call to rohc_comp_debug()
by a call to rohc_debugf(). Let me know if it works for you. If so,
I'll commit it and it will be part of next 1.7.0 release.

Regards,
Didier

=== modified file 'src/comp/rohc_comp.c'
--- src/comp/rohc_comp.c 2013-08-12 17:01:11 +0000
+++ src/comp/rohc_comp.c 2013-08-15 16:18:48 +0000
@@ -2552,6 +2552,58 @@ int rohc_feedback_flush(struct rohc_comp
 }

+/**
+ * @brief How many bytes of unsent feedback data are available at compressor?
+ *
+ * @param comp The ROHC compressor
+ * @return The number of bytes of unsent feedback data,
+ * 0 if no unsent feedback data is available
+ *
+ * @ingroup rohc_comp
+ */
+size_t rohc_feedback_avail_bytes(const struct rohc_comp *const comp)
+{
+ size_t feedback_length;
+ size_t i;
+
+ /* check input validity */
+ if(comp == NULL)
+ {
+ goto error;
+ }
+
+ feedback_length = 0;
+ for(i = 0; i < FEEDBACK_RING_SIZE; i++)
+ {
+ /* take only defined, unlocked feedbacks into account */
+ if(comp->feedbacks[i].length > 0 && !comp->feedbacks[i].is_locked)
+ {
+ /* retrieve the length of the feedback data */
+ feedback_length += comp->feedbacks[i].length;
+
+ /* how many additional bytes are required to encode length? */
+ if(feedback_length < 8)
+ {
+ feedback_length++;
+ }
+ else
+ {
+ feedback_length += 2;
+ }
+ }
+ }
+
+ rohc_debug(comp, ROHC_TRACE_COMP, ROHC_PROFILE_GENERAL,
+ "there are %zu byte(s) of available unsent feedback data\n",
+ feedback_length);
+
+ return feedback_length;
+
+error:
+ return 0;
+}
+
+
 #if !defined(ENABLE_DEPRECATED_API) || ENABLE_DEPRECATED_API == 1

 /**

=== modified file 'src/comp/rohc_comp.h'
--- src/comp/rohc_comp.h 2013-08-12 17:01:11 +0000
+++ src/comp/rohc_comp.h 2013-08-15 16:22:23 +0000
@@ -412,6 +412,8 @@ bool ROHC_EXPORT rohc_comp_deliver_feedb
 int ROHC_EXPORT rohc_feedback_flush(struct rohc_comp *comp,
                                     unsigned char *obuf,
                                     int osize);
+size_t ROHC_EXPORT rohc_feedback_avail_bytes(const struct rohc_comp *const comp)
+ __attribute__((warn_unused_result));
 bool ROHC_EXPORT rohc_feedback_remove_locked(struct rohc_comp *const comp)
  __attribute__((warn_unused_result));
 bool ROHC_EXPORT rohc_feedback_unlock(struct rohc_comp *const comp)

Friedrich (hitlbs) said : #7

Thanks Didier. Having put the codes in the places, the compilation looked perfect. But there are two strange outputs I could not understand so far. I am providing the logs.

1.
cycle W:
                rohc_comp.c: There are 5 byte(s) of available unsent feedback data.
                call_feedback_flush: Output of rohc_feedback_flush(): 5 bytes

cycle X:
                rohc_comp.c: There are 11 byte(s) of available unsent feedback data.
                call_feedback_flush: Output of rohc_feedback_flush(): 5 bytes

cycle Y:
                rohc_comp.c: There are 17 byte(s) of available unsent feedback data.
                call_feedback_flush: Output of rohc_feedback_flush(): 5 bytes

....

cycle Z:
                rohc_comp.c: There are 59 byte(s) of available unsent feedback data.
                call_feedback_flush: Output of rohc_feedback_flush(): 5 bytes

Question is: why does the unsent feedback bytes keep increasing by interval 6 bytes? It seems that it still counted the flushed feedback bytes.

2.
In cycle X, the new feedback bytes are 11-5=6 bytes, I guess. But why does the rohc_feedback_flush() only output 5 bytes. Nonetheless, it may relate to Point 1 above. I think we can fix Point 1 first, and then touch base on this point.

Hello,

> Thanks Didier. Having put the codes in the places, the compilation
> looked perfect. But there are two strange outputs I could not
> understand so far. I am providing the logs.
>
[...]
>
> Question is: why does the unsent feedback bytes keep increasing by
> interval 6 bytes? It seems that it still counted the flushed feedback
> bytes.

Maybe you aren't flushing quickly enough: how large is the buffer that
you gives to the rohc_feedback_flush() function?

Another thing, there is a small problem with the function I gave to
you. Please change the following line:
  - if(feedback_length < 8)
  + if(comp->feedbacks[i].length < 8)

> 2.
> In cycle X, the new feedback bytes are 11-5=6 bytes, I guess. But why
> does the rohc_feedback_flush() only output 5 bytes. Nonetheless, it
> may relate to Point 1 above. I think we can fix Point 1 first, and
> then touch base on this point.

This looks like a consequence of the problem I tell you above.

Regards,
Didier

Friedrich (hitlbs) said : #9

Thanks, Didier. The fix of +if(comp->feedbacks[i].length < 8) solved the issue.

Friedrich,

Thank you for the feedback. The new function is now in main branch [1]. It will be shipped with next 1.7.0 release.

Regards,
Didier

[1] http://bazaar.launchpad.net/~didier-barvaux/rohc/main/revision/798