Icecast Streaming Media Server Forum Index Icecast Streaming Media Server
Icecast is a Xiph Foundation Project
 
 FAQFAQ   SearchSearch   MemberlistMemberlist   UsergroupsUsergroups   RegisterRegister 
 ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

meta data badly inserted (on network errors) 2.3.2-kh22

 
Post new topic   Reply to topic    Icecast Streaming Media Server Forum Index -> Bug Reports
View previous topic :: View next topic  
Author Message
brunonieuwenhuys



Joined: 20 Apr 2010
Posts: 5

PostPosted: Wed May 12, 2010 1:20 pm    Post subject: meta data badly inserted (on network errors) 2.3.2-kh22 Reply with quote

Hello

The new version of icecast does an optimization to merge metadata with preceding mp3 data and sends it as one chunk, to reduce the number of send() calls.
If this send succeeds only partially and before reaching metadata, icecast tries sending further mp3 data -- which is incorrect because they didn't send medata yet.
However, if the first send attempt failed because it was a network error, the second send attempt will (almost always) fail as well and therefore the bug does not manifest itself. but if it does work then the meta data is not inserted correctly. Not a very probable bug, but still a little one Wink

we have partially identified the problem
The line 452 of format_mp3.c of icecast-2.3.2-kh22 contains an obvious bug:

/* limit how much mp3 we send if using small intervals */
if (len > client_mp3->interval)
len = client_mp3->interval;

We think It should be:
if (len > client_mp3->interval - client_mp3->since_meta_block - 1)
len = client_mp3->interval - client_mp3->since_meta_block - 1;

Fixing this doesn't completely remove the problem it seems there are other issues related to partial sends which we didn't investigate further.

To reproduce the problem in an artificial way we hooked the send function with this function :

ssize_t send(int socket, const void* buffer, size_t length, int flags)

{

bool wouldBlock = false;
if( rand() % 500 == 0 )
{
printf("Randomly sending only a part of the buffer with an EWOULDBLOCK result\n"); fflush(stdout);
length = max(1ul, length/2);
wouldBlock = true;
}
int ret = SocketsAPI::getInstance()->send_(socket, buffer, length, flags);
if( wouldBlock && ret > 0 )
errno = EWOULDBLOCK;
return ret;
}

Hope this helps
_________________
www.adswizz.com
Back to top
View user's profile Send private message
karlH
Code Warrior
Code Warrior


Joined: 13 Jun 2005
Posts: 5476
Location: UK

PostPosted: Fri May 28, 2010 11:28 pm    Post subject: Reply with quote

I see the issue there, but I don't see that change working out. I think what would be better is

if (client->flags & CLIENT_IN_METADATA)
break;
buf += remaining;
len -= remaining;
if (ret <= remaining || len > client_mp3->interval)
break;

this way, very short writes (falling short of the metadata) break out, but we also break out if the rest of the mp3 data in the block is larger than the next metadata interval, if we break out there then another merge could be done

karl.
Back to top
View user's profile Send private message Send e-mail Visit poster's website
Display posts from previous:   
Post new topic   Reply to topic    Icecast Streaming Media Server Forum Index -> Bug Reports All times are GMT
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2002 phpBB Group
subRebel style by ktauber