Really bad code to critique

Dave Mateer dave_mateer at ntm.org
Tue May 1 13:55:59 PDT 2012


I'm brand new to gstreamer, but am pretty sure it is going to be the right library for an application we are working on. I am very impressed with the feature set that the library implements. I have a proof-of-concept application that reads a video file and then plays the audio on the computer and displays the video in a Qt widget. I was wondering if some of the more experienced devs would mind offering a critique--I'm sure I'm doing a bunch of things The Wrong Way, but there are a lot of new things for me here: the gstreamer API itself, C (I'm more of a C++ guy), the GObject system, etc.

Among other things, the fact that I'm testing for audio vs. video pad by comparing a string prefix seems fragile and smelly. That can't be right. Also, it seems like I'm leaking memory in the call to gst_bin_get_by_name because the docs say that transfers ownership (not just a reference count?). Not sure about that one. I'm sure there are many other things as well.

I appreciate any help anyone could offer. It's my first attempt, so I know it's bad, so be honest! I stripped out some of the more Qt-specific stuff.

static void on_pad_added(GstElement *element,
                         GstPad *new_pad,
                         gpointer user_data) {
  GstCaps *caps = gst_pad_get_caps_reffed(new_pad);
  gchar *name = gst_caps_to_string(caps);
  GstBin *pipeline = (GstBin*)user_data;

  if (g_str_has_prefix(name, "audio")) {
    g_print("audio\n");

    // Leaking memory?? gst_bin_get_by_name transfers ownership?
    GstElement *audio_sink = gst_bin_get_by_name(pipeline, "audio-sink");
    if (!audio_sink) {
      throw std::runtime_error("Could not extract audio sink from pipeline");
    }

    GstPad *sink_pad = gst_element_get_static_pad(audio_sink, "sink");
    if (!sink_pad) {
      throw std::runtime_error("Could not get sink pad from audio element.");
    }

    if (GST_PAD_LINK_FAILED(gst_pad_link(new_pad, sink_pad))) {
      throw std::runtime_error("Failed to link audio src and sink");
    }

    gst_object_unref(sink_pad);
  } else {
    g_print("video\n");

    // Leaking memory?? gst_bin_get_by_name transfers ownership?
    GstElement *video_sink = gst_bin_get_by_name(pipeline, "video-sink");
    if (!video_sink) {
      throw std::runtime_error("Could not extract video sink from pipeline");
    }

    GstPad *sink_pad = gst_element_get_static_pad(video_sink, "sink");
    if (!sink_pad) {
      throw std::runtime_error("Could not get sink pad from video element.");
    }

    if (GST_PAD_LINK_FAILED(gst_pad_link(new_pad, sink_pad))) {
      throw std::runtime_error("Failed to link video src and sink");
    }

    gst_object_unref(sink_pad);
  }

  g_free(name);
  gst_caps_unref(caps);
}

MainWindow::MainWindow(QWidget *parent)
  : QMainWindow(parent),
    ui(new Ui::MainWindow) {
  ui->setupUi(this);

  // Create the top-level pipeline.
  pipeline_ = gst_pipeline_new(NULL);
  if (!pipeline_) {
    throw std::runtime_error("Could not create pipeline");
  }

  // Create the decode bin for our video source.
  GstElement *src = gst_element_factory_make("uridecodebin", NULL);
  if (!src) {
    throw std::runtime_error("Could not create uridecodebin");
  }
  g_object_set(src, "uri", "file:///path/to/media.ogg", NULL);

  // Add the decode bin to the pipeline.
  if (!gst_bin_add(GST_BIN(pipeline_), src)) {
    throw std::runtime_error("Could not add uridecodebin to pipeline");
  }

  // Create the video sink. This will be linked in the pad-added signal handler.
  GstElement *video_sink = gst_element_factory_make("xvimagesink",
                                                    "video-sink");
  if (!video_sink) {
    throw std::runtime_error("Could not create xvimagesink");
  }
  if (!gst_bin_add(GST_BIN(pipeline_), video_sink)) {
    throw std::runtime_error("Could not add video sink to pipeline");
  }
  WId id = ui->video_widget->winId();
  gst_x_overlay_set_window_handle(GST_X_OVERLAY(video_sink), id);
  qApp->syncX();

  // Create the audio sink. This will be linked in the pad-added signal handler.
  GstElement *audio_sink = gst_element_factory_make("autoaudiosink",
                                                    "audio-sink");
  if (!audio_sink) {
    throw std::runtime_error("Could not create autoaudiosink");
  }
  if (!gst_bin_add(GST_BIN(pipeline_), audio_sink)) {
    throw std::runtime_error("Could not add audio sink to pipeline");
  }

  // Register our interest in the pad-added signal so we can connect our sinks.
  g_signal_connect(src, "pad-added", G_CALLBACK(on_pad_added), pipeline_);

  // Start the playback.
  gst_element_set_state(pipeline_, GST_STATE_PLAYING);
}

MainWindow::~MainWindow() {
  gst_element_set_state (pipeline_, GST_STATE_NULL);
  gst_object_unref(pipeline_);
  delete ui;
}




More information about the gstreamer-devel mailing list