After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 320765 - [ffmpegcolorspace] make win32+msvc compliant, don't use __VA_ARGS__ macros
[ffmpegcolorspace] make win32+msvc compliant, don't use __VA_ARGS__ macros
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Windows
: Normal normal
: 0.10.6
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2005-11-05 15:57 UTC by Sebastien Moutte
Modified: 2006-03-15 11:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
a patch to replace macros with variables params. (8.16 KB, patch)
2005-11-05 15:59 UTC, Sebastien Moutte
rejected Details | Review
alternative to __VA_ARG__ (6.09 KB, patch)
2006-02-01 13:10 UTC, Sebastien Moutte
committed Details | Review

Description Sebastien Moutte 2005-11-05 15:57:26 UTC
Distribution/Version: VC6.0

ffmpegcolorspace can't build on visual studio c compiler.
There are macro with variable numbers of parameters in gstffmpegcodecmap.c

Attached a patch in the same code without macros.
Comment 1 Sebastien Moutte 2005-11-05 15:59:34 UTC
Created attachment 54355 [details] [review]
a patch to replace macros with variables params.
Comment 2 Ronald Bultje 2005-11-05 19:41:38 UTC
I don't particularly like this; can we do this in another way?
Comment 3 Jan Schmidt 2006-01-30 23:42:03 UTC
I agree with Ronald. 

Perhaps we could bust it out to an inline function that creates a new caps and then uses gst_caps_set_simple_valist?
Comment 4 Sebastien Moutte 2006-01-31 12:36:02 UTC
ok, so 
#define GST_FF_VID_CAPS_NEW(mimetype, ...)                      \
    (context != NULL) ?                                         \
    gst_caps_new_simple (mimetype,                              \
        "width",     G_TYPE_INT,   context->width,              \
        "height",    G_TYPE_INT,   context->height,             \
        "framerate", GST_TYPE_FRACTION,                         \
        (gint) context->frame_rate, (gint) context->frame_rate_base, \
        __VA_ARGS__, NULL)                                      \
    :                                                           \
    gst_caps_new_simple (mimetype,                              \
        "width",     GST_TYPE_INT_RANGE, 1, G_MAXINT,           \
        "height",    GST_TYPE_INT_RANGE, 1, G_MAXINT,           \
        "framerate", GST_TYPE_FRACTION_RANGE, 0, 1, G_MAXINT, 1,\
        __VA_ARGS__, NULL)

becomes something like 
void gst_ff_vid_caps_new(AVCodecContext * context, const char *mimetype, ...)
{
  GstCaps *caps = NULL;
  va_list var_args;

  if(context != NULL) {
    caps = gst_caps_new_simple (mimetype,
      "width",     G_TYPE_INT,   context->width,
      "height",    G_TYPE_INT,   context->height,
      "framerate", GST_TYPE_FRACTION,
      (gint) context->frame_rate, (gint) context->frame_rate_base,
      NULL);
  }
  else {
    caps = gst_caps_new_simple (mimetype,
      "width",     GST_TYPE_INT_RANGE, 1, G_MAXINT,
      "height",    GST_TYPE_INT_RANGE, 1, G_MAXINT,
      "framerate", GST_TYPE_FRACTION_RANGE, 0, 1, G_MAXINT, 1,
      NULL);
  }

  va_start (var_args, mimetype);
  gst_caps_set_simple_valist (caps, ????, var_args);
  va_end (var_args);

  return caps;
}

What should be first field name ? can we extract it from var_args or use NULL then it should take the first one?

Comment 5 Sebastien Moutte 2006-02-01 13:06:28 UTC
Ok you can forget my previous solution which i haven't tested. it can't work.
I'm attaching a pacth with a solution i've just test and which works for me.
Please check, test and let me know ...
Thanks,
Seb
Comment 6 Sebastien Moutte 2006-02-01 13:10:32 UTC
Created attachment 58514 [details] [review]
alternative to __VA_ARG__
Comment 7 Wim Taymans 2006-02-16 17:39:03 UTC
looks fine to me, everybody happy with this?
Comment 8 Tim-Philipp Müller 2006-03-15 11:32:12 UTC
Committed (without the stray \ in gst_ff_aud_caps_new()):

 2006-03-15  Tim-Philipp Müller  <tim at centricular dot net>

        Patch by: Sebastien Moutte  <sebastien moutte net>

        * gst/ffmpegcolorspace/gstffmpegcodecmap.c: (gst_ff_vid_caps_new),
        (gst_ff_aud_caps_new), (gst_ffmpeg_pixfmt_to_caps),
        (gst_ffmpeg_smpfmt_to_caps):
          Replace __VA_ARGS__ caps creation macros with varargs functions.
          Makes things compile on MSVC (#320765), looks nicer, and we can
          tell the compiler to check for the NULL terminator.