Re: [PATCH spice-gtk] Update dropping frames debug messages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,


On 07/16/2018 10:52 AM, Frediano Ziglio wrote:
Since non-mjpeg decoders are available in spice-gtk late frames
are not always dropped
This is confusing, the reason is not simply other decoders but that
in some of the other decoders we support dropping data like this causes
terrible artifacts. Is also not exactly a problem of the decoder (as a
piece of software) but of the streaming format (so independently of the
software).

Hmm, indeed, inside my head it sounds more like - decoders that support codecs
other than mjpeg (non-mjpeg decoders?) are not likely to drop these non-jpeg
frames .

Will "Decoded frames are not necessarily dropped, update related debug messages
and comment" be sufficient?


---
In stats late frames are still counted & named as dropped meanwhile i
leave it as is.
---
  src/channel-display-mjpeg.c | 1 +
  src/channel-display.c       | 3 ++-
  2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c
index 7b2f775..6f7ded6 100644
--- a/src/channel-display-mjpeg.c
+++ b/src/channel-display-mjpeg.c
@@ -262,6 +262,7 @@ static gboolean mjpeg_decoder_queue_frame(VideoDecoder
*video_decoder,
       * So drop late frames as early as possible to save on processing time.
       */
      if (latency < 0) {
+        SPICE_DEBUG("dropping a late MJPEG frame");
          frame->free(frame);
          return TRUE;
      }
diff --git a/src/channel-display.c b/src/channel-display.c
index 44ba043..9a9bc99 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -1544,11 +1544,12 @@ static void display_handle_stream_data(SpiceChannel
*channel, SpiceMsgIn *in)
latency = op->multi_media_time - mmtime;
      if (latency < 0) {
-        CHANNEL_DEBUG(channel, "stream data too late by %u ms (ts: %u,
mmtime: %u), dropping",
+        CHANNEL_DEBUG(channel, "stream data too late by %u ms (ts: %u,
mmtime: %u)",
                        mmtime - op->multi_media_time, op->multi_media_time,
                        mmtime);
          st->arrive_late_time += mmtime - op->multi_media_time;
          st->arrive_late_count++;
+ /* Late frames are counted as drops in the stats but aren't
necessarily dropped - depends on codec and decoder */
Line too long for style.
Would not be clearer to put this comment before the debug message?

Actually i put this comment here since it is counted in the stats as
dropped, but it is not actually dropped, this naming may\should be
changed in several components (or even changing the way stats are
being collected in other codecs? idk)
So meanwhile i have just leaved it as is and added this comment
to make it clear (different patch?)


          if (!st->cur_drops_seq_stats.len) {
              st->cur_drops_seq_stats.start_mm_time = op->multi_media_time;
          }
Frediano
Thanks
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]