[Gnash-dev] Choppy sound on video playback [PATCH]

[Deomid Ryabkov]

hi, gnash developers.

i've been using gnash for a while.
it has been and still a bit painful, but so far i'm getting along and welcome 
the many recent improvements. :)

one particular pain point for me has been choppy sound during youtube video 
playback.
my system is ubuntu 8.04, hardware is a thinkpad t43p (pentium m 2.1ghz).
anyway, sound was choppy when playing youtube videos, yet processor utilization 
was at all times around 35%.
it annoyed me enough that i checked out gnash source from cvs and tested - 
unfortunately, it was the same thing.

i noticed that audio would skip when there was lot of change in the video - 
possibly when video decoder had more stuff to do.
i dug into it and found that gnash had very little to do with processing apart 
from setting up a gstreamer pipeline.
trying various things, i finally found a solution: insert additional queues for 
audio and video streams after the decoder.
this makes sense - presumably, enough audio data can now be queued while more 
intensive video processing is being done.
it may also help that each queue implicitly adds a thread. in any case, it 
works for me - no more audio skips!
patch for server/asobj/NetStreamGst.cpp is attached, and, to illustrate the 
change, here's gstreamer pipeline before and after:

//                                        video -> ffmpegcolorspace -> videoscale 
-> capsfilter -> fakesink
//                                       /
// (GstUriHandler) -> queue -> decodebin
//                                       |
//                                        audio -> audioconvert -> autoaudiosink

//                                         queue (video) -> ffmpegcolorspace -> 
videoscale -> capsfilter -> fakesink
//                                       /
// (GstUriHandler) -> queue -> decodebin
//                                       \
//                                         queue (audio) -> audioconvert -> 
autoaudiosink


--
Deomid Ryabkov aka Rojer
address@hidden
address@hidden
ICQ: 8025844

Index: ./server/asobj/NetStreamGst.cpp
===================================================================
RCS file: /sources/gnash/gnash/server/asobj/NetStreamGst.cpp,v
retrieving revision 1.90
diff -u -b -r1.90 NetStreamGst.cpp
--- ./server/asobj/NetStreamGst.cpp     6 Jun 2008 21:06:55 -0000       1.90
+++ ./server/asobj/NetStreamGst.cpp     21 Jun 2008 08:01:43 -0000
@@ -38,11 +38,11 @@
 #endif // GST_HAS_MODERN_PBUTILS
 
 
-//                                        video -> ffmpegcolorspace -> 
capsfilter -> fakesink
+//                                         queue (video) -> ffmpegcolorspace 
-> videoscale -> capsfilter -> fakesink
 //                                       /
 // (GstUriHandler) -> queue -> decodebin
-//                                       |
-//                                        audio -> audioconvert -> 
autoaudiosink
+//                                       \
+//                                         queue (audio) -> audioconvert -> 
autoaudiosink
 
 namespace gnash {
 
@@ -85,6 +85,7 @@
 
   // Setup video conversion and sink
 
+  GstElement* video_queue = gst_element_factory_make ("queue", 
"gnash_videoqueue");
 
   // setup the video colorspaceconverter converter
   GstElement* colorspace = gst_element_factory_make ("ffmpegcolorspace", 
"gnash_colorspace");
@@ -113,21 +114,29 @@
 
   // Create the video pipeline and link the elements. The pipeline will
   // dereference the elements when they are destroyed.
-  gst_bin_add_many (GST_BIN (_videobin), colorspace, videoscale, videocaps, 
videosink, NULL);
+  gst_bin_add_many (GST_BIN (_videobin), video_queue, colorspace, videoscale,
+                                         videocaps, videosink, NULL);
   
-  if (!colorspace || !videoscale || !videocaps || !videosink) {
+  if (!video_queue || !colorspace || !videoscale || !videocaps || !videosink) {
     log_error(_("Couldn't create the Gstreamer video conversion elements. "
                 "Please make sure Gstreamer and gstreamer-plugins-base are "
                 "correctly installed. Video playback will not be possible."));
   }
   
-  rv = gst_element_link_many(colorspace, videoscale, videocaps, videosink, 
NULL);
+  rv = gst_element_link_many(video_queue, colorspace, videoscale, videocaps, 
videosink, NULL);
   if (!rv) {
     log_error(_("Failed to link video conversion elements. Video playback will"
                 " not be possible"));
   }
 
+  GstPad* target_videopad = gst_element_get_static_pad (video_queue, "sink");
+  gst_element_add_pad(_videobin, gst_ghost_pad_new ("sink", target_videopad));
+  gst_object_unref(GST_OBJECT(target_videopad));
+
+
   // Setup audio sink
+  GstElement* audio_queue = gst_element_factory_make ("queue", 
"gnash_audioqueue");
+    
   GstElement* audioconvert = gst_element_factory_make ("audioconvert", NULL);  
   
   GstElement* audiosink;
@@ -143,27 +152,20 @@
     audiosink = gst_element_factory_make ("fakesink", NULL);
   }
 
-  gst_bin_add_many(GST_BIN(_audiobin), audioconvert, audiosink, NULL);
+  gst_bin_add_many(GST_BIN(_audiobin), audio_queue, audioconvert, audiosink, 
NULL);
 
-  if (!audioconvert || !audiosink) {
+  if (!audio_queue || !audioconvert || !audiosink) {
     log_error("Couldn't create Gstreamer audio elements. Audio playback will "
               "not be possible");
   }
-  rv = gst_element_link(audioconvert, audiosink);
+  rv = gst_element_link_many(audio_queue, audioconvert, audiosink, NULL);
   if (!rv) {
     log_error("Couldn't link audio elements. There will be no audio 
playback.");
   }
 
-  GstPad* target_audiopad = gst_element_get_static_pad (audioconvert, "sink");
-  GstPad* target_videopad = gst_element_get_static_pad (colorspace, "sink");
-  
-  gst_element_add_pad(_videobin, gst_ghost_pad_new ("sink", target_videopad));
+  GstPad* target_audiopad = gst_element_get_static_pad (audio_queue, "sink");
   gst_element_add_pad(_audiobin, gst_ghost_pad_new ("sink", target_audiopad));
-  
-  gst_object_unref(GST_OBJECT(target_videopad));
   gst_object_unref(GST_OBJECT(target_audiopad));
-
-
 }
 
 NetStreamGst::~NetStreamGst()