View previous topic :: View next topic |
Author |
Message |
brunonieuwenhuys
Joined: 20 Apr 2010 Posts: 5
|
Posted: Wed May 12, 2010 1:20 pm Post subject: meta data badly inserted (on network errors) 2.3.2-kh22 |
|
|
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
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 |
|
![](templates/subRebel/images/spacer.gif) |
karlH Code Warrior
![Code Warrior Code Warrior](templates/subRebel/images/ranks/rank_dev.gif)
Joined: 13 Jun 2005 Posts: 5476 Location: UK
|
Posted: Fri May 28, 2010 11:28 pm Post subject: |
|
|
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 |
|
![](templates/subRebel/images/spacer.gif) |
|
|
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
|