[Spice-devel] [PATCH spice-gtk] pulse: do not abort on corking no stream

[Christophe Fergeau]

On Tue, Nov 26, 2013 at 05:26:01PM +0100, Marc-André Lureau wrote:
> Unless you have further concerns or alternative solutions, I am
> considering this reviewed, please ack.

Getting a bit tired of having to ask multiple times the same question...
Did the g_reutrn_val_if_fail trigger in some situation, if yes, is there
another place we need to fix, ... Having all the information about the
why/how/.. in the commit log in the first place would really help the
review.

Christophe

> 
> On Tue, Nov 26, 2013 at 12:27 PM, Marc-André Lureau
> <marcandre.lureau at gmail.com> wrote:
> > On Tue, Nov 26, 2013 at 12:24 PM, Christophe Fergeau
> > <cfergeau at redhat.com> wrote:
> >> On Mon, Nov 25, 2013 at 08:41:27AM -0500, Marc-André Lureau wrote:
> >>>
> >>>
> >>> ----- Original Message -----
> >>> > On Mon, Nov 25, 2013 at 02:31:10PM +0100, Marc-André Lureau wrote:
> >>> > > On Mon, Nov 25, 2013 at 1:41 PM, Christophe Fergeau <cfergeau at redhat.com>
> >>> > > wrote:
> >>> > > > On Mon, Nov 25, 2013 at 01:33:52PM +0100, Marc-André Lureau wrote:
> >>> > > >> Check that there is an actual stream before calling cork().
> >>> > > >
> >>> > > > Is it expected that we are getting called with a non-NULL stream in the
> >>> > > > first place?
> >>> > >
> >>> > > Since this is sent by server on mjpeg stream creation, I would say
> >>> > > playback stream isn't mandatory.
> >>> > >
> >>> > > Anyway, it should not crash.
> >>> >
> >>> > Of course not, but we should fix things in the right place, hence the
> >>> > question to know if this is the right place for that fix.
> >>> > Your answer seems to be about playback_min_latency_changed(), what about
> >>> > stream_uncork? Is this happening in the same situation?
> >>> >
> >>>
> >>> No, there are checks for stream presence (it was checked everywhere, only
> >>> since efa65b49f it didn't check for min-latency property changes)
> >>
> >> This belongs in the commit log imo, and I did not understand what the
> >> g_return_if_fail is doing in that commit from your answer.
> >
> > Avoiding a call that would lead to an abort(), catching the error
> > earlier with a warning.
> >
> > --
> > Marc-André Lureau
> 
> 
> 
> -- 
> Marc-André Lureau
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20131126/fde0d301/attachment.pgp>