Re: [server] streaming: Don't start streaming if there is no video encoder

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 27, 2016 at 05:00:03PM +0300, Uri Lublin wrote:
> On 07/25/2016 07:37 PM, Francois Gouget wrote:
> > On Sun, 24 Jul 2016, Uri Lublin wrote:
> > [...]
> > > This patch prevents a crash as few lines below there is access
> > > to agent->video_encoder->codec_type.
> > > 
> > > I think it would be better to make the check it in
> > > dcc_create_stream(), and replace the patch here to assert.
> > 
> > I think it makes more sense to have it this way.
> > 
> > Even if we fail in dcc_create_stream() the server will still keep
> > detecting screen updates that look like a video. Without a corresponding
> > stream, each time it will and try to create one and fail. Each attempt
> > will waste time attempting to create not only the stream object itself,
> > but potentially also the video encoder object[1].
> > 
> > So it's better to keep the corresponding stream object around and skip
> > attempts to recreate it.
> 
> No need to fail dcc_create_stream, just skip adding
> the item to the pipe in such a case.
> There is no point of adding an item of type
> RED_PIPE_ITEM_TYPE_STREAM_CREATE to the pipe
> just to ignore it later.
> 
> Theoretically, in such a scenario there is no need to
> detect streams at all but that's a bigger change.
> 
> Anyway, the patch is good and does prevent a crash, so
> ack from me.

Thanks, I've pushed it now.

Christophe

Attachment: signature.asc
Description: PGP signature

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

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]