[spice-common,2/2] RFC protocol: Allows to send partial frame data

Submitted by Frediano Ziglio on Nov. 6, 2017, 5:13 p.m.

Details

Message ID 20171106171311.12305-3-fziglio@redhat.com
State New
Headers show
Series "RFC protocol: Allows to send partial frame data" ( rev: 4 3 2 ) in Spice

Not browsing as part of any series.

Commit Message

Frediano Ziglio Nov. 6, 2017, 5:13 p.m.
Reduce the needs to buffer the entire frame and than send
and on the other end to wait the entire frame before processing.
Some encodings allow to start processing before having a full
frame allowing to reduce latency and buffering with huge frames.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
 common/messages.h | 6 ++++++
 spice.proto       | 9 +++++++++
 2 files changed, 15 insertions(+)

Patch hide | download patch | download mbox

diff --git a/common/messages.h b/common/messages.h
index b838881..18e2036 100644
--- a/common/messages.h
+++ b/common/messages.h
@@ -368,6 +368,12 @@  typedef struct SpiceMsgDisplayStreamDestroy {
     uint32_t id;
 } SpiceMsgDisplayStreamDestroy;
 
+typedef struct SpiceMsgDisplayStreamPartialData {
+    SpiceStreamDataHeader base;
+    uint32_t data_size;
+    uint8_t *data;
+} SpiceMsgDisplayStreamPartialData;
+
 typedef struct SpiceMsgDisplayStreamActivateReport {
     uint32_t stream_id;
     uint32_t unique_id;
diff --git a/spice.proto b/spice.proto
index 2896966..8e88649 100644
--- a/spice.proto
+++ b/spice.proto
@@ -771,6 +771,15 @@  channel DisplayChannel : BaseChannel {
 
     Empty stream_destroy_all;
 
+    /* This message is used to transmit part of a stream frame.
+     * Last stream frame data chunk should be sent with a stream_data
+     * message.
+     */
+    message {
+	StreamDataHeader base;
+	uint8 data[] @as_ptr(data_size) @nomarshal;
+    } stream_partial_data;
+
     message {
 	DisplayBase base;
 	struct Fill {

Comments

Frediano Ziglio writes:

> Reduce the needs to buffer the entire frame and than send
> and on the other end to wait the entire frame before processing.
> Some encodings allow to start processing before having a full
> frame allowing to reduce latency and buffering with huge frames.
>
> Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> ---
>  common/messages.h | 6 ++++++
>  spice.proto       | 9 +++++++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/common/messages.h b/common/messages.h
> index b838881..18e2036 100644
> --- a/common/messages.h
> +++ b/common/messages.h
> @@ -368,6 +368,12 @@ typedef struct SpiceMsgDisplayStreamDestroy {
>      uint32_t id;
>  } SpiceMsgDisplayStreamDestroy;
>
> +typedef struct SpiceMsgDisplayStreamPartialData {
> +    SpiceStreamDataHeader base;
> +    uint32_t data_size;
> +    uint8_t *data;
> +} SpiceMsgDisplayStreamPartialData;

Why put the SpiceMsgDisplayStreamPartialData after
SpiceMsgDisplayStreamDestroy and not after SpiceMsgDisplayStreamDataSized?

> +
>  typedef struct SpiceMsgDisplayStreamActivateReport {
>      uint32_t stream_id;
>      uint32_t unique_id;
> diff --git a/spice.proto b/spice.proto
> index 2896966..8e88649 100644
> --- a/spice.proto
> +++ b/spice.proto
> @@ -771,6 +771,15 @@ channel DisplayChannel : BaseChannel {
>
>      Empty stream_destroy_all;
>
> +    /* This message is used to transmit part of a stream frame.
> +     * Last stream frame data chunk should be sent with a stream_data
> +     * message.
> +     */
> +    message {
> +	StreamDataHeader base;
> +	uint8 data[] @as_ptr(data_size) @nomarshal;
> +    } stream_partial_data;

Same comment about the location?

> +
>      message {
>  	DisplayBase base;
>  	struct Fill {


--
Cheers,
Christophe de Dinechin (IRC c3d)
> 
> Frediano Ziglio writes:
> 
> > Reduce the needs to buffer the entire frame and than send
> > and on the other end to wait the entire frame before processing.
> > Some encodings allow to start processing before having a full
> > frame allowing to reduce latency and buffering with huge frames.
> >
> > Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
> > ---
> >  common/messages.h | 6 ++++++
> >  spice.proto       | 9 +++++++++
> >  2 files changed, 15 insertions(+)
> >
> > diff --git a/common/messages.h b/common/messages.h
> > index b838881..18e2036 100644
> > --- a/common/messages.h
> > +++ b/common/messages.h
> > @@ -368,6 +368,12 @@ typedef struct SpiceMsgDisplayStreamDestroy {
> >      uint32_t id;
> >  } SpiceMsgDisplayStreamDestroy;
> >
> > +typedef struct SpiceMsgDisplayStreamPartialData {
> > +    SpiceStreamDataHeader base;
> > +    uint32_t data_size;
> > +    uint8_t *data;
> > +} SpiceMsgDisplayStreamPartialData;
> 
> Why put the SpiceMsgDisplayStreamPartialData after
> SpiceMsgDisplayStreamDestroy and not after SpiceMsgDisplayStreamDataSized?
> 
> > +
> >  typedef struct SpiceMsgDisplayStreamActivateReport {
> >      uint32_t stream_id;
> >      uint32_t unique_id;
> > diff --git a/spice.proto b/spice.proto
> > index 2896966..8e88649 100644
> > --- a/spice.proto
> > +++ b/spice.proto
> > @@ -771,6 +771,15 @@ channel DisplayChannel : BaseChannel {
> >
> >      Empty stream_destroy_all;
> >
> > +    /* This message is used to transmit part of a stream frame.
> > +     * Last stream frame data chunk should be sent with a stream_data
> > +     * message.
> > +     */
> > +    message {
> > +	StreamDataHeader base;
> > +	uint8 data[] @as_ptr(data_size) @nomarshal;
> > +    } stream_partial_data;
> 
> Same comment about the location?
> 

Apparently when stream_data_sized was added nobody noted that
the messages related to streaming were together and just added
to the end. I just didn't do the same mistake :-)

> > +
> >      message {
> >  	DisplayBase base;
> >  	struct Fill {
> 

Frediano