Re: [PATCH spice-server 2/7] VideoStream: store channel in stream

[Uri Lublin <uril@xxxxxxxxxx>]

On 11/29/2017 10:16 PM, Jonathon Jongsma wrote:
> This allows us to unref the stream directly rather than needing to pass
> the associated DisplayChannel to stream_unref(). The same is also true
> for stream_agent_unref, since the only reason that stream_agent_unref()
> required a DisplayChannel parameter was to pass it to stream_unref().
> This also resulted in some additional minor redesign due to the fact
> that streams are pre-allocated in a pool in the display channel, and
> freeing a stream simply means moving it to a different ring to be
> re-used.
>
> Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> ---
>   server/display-channel.c | 27 ++++++++++++++++++++++++++-
>   server/display-channel.h |  1 +
>   server/video-stream.c    | 44 ++++++++++++--------------------------------
>   server/video-stream.h    |  8 ++++----
>   4 files changed, 43 insertions(+), 37 deletions(-)
>
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 2caaa6433..d3db1e0db 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c

> +
> +void display_channel_free_video_stream(DisplayChannel *display, VideoStream *stream)

Hi Jonathon,

You do not need to pass "display" here. Just use stream->display.

> +{
> +    display_channel_mark_stream_unused(stream->display, stream);
> +    display->priv->stream_count--;
> +}
> +


> diff --git a/server/display-channel.h b/server/display-channel.h
> index 4f7def216..a0470ec67 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -168,6 +168,7 @@ gboolean display_channel_surface_has_canvas(DisplayChannel *display, uint32_t su
>   void display_channel_reset_image_cache(DisplayChannel *self);
>   
>   void display_channel_debug_oom(DisplayChannel *display, const char *msg);
> +void display_channel_free_video_stream(DisplayChannel *display, VideoStream *stream);

And here.

>   
>   G_END_DECLS
>   
> diff --git a/server/video-stream.c b/server/video-stream.c
> index 65da3f8a0..a24c787ef 100644
> --- a/server/video-stream.c
> +++ b/server/video-stream.c

<snip>

> -void video_stream_unref(DisplayChannel *display, VideoStream *stream)
> +void video_stream_unref(VideoStream *stream)
>   {
>       if (--stream->refs != 0)
>           return;
>   
>       spice_warn_if_fail(!ring_item_is_linked(&stream->link));
>   
> -    video_stream_free(display, stream);
> -    display->priv->stream_count--;
> +    display_channel_free_video_stream(stream->display, stream);

This is the only caller and it uses stream->display.

Thanks,
    Uri.
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel