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.
> What happens with the stream in that case, dropped ?
The stream object will not have an associated video_encoder.
red_marshall_stream_data() will notice that and return FALSE which will
cause the frame to be sent as a regular screen update (same as when the
frame format is not supported, e.g. if it's too small ( typically < 16
pixels in one dimension)).
[1] In the specific case I mentioned we would not even attempt to
create a video encoder. But there may be cases where we attempt to
create the video encoder only to have that fail, for instance if
create_pipeline() starts failing for whatever reason. So repeated
attempts could start being a bit expensive.
--
Francois Gouget <fgouget@xxxxxxxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel