[Spice-devel] [PATCH v6 23/26] spice-gtk: Add a GStreamer video decoder for MJPEG, VP8 and h264

[Francois Gouget]

On Fri, 16 Oct 2015, Victor Toso wrote:
[...]
> > @@ -321,9 +321,9 @@ AS_IF([test "x$with_audio" = "xauto"],
> >  AS_IF([test "x$with_audio" = "xpulse"],
> >        [AS_IF([test "x$have_pulse" = "xyes"],
> >               [AC_DEFINE([WITH_PULSE], 1, [Have pulseaudio?])],
> > -             [AC_MSG_ERROR([PulseAudio requested but not found])
> > -      ])
> > -])
> > +             [AC_MSG_ERROR([PulseAudio requested but not found])]
> > +      )]
> > +)
> 
> Order is inverted )] ?
> 
> checking for GSTVIDEO... yes
> ./configure: line 17978: ]: command not found

Ah. I missed that one in the middle of the configure output.

I had also meant to fix the patch to follow the existing practice 
regarding the parentheses and square brackets though I don't like it 
because it's really really misleading.

Take for instance the following gst-audio check:

1: AS_IF([test "x$with_audio" = "xgstreamer"],
2:       [AS_IF([test "x$have_gst_audio" = "xyes"],
3:              ...
4:              ...
5:       ])
6: ])

Clearly the square bracket on line 5 closes '[AS_IF' since it's at the 
same level. So then the parenthesis closes the 'AS_IF' from line 1. But 
then what's this on line 6???

In fact the square bracket on line 5 closes a one-liner expression on 
line 4. And the square bracket in column 1 on line 6 closes the bracket 
one indentation level down on line 2.

So I think it makes much more sense to write this as:

1: AS_IF([test "x$with_audio" = "xgstreamer"],
2:       [AS_IF([test "x$have_gst_audio" = "xyes"],
3:              ...
4:              ...
5:        )]
6: )

The ')]' on line 5 are correctly aligned with the corresponding 
'[AS_IF(' on line 2, and the lone ')' on line 6 is still aligned with 
the 'AS_IF(' on line 1.

And line 4 would be self contained:

             [AC_MSG_ERROR([GStreamer 1.0 audio requested but not found])]


Anyway, I changed the patch to follow existing convention, added a 
GStreamer Video line in the summary, and removed the extraneous square 
bracket which was one line down. See the updated attached patch.

-- 
Francois Gouget <fgouget at codeweavers.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: p23.diff
Type: text/x-diff
Size: 13315 bytes
Desc: p23.diff
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20151016/b5998554/attachment-0001.diff>