Closed
Bug 216218
Opened 21 years ago
Closed 20 years ago
streaming sound will pause after a few seconds
Categories
(Core Graveyard :: Plug-ins, defect)
Core Graveyard
Plug-ins
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: czimmerman, Assigned: jst)
References
()
Details
(Keywords: shockwave)
Attachments
(6 files, 1 obsolete file)
38.89 KB,
application/zip
|
Details | |
56.65 KB,
patch
|
Details | Diff | Splinter Review | |
49.87 KB,
patch
|
Details | Diff | Splinter Review | |
51.41 KB,
patch
|
darin.moz
:
superreview+
|
Details | Diff | Splinter Review |
51.44 KB,
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
58.43 KB,
patch
|
jst
:
review+
jst
:
superreview+
asa
:
approval1.7+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1) Build Identifier: playing streaming sound in Shockwave movie will fail after ~53% streamed Reproducible: Always Steps to Reproduce: 1. play SW movie that contains streaming SWA 2. 3. Actual Results: it will stop playing after ~53 % streamed audio Expected Results: should play all 100%
Comment 1•21 years ago
|
||
Do you have an excample URL and which flash version Do you used ?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 2•21 years ago
|
||
Comment 3•21 years ago
|
||
confirming with win2k build 20030814 and Macromedia Shockwave for Director Netscape plug-in, version 8.5 The shockwave streaming stops at ~51% with a networking error. The testcase works with the same plugin in NS4.7x
Comment 4•21 years ago
|
||
From Macromedia's public beta site: http://www.macromedia.com/software/shockwaveplayer/beta/release_notes/ Macromedia will withhold Shockwave support for any Mozilla based browsers until the following bugs are fixed: 213331 QT spills and jumps all over Netscape 7.1 (OSX) 213329 Japanese error message doesn't display double-byte text correctly 216216 Editable text field does not allow text entry 216218 Netscape 7.x: Streaming sound issues
Comment 6•20 years ago
|
||
The testcase doesn't work at the moment because http://audio.macromedia.com seems to be dead
Comment 7•20 years ago
|
||
One example of this problem occurs at http://www.morganic.com/WardM/Music.html For the first song (length 2:28), it stops playing for me at 13 seconds. For the last song (length 3:44), it stops playing for me at 23 seconds. So, for each one, it gets about 10% through before stopping. I'm using the 3/30/04 OSX build of Mozilla, with Shockwave 10.0.0.210, under MacOSX 10.3.3. (It plays completely in Safari 1.2.1 with the same version of Shockwave.)
Assignee | ||
Comment 8•20 years ago
|
||
This patch is not exactly trivial, but it seems to work and I don't see how this can be fixed w/o something along these lines. The problem we're running into here is that we're getting data off the network faster than the plugin can consume it, and we simply bail and close the stream etc etc. The fix is to not bail, but to suspend the request, and spoonfeed the plugin with what data we already got off the network, until it consumes most of it, then resume the request to get more data off the network, and do that as long as there's more data...
Assignee | ||
Updated•20 years ago
|
Assignee: peterlubczynski-bugs → jst
Status: NEW → ASSIGNED
Assignee | ||
Updated•20 years ago
|
Attachment #145513 -
Attachment description: Proposed fix, Suspend necko & manually pump data, then resume... → Proposed fix. Suspend necko & manually pump data, then resume...
Assignee | ||
Comment 9•20 years ago
|
||
Assignee | ||
Comment 10•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Attachment #145573 -
Flags: superreview?(darin)
Assignee | ||
Comment 11•20 years ago
|
||
Attachment #145573 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Attachment #145573 -
Flags: superreview?(darin)
Assignee | ||
Updated•20 years ago
|
Attachment #145576 -
Flags: superreview?(darin)
Comment 12•20 years ago
|
||
Comment on attachment 145576 [details] [diff] [review] Updated diff -w, fixes some plugin stream offset problems in the earlier version, and also makes sure a suspended request is canceld when the plugin stream is destroyed. >Index: modules/plugin/base/src/ns4xPluginInstance.cpp > ns4xPluginStreamListener::OnDataAvailable(nsIPluginStreamInfo* pluginInfo, ... > PRUint32 contentLength; > pluginInfo->GetLength(&contentLength); >+ >+ mStreamBufferSize = PR_MAX(length, contentLength); So, I don't think there's any point to setting mStreamBufferSize above since its value is going to be changed immediately to something else below: >+ >+ // Limit the size of the initial buffer to MAX_PLUGIN_NECKO_BUFFER >+ // (16k). This buffer will grow if needed, as in the case where >+ // we're getting data faster than the plugin can process it. >+ mStreamBufferSize = PR_MIN(length, MAX_PLUGIN_NECKO_BUFFER); >+ if (amountRead == 0) { >+ NS_WARNING("input->Read() returns no data, it's almost impossible " >+ "to get here"); Make this a NS_NOTREACHED instead since yeah, you are right. This should never happen. >+class nsI4xPluginStreamInfo : public nsISupports perhaps it makes sense for this interface to inherit from nsIPluginStreamInfo? otherwise, this patch looks great to me. sr=darin
Attachment #145576 -
Flags: superreview?(darin) → superreview+
Assignee | ||
Comment 13•20 years ago
|
||
The idea with the code that does the following: mStreamBufferSize = PR_MAX(length, contentLength); ... mStreamBufferSize = PR_MIN(length, MAX_PLUGIN_NECKO_BUFFER); was to make us malloc a buffer large enough to fit the whole stream (i.e. contentLength) but only if that's smaller than MAX_PLUGIN_NECKO_BUFFER, which means that the code is incorrect, what I meant for it to do was this: mStreamBufferSize = PR_MAX(length, contentLength); ... mStreamBufferSize = PR_MIN(mStreamBufferSize, MAX_PLUGIN_NECKO_BUFFER); since I don't want us to get into a situation where the first ODA call hands us 8 bytes (or whatever small number), and subsequent calls hands us 2kb, or whatever, and in the end, the whole plugin stream would've fit in MAX_PLUGIN_NECKO_BUFFER. This is trying to avoid us reading only a tiny bit of data and then writing that to the plugin, and then reading a tiny bit again, and so on. I can go either way on the interface inheritance, I'll change that since you pointed it out...
Assignee | ||
Comment 14•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Attachment #145641 -
Flags: review?(brendan)
Comment 15•20 years ago
|
||
Comment on attachment 145641 [details] [diff] [review] Fix darins issues. >+ nsresult rv = NS_OK; >+ while (NS_SUCCEEDED(rv) && length > 0) { >+ if (input && length) { >+ if (mStreamBufferSize < mStreamBufferByteCount + length && >+ mIsSuspended) { >+ // We're in the ::OnDataAvailable() call that we might get >+ // after suspending a request, or we suspended the request >+ // from within this ::OnDataAvailable() call while there's >+ // still data in the input, and we don't have enough space to >+ // store what we got off the network. Reallocate our internal >+ // buffer. >+ mStreamBufferSize = mStreamBufferByteCount + length; >+ mStreamBuffer = (char*)PR_Realloc(mStreamBuffer, mStreamBufferSize); Use a temporary for the return value, so you don't leak the unrealloced (after realloc failure) existing allocation, setting mStreamBuffer non-null only on success. >+ if (!mStreamBuffer) >+ return NS_ERROR_OUT_OF_MEMORY; >+ } >- if (numtowrite > amountRead) >- numtowrite = amountRead; >+ >+ // Break out of the for loop, but keep going through the >+ // while loop in case there's more data to read from the >+ // input stream. No more for loop, it's an outer while now, eh? >+ // XXX: Why couldn't this be tracked on the plugin >+ // info, and not in a *hash table*? >+ nsPRUintKey key(absoluteOffset); >+ PRInt32 amtForwardToPlugin = >+ NS_PTR_TO_INT32(mDataForwardToRequest->Get(&key)); Can you file bug(s) as needed to followup on the XXX's you've added? Maybe these should be FIXME: <bug#> comments instead -- that's something the GNOME guys do, and it tends to lead to further fixing better than ye olde XXX comments. sr=me for 1.7. Thanks for wading into the swamp a bit! /be
Attachment #145641 -
Flags: review?(brendan) → review+
Assignee | ||
Comment 16•20 years ago
|
||
Assignee | ||
Comment 17•20 years ago
|
||
Comment on attachment 145785 [details] [diff] [review] Fix brendan's issues. Carrying r=darin and sr=brendan forward, and requesting approval for 1.7 finald for this macromedia blocker.
Attachment #145785 -
Flags: superreview+
Attachment #145785 -
Flags: review+
Attachment #145785 -
Flags: approval1.7?
Comment 18•20 years ago
|
||
Comment on attachment 145785 [details] [diff] [review] Fix brendan's issues. a=asa (on behalf of drivers) for checkin to 1.7
Attachment #145785 -
Flags: approval1.7? → approval1.7+
Assignee | ||
Comment 19•20 years ago
|
||
Fixed, but I incorrectly quoted bug 237076 in the checkin comment (looked at the wrong window while typing), duh. Marking bug 89270 fixed too, as it was an earlier report of the same problem (AFAICT), IOW, this should've been a dupe of that bug.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Flags: blocking1.7?
Comment 20•20 years ago
|
||
Could somebody take a look at bug 241431 and bug 241592? They might be related to this checkin.
Assignee | ||
Comment 21•20 years ago
|
||
Looking. Thanks.
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•