Spencer has been working on an interesting branch which appears to use the web-pushing framework to support HTML5 embedded audio/video. I'm assuming his ogg and mp3 filters are working; I haven't looked at them yet. Anyway, I have spent a fair amount of time with the web-pushing framework, so I should be able to give some useful feedback.
Preliminary: The web-pushing branch has been merged, but is off by default, mostly because it tends to stall. This is most easily reproduced by opening the Activelink Index with web pushing enabled in a slow browser (e.g. a browser accessed via ssh -X user at localhost); eventually it stalls. This appears to be related to the fact that messages can at least theoretically get lost; imho for the terminal message (the one that finally updates an image after we've downloaded it, or failed) we need an acknowledgement and resend system. This is of course way out of scope for sajack's GSoC, but it's something somebody will need to address before we start using web-pushing by default. Which would be a very interesting addition to 0.8, 0.8.5 or 0.9 ... Code review: (up to 2553fa818f33067d76c253bd1f53072b8545ade2) Changes: - MERGES OGG FILTER BRANCH. - Automates rebuilding the javascript with GWT, downloading the jars from a maven repository. - HTML filter: Allow boolean attributes. Add HTML5 video/audio/source tags. - Max output length = 0 => ignore size. - Eliminate all size limits in splitfile fetcher. Doesn't make much sense to me! - Max size 0 means no max size. - Add ?max-size=0 to audio/video URL's via a tag replacer callback in fproxy. - Refactor out MediaElement base class for ImageElement. Factor out various methods, subsequentStateDisplay(), addCompleteElement(). - If pushing is enabled, create a MultimediaElement for each <video> or <audio> tag, including its <source>'s. - Remove unused argument initial from BaseUpdateableElement.updateState(). - Don't call updateState() in BaseUpdateableElement constructor. Create the initial state in each element's constructor. This avoids having to store the original tag in some cases. - New push type: MultimediaElementUpdater. This is an image which is refreshed (showing progress %) until the final element (video etc) has been loaded. Seems to just be based on fetching the src in the <video> or <audio> at the moment i.e. ignores <source>'s. - Elements initially say "Start Playback" (this is localised). Clicking on them will load the video/audio tag. Only do this if a special class name "unFinalized" is set. Which it is, by MultimediaElement. We don't start fetching until clicked. - New /queuefetch/ toadlet so client side can choose which source to fetch. - Client side goes over all the possible sources (which got through filtering) and starts a fetch for one which is compatible with the browser. - Allow populating HTMLNode attributes directly from a Map. Fix a related unit test. - Set the loading image size correctly. - Move makeHtmlNodeForParsedTag to ParsedTag.toHTMLNode() - Pass in an HTMLNode to MultimediaElement, including the child source's. - Convert all media source url's to absolute in tag replacer. - Replacement images hve a default size of 300x150 pixels, from the HTML5 spec. - Add processText() to FilterCallback, use it to capture the inner content of <video>'s. - Add inFlowContent(). Doesn't seem to be used at present except internally. - Browser compatibility detection is based on file extensions. See below. - Generate javadocs for GWT. - Logging. - Documentation. - Show a failure if we can't find a file to play or otherwise fail. - Allow fproxy progress page to be disabled via ?noprogress - Use ?noprogress to avoid race conditions etc when fetching the final file, or when not pushing. Some browsers don't send an accept header for video apparently... this doesn't happen with images because they do send such a header. eef058228dea8e94e2103cebaca082d79104faab - you need a special purpose TagVerifier for audio/video and another for source. - preload is not boolean, it's enumerated. You need to handle it specifically. See http://dev.w3.org/html5/spec/Overview.html#attr-media-preload - On the other hand, you're not actually using preload, and it doesn't make much sense given our usage... - source element: MIME type must be passed in to the content filter by modifying the URL with ?type=<specified type>, similarly to how we handle CSS. And you should delete the element if a MIME type is specified which we do not support. b20f8babb8758959665ff89e7994057687dbf7b6 - Max output length 0 = ignore both size and temp size. Might be better to use Long.MAX_VALUE, or transform it early on from one to the other. - Storing eventualLength in the constructor is not back compatible. It will break old persistent downloads. - Using eventualLength for maximum length doesn't make sense, is the intention to eliminate all size limits regardless of the max size??? That seems to be what happens for splitfiles here! 42c007e20c4d525f3925c062675bd60490d24353 - This is ok, but 1) there should be a size limit (maybe bigger) if the user is expected to wait for the download (i.e. it's okay to have no size limit if we are pushing, but we should have a size limit if there is no feedback, especially if it's automatic i.e. not after clicking), and 2) you should parse the url properly, there is code to do this in some other places. Specifically there might already be a ?type= or even ?max-size= 5f85577d5732fd55009fed5cf2b416f48d07a4e9 - Again you are adding ?max-size=0. There is code that does something similar but does it properly, find it and use it. d72bd247e3c600d2bd4c2e646527d6c90d9e2509 - I don't understand why this works, doesn't it fetch the current status of the pushed element? 28edafa2d5d6e19cf2938b2925feefa9adc680f4 - When exactly do we choose which key to load? Why do we not add the key from the original src to keys? d787063c0299c334a549f2087e4ee33b122a3229 - Does this mean we could have corruption in *all* web-pushing stuff that uses base64? Do we even use the variant that uses + ? E.g. the freenet variant of base64 doesn't use +... 5bfedfd1aa285c3a4f0a2e0590631a2afd3f631d - Automating GWT build. - The reason we didn't do this in the first place is that GWT is a nightmare to build cleanly from source. - It must be possible to get a clean build by putting your own jars in; IIRC that is true now. - If we do auto-fetch the jars, ideally the jars would be hosted by us and built cleanly by us. But using the official jars is okay provided they are versioned (thanks for finding versioned jars!) and checksummed (these are not, see the plugins' build.xml for an example of checksums). - On the whole it's probably a good thing, but we do need the checksums, please. f174c7c900a024dd7709acb020765455c348f43e - Browser compatibility is based on file extensions. This is not actually dangerous provided the browser respects the MIME type given when the download completes, however we should probably check it? If there is a mime type provided in the <source>, we should use it rather than guessing. - In any case, we need to use the specified MIME type if there is one, and if there is one, we need to force it on the fetch with ?type=. If the browser gets two different MIME types, one from the tag and one from the actual data, we could very well end up breaching the filter. General questions: What happens if web-pushing is disabled? We get the <video> tag without any support mechanisms? What about src= on video/audio tags? AFAICS we only consider src's from <source>? But we still send the browser the src from the base element itself? -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 197 bytes Desc: This is a digitally signed message part. URL: <https://emu.freenetproject.org/pipermail/devl/attachments/20100806/c3a40126/attachment.pgp>
