>
> 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?
>
sounds good for me.
>
> >> ---
> >> 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?)
>
Same patch is fine. Ok, make sense.
> >
> >> if (!st->cur_drops_seq_stats.len) {
> >> st->cur_drops_seq_stats.start_mm_time =
> >> op->multi_media_time;
> >> }
> Thanks
>
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel