[Spice-devel] [PATCH spice-gtk] display: video streaming: add support for frames of different sizes

[Marc-André Lureau]

Hi

Minor comments below

On Wed, May 2, 2012 at 4:05 PM, Yonit Halperin <yhalperi at redhat.com> wrote:
> --- a/gtk/channel-display-priv.h
> +++ b/gtk/channel-display-priv.h
> @@ -73,6 +73,11 @@ typedef struct display_stream {
>     SpiceChannel                *channel;
>  } display_stream;
>
> +void stream_get_dimensions(display_stream *st, int *width, int *height);
> +SpiceRect *stream_get_dest(display_stream *st);
> +uint32_t stream_get_flags(display_stream *st);
> +uint32_t stream_get_mjpeg_frame(display_stream *st, uint8_t **data);

We don't declare functions in headers that aren't used outside of
their source files.

> +void stream_get_dimensions(display_stream *st, int *width, int *height)
> +{

This one is correctly in priv header, but doesn't have
G_GNUC_INTERNAL, so it's not obvious if it 's a public function when
reading it.

> +SpiceRect *stream_get_dest(display_stream *st)

This function doesn't need to be in header, and should be static.

> +uint32_t stream_get_flags(display_stream *st)

same apply for this one

> +uint32_t stream_get_mjpeg_frame(display_stream *st, uint8_t **data)

That one could be moved to gtk/channel-display-mjpeg.c and made
static. Or perhaps the name should be changed since it's in fact not
mjpeg specific.



-- 
Marc-André Lureau