[Spice-devel] [PATCH spice-gtk] Update dropping frames debug messages

[Snir Sheriber]

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