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 337415 - esd hangs when pausing a gstreamer-0.10 based sound
esd hangs when pausing a gstreamer-0.10 based sound
Status: RESOLVED FIXED
Product: esound
Classification: Deprecated
Component: general
unspecified
Other Linux
: Normal critical
: ---
Assigned To: Esound Maintainers
Esound Maintainers
: 335116 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2006-04-05 17:52 UTC by Fernando Herrera
Modified: 2007-04-23 08:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch fixing esd when client send a packet lower than buffer size (793 bytes, patch)
2006-04-05 18:22 UTC, Fernando Herrera
none Details | Review

Description Fernando Herrera 2006-04-05 17:52:57 UTC
1.- run esd
2.- open totem or muine 
3.- play a song
4.- stop it
5.- play it again (sam)
then daemon hang on:
  • #1 __read_nocancel
    from /lib/libc.so.6
  • #2 read_player
    at players.c line 297
  • #3 mix_players
    at mix.c line 907

and this make all player hang.
Comment 1 Fernando Herrera 2006-04-05 18:22:09 UTC
Created attachment 62815 [details] [review]
Patch fixing esd when client send a packet lower than buffer size
Comment 2 Fernando Herrera 2006-04-05 18:28:59 UTC
Comment on attachment 62815 [details] [review]
Patch fixing esd when client send a packet lower than buffer size


>+		if (bytes_read < player->buffer_length - player->actual_length)
>+			break;

Well, maybe this check should be after the checking for the end of stream or just returning, I haven't read corde beyond than the obvious read hand.
Comment 3 Baybal Ni 2006-04-26 12:42:43 UTC
*** Bug 335116 has been marked as a duplicate of this bug. ***
Comment 4 David Schleef 2006-08-08 02:03:34 UTC
Looks good.  Applied.
Comment 5 Frederic Crozat 2006-08-22 14:12:29 UTC
reopening, this patch is causing 100% cpu usage, as explained on http://qa.mandriva.com/show_bug.cgi?id=24365

To duplicate :
-start mozilla-thunderbird
-go to Edit/Preferences/General/When a new message arrive / Advanced 
- choose a wav file
-press preview several times => esd will take 100% cpu
Comment 6 Fernando Herrera 2006-08-22 14:50:38 UTC
YEs, that is as comment #2 says, because the added check should be after checking the EOF:

  diff -u -u -r1.48 players.c
--- players.c   8 Aug 2006 02:03:18 -0000       1.48
+++ players.c   22 Aug 2006 14:47:22 -0000
@@ -310,9 +310,6 @@
                              player->data_buffer + player->actual_length, 
                              player->buffer_length - player->actual_length,
                              actual, "str rd" );
-               if (bytes_read < player->buffer_length - player->actual_length)
-                       break;
-
                /* check for end of stream */
                if ( actual == 0 
                     || ( actual < 0 && errno != EAGAIN && errno != EINTR ) )
@@ -320,6 +317,9 @@
                /* more data, save how much we got */
                if ( actual > 0 )
                   player->actual_length += actual;
+
+               if (bytes_read < player->buffer_length - player->actual_length)
+                       break;
            } while (player->actual_length < player->buffer_length);
 
            /* endian swap multi-byte data if we need to */
Comment 7 Fernando Herrera 2006-08-22 16:06:38 UTC
Well, not I'm 100% lost. Looking at the reading loop code:
 do {
		bytes_read = ESD_READ_BIN( player->source_id,
			      player->data_buffer + player->actual_length, 
			      player->buffer_length - player->actual_length,
			      actual, "str rd" );
		if (bytes_read < player->buffer_length - player->actual_length)
			break;
		
		/* check for end of stream */
		if ( actual == 0 
		     || ( actual < 0 && errno != EAGAIN && errno != EINTR ) )
		    return -1;
		/* more data, save how much we got */
		if ( actual > 0 )
		   player->actual_length += actual;
    } while (player->actual_length < player->buffer_length);

actual is used everywhere, but ESD_READ_BIN is defined as:
#define ESD_READ_BIN(s,a,l,r,d)  r = read( s, a, l )

so this actual value is never filled!. I wonder how can this works.
Comment 8 Fernando Herrera 2006-08-22 16:17:54 UTC
ok, r = read
please forget my last comment, should be my backpain.
Comment 9 Kjartan Maraas 2007-01-05 09:47:57 UTC
Any news on this Fernando?
Comment 10 Fernando Herrera 2007-04-23 08:50:06 UTC
Yeah, same patch as comment #6 was applied after bug #412951, closing as fixed.