From 4aadf50012cb057422f29644b41fcaa30c1a2dd2 Mon Sep 17 00:00:00 2001
From: Edward Hervey <edward@centricular.com>
Date: Sun, 2 Oct 2016 09:34:51 +0200
Subject: [PATCH] adaptivedemux: Calculate values before queue2

In order to calculate the *actual* bitrate for downloading a fragment
we need to take into account the time since we requested the fragment.

Without this, the bitrate calculations (previously reported by queue2)
would be biased since they wouldn't take into account the request latency
(that is the time between the moment we request a specific URI and the
moment we receive the first byte of that request).

Such examples were it would be biased would be high-bandwith but high-latency
networks. If you download 5MB in 500ms, but it takes 200ms to get the first
byte, queue2 would report 80Mbit/s (5Mb in 500ms) , but taking the request
into account it is only 57Mbit/s (5Mb in 700ms).

While this would not cause too much issues if the above fragment represented
a much longer duration (5s of content), it would cause issues with short
ones (say 1s, or when doing keyframe-only requests which are even shorter)
where the code would expect to be able to download up to 80Mbit/s ... whereas
if we take the request time into account it's much lower (and we would
therefore end up doing late requests).

Also calculate the request latency for debugging purposes and further
usage (it could allow us to figure out the maximum request rate for
example).

https://bugzilla.gnome.org/show_bug.cgi?id=733959

https://bugzilla.gnome.org/show_bug.cgi?id=772330
---
 gst-libs/gst/adaptivedemux/gstadaptivedemux.c | 59 ++++++++++++++++++-
 gst-libs/gst/adaptivedemux/gstadaptivedemux.h |  9 +++
 2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/gst-libs/gst/adaptivedemux/gstadaptivedemux.c b/gst-libs/gst/adaptivedemux/gstadaptivedemux.c
index 715893c48e..a47b19676b 100644
--- a/gst-libs/gst/adaptivedemux/gstadaptivedemux.c
+++ b/gst-libs/gst/adaptivedemux/gstadaptivedemux.c
@@ -1882,8 +1882,7 @@ gst_adaptive_demux_stream_update_current_bitrate (GstAdaptiveDemux * demux,
     return demux->connection_speed;
   }
 
-  g_object_get (stream->queue, "avg-in-rate", &fragment_bitrate, NULL);
-  fragment_bitrate *= 8;
+  fragment_bitrate = stream->last_bitrate;
   GST_DEBUG_OBJECT (demux, "Download bitrate is : %" G_GUINT64_FORMAT " bps",
       fragment_bitrate);
 
@@ -2353,6 +2352,58 @@ _src_query (GstPad * pad, GstObject * parent, GstQuery * query)
   return gst_pad_peer_query (stream->pad, query);
 }
 
+static GstPadProbeReturn
+_uri_handler_probe (GstPad * pad, GstPadProbeInfo * info,
+    GstAdaptiveDemuxStream * stream)
+{
+  GstPadProbeReturn ret = GST_PAD_PROBE_OK;
+
+  if (GST_PAD_PROBE_INFO_TYPE (info) & GST_PAD_PROBE_TYPE_BUFFER) {
+    GstBuffer *buf = GST_PAD_PROBE_INFO_BUFFER (info);
+    if (stream->fragment_bytes_downloaded == 0) {
+      stream->last_latency =
+          gst_adaptive_demux_get_monotonic_time (stream->demux) -
+          (stream->download_start_time * GST_USECOND);
+      GST_DEBUG_OBJECT (pad,
+          "FIRST BYTE since download_start %" GST_TIME_FORMAT,
+          GST_TIME_ARGS (stream->last_latency));
+    }
+    stream->fragment_bytes_downloaded += gst_buffer_get_size (buf);
+    GST_LOG_OBJECT (pad,
+        "Received buffer, size %" G_GUINT64_FORMAT " total %" G_GUINT64_FORMAT,
+        gst_buffer_get_size (buf), stream->fragment_bytes_downloaded);
+  } else if (GST_PAD_PROBE_INFO_TYPE (info) &
+      GST_PAD_PROBE_TYPE_EVENT_DOWNSTREAM) {
+    GstEvent *ev = GST_PAD_PROBE_INFO_EVENT (info);
+    GST_LOG_OBJECT (pad, "Received event %s %" GST_PTR_FORMAT,
+        GST_EVENT_TYPE_NAME (ev), ev);
+    switch (GST_EVENT_TYPE (ev)) {
+      case GST_EVENT_SEGMENT:
+        stream->fragment_bytes_downloaded = 0;
+        break;
+      case GST_EVENT_EOS:
+      {
+        stream->last_download_time =
+            gst_adaptive_demux_get_monotonic_time (stream->demux) -
+            (stream->download_start_time * GST_USECOND);
+        stream->last_bitrate =
+            gst_util_uint64_scale (stream->fragment_bytes_downloaded,
+            8 * GST_SECOND, stream->last_download_time);
+        GST_DEBUG_OBJECT (pad,
+            "EOS since download_start %" GST_TIME_FORMAT " bitrate %"
+            G_GUINT64_FORMAT " bps", GST_TIME_ARGS (stream->last_download_time),
+            stream->last_bitrate);
+        /* Calculate bitrate since URI request */
+      }
+        break;
+      default:
+        break;
+    }
+  }
+
+  return ret;
+}
+
 /* must be called with manifest_lock taken.
  * Can temporarily release manifest_lock
  */
@@ -2563,6 +2614,10 @@ gst_adaptive_demux_stream_update_source (GstAdaptiveDemuxStream * stream,
       return FALSE;
     }
 
+    /* Add a downstream event and data probe */
+    gst_pad_add_probe (uri_handler_src, GST_PAD_PROBE_TYPE_DATA_DOWNSTREAM,
+        (GstPadProbeCallback) _uri_handler_probe, stream, NULL);
+
     g_object_unref (queue_sink);
     g_object_unref (uri_handler_src);
     queue_src = gst_element_get_static_pad (queue, "src");
diff --git a/gst-libs/gst/adaptivedemux/gstadaptivedemux.h b/gst-libs/gst/adaptivedemux/gstadaptivedemux.h
index e876600add..780f4d93fa 100644
--- a/gst-libs/gst/adaptivedemux/gstadaptivedemux.h
+++ b/gst-libs/gst/adaptivedemux/gstadaptivedemux.h
@@ -167,6 +167,15 @@ struct _GstAdaptiveDemuxStream
   gint64 download_total_bytes;
   guint64 current_download_rate;
 
+  /* amount of data downloaded in current fragment (pre-queue2) */
+  guint64 fragment_bytes_downloaded;
+  /* bitrate of the previous fragment (pre-queue2) */
+  guint64 last_bitrate;
+  /* latency (request to first byte) and full download time (request to EOS)
+   * of previous fragment (pre-queue2) */
+  GstClockTime last_latency;
+  GstClockTime last_download_time;
+
   /* Average for the last fragments */
   guint64 moving_bitrate;
   guint moving_index;