GNOME Bugzilla – Bug 690555
Dynamic Adaptive Streaming over HTTP (DASH) plugin
Last modified: 2013-09-04 08:38:09 UTC
Created attachment 231977 [details] [review] dashdemux sources initial import dashdemux: A new GStreamer plugin allowing the playback of MPEG DASH streams. The plugin is based on some basic objects defined in the GStreamer HLS Demux plugin from the gst-plugins-bad module. To launch gst-dashdemux: gst-launch playbin uri=http://www-itec.uni-klu.ac.at/ftp/datasets/mmsys13/redbull_4sec.mpd gst-launch playbin uri=http://download.tsi.telecom-paristech.fr/gpac/DASH_CONFORMANCE/TelecomParisTech/mp4-main-multi/mp4-main-multi-mpd-AV-BS.mpd
Nice!
Thanks for the integration patch :) I didn't look closer at the patch yet but please add this to gst/fragmented without duplicating the helper objects
OK, but: - since gst/fragmented doesn't exist, I would need to do some refactoring, ie rename hls into fragmented, right ? - I have put dashdemux under ext, as it has a dependency on libxml2: is it correct, and how can I have both hls and dash in the same plugin, then ?
(In reply to comment #3) > OK, but: > - since gst/fragmented doesn't exist, I would need to do some refactoring, ie > rename hls into fragmented, right ? Oh right, the directory is called hls but the plugin is already called fragmented. Would just need to be moved (if you do that, please as a separate commit). > - I have put dashdemux under ext, as it has a dependency on libxml2: is it > correct, and how can I have both hls and dash in the same plugin, then ? I guess the best would be to move it to ext/fragmented then. Other similar demuxers that could be added here later (MS smooth streaming) probably need other external dependencies too.
Does the spec say what character encoding the XML manifest should be? e.g. is it always in ASCII/UTF-8 encoding by any chance?
Created attachment 231993 [details] [review] hls: Moved HLS plugin from gst to ext to prepare merge with DASH
Created attachment 231994 [details] [review] hls: Renamed hls to fragmented to match the actual plugin name
Created attachment 231996 [details] [review] uridownloader: when possible, reuse existing element to download a new URI
Created attachment 231997 [details] [review] dashdemux: a 'fake' demux allowing the playback of MPEG DASH streams.
Created attachment 231998 [details] [review] dashdemux: Added a custom typefind for DASH mpd Here is a new set of patch merging dashdemux into fragmented
(In reply to comment #5) > Does the spec say what character encoding the XML manifest should be? e.g. is > it always in ASCII/UTF-8 encoding by any chance? I don't think so, but I have asked for a confirmation.
Hi, I'm going to take a deeper look at this. I already did quick reading and got some ideas/fixes here and there. I'll try to come up with a patch on top of yours and we can proceed to integrate it into git. Nice work!
BUMP
Sorry for the delay, this took a little longer than expected. I'm merging all the code into a single branch now (100+ patches) and incorporating a few review/requests from colleagues. After the changes are up I will put this up for more review and port the final version to 1.0.
Sorry for the long delay, here are the patches: http://cgit.collabora.com/git/user/thiagoss/gst-plugins-bad.git/log/ Also needs patches for: http://cgit.collabora.com/git/user/thiagoss/gst-plugins-good.git/log/ http://cgit.collabora.com/git/user/thiagoss/gst-plugins-base.git/log/ http://cgit.collabora.com/git/user/thiagoss/gstreamer.git/log
Well, I see now why it takes so long ... I don't have much to say on your changes: you certainly have a better understanding of the GStreamer inner mechanisms than I do. Some of the comments in the code are however now obsoleted by your patches: do you plan to update these ?
(In reply to comment #16) > Well, I see now why it takes so long ... > > I don't have much to say on your changes: you certainly have a better > understanding of the GStreamer inner mechanisms than I do. > > Some of the comments in the code are however now obsoleted by your patches: do > you plan to update these ? Yes, there is still some work to do for the plugin. But I think we should try to upstream it ASAP so more people can work on it concurrently instead of having it living on private repositories for more time.
Sure: what are the next steps ?
Thiago: I think the core changes can go in already, they look fine to me. > Sure: what are the next steps ? Wait until it gets reviewed :)
Quick not-in-depth review: Why do you have a separate copy of the LGPL in ext/dash/COPYING, the README and AUTHORS file should also probably go. The dash typefinder should move to the typefind plugin GST_PLUGIN_DEFINE should use more macros Remove #define GLIB_DISABLE_DEPRECATION_WARNINGS and fix the deprecated API usage Use gst_event_new_custom (GST_EVENT_CUSTOM_DOWNSTREAM) for the custom event, I don't think you're supposed to randomly invent new event numbers outside the core.
http://cgit.collabora.com/git/user/thiagoss/gst-plugins-base.git/commit/?id=2590fe386eec646958655729d76d8f6412dc52f1 Should check before the memcmp() if enough data is available, same for the data returned by gst_type_find_peek(). http://cgit.collabora.com/git/user/thiagoss/gst-plugins-base.git/commit/?id=c9d8b87b8a6ab9be19f98a44635336992a9fa7d3 For this please use the new BANDWIDTH_LIMITED flag of the scheduling query. Otherwise the base changes look good to push too.
(In reply to comment #21) > http://cgit.collabora.com/git/user/thiagoss/gst-plugins-base.git/commit/?id=2590fe386eec646958655729d76d8f6412dc52f1 > > Should check before the memcmp() if enough data is available, same for the data > returned by gst_type_find_peek(). Fixed for the memcmp, the _peek shouldn't need as it should always respect the requested size. > > http://cgit.collabora.com/git/user/thiagoss/gst-plugins-base.git/commit/?id=c9d8b87b8a6ab9be19f98a44635336992a9fa7d3 > > For this please use the new BANDWIDTH_LIMITED flag of the scheduling query. Removed this commit from the stack as uridecodebin already does this query. > > > > Otherwise the base changes look good to push too. Pushed. Thanks for the review.
I think it would make sense to push the DASH/MSS plugins, and let us fix up any things later. This way other people can easily start working on it, test it, fix things, report bugs, etc. I reviewed the HLS changes a longer time ago and they all looked fine to me and ready to push. Any opinions?
Yes, I would also like to see this get in (into master) as soon as possible, so people can give it a spin. There's only so much one can do by reviewing.
Myself and another engineer are actively working on bug fixes and enhancements for the dashdemux element. Right now, we are just working off a fork of Thiago's dev repo. We would love to have all of this code upstreamed so we can start submitting our patches!
Before pushing this to master, please edit the commit in history where this is enabled in configure.ac and added as subdirectory in ext/Makefile.am, and move those bits into a later commit (e.g. the 'port to 1.0' one, or add a new one). Otherwise we end up in a situation where the tree isn't compilable when doing a git-rebase, because it's got 0.10 code in it.
Pushed to -bad master. Thanks for the code and reviews. (In reply to comment #25) > Myself and another engineer are actively working on bug fixes and enhancements > for the dashdemux element. Right now, we are just working off a fork of > Thiago's dev repo. We would love to have all of this code upstreamed so we can > start submitting our patches! Go go go!
Hi, I have one doubt here, what is the region by adding the MPEG DASH part of HLS? 1) is MPEG DASH required HLS support in the down the layer? 2) MPEG DASH will work without HLS? 3) To play any MPEG DASH .mpd content is HLS functionality required?
DASH and HLS don't have anything to do with each other
Is current plugin will support for the both audio and video streams? because I am trying to play stream which contains both but it is not playing. but able to ply only video. 1) for only video i have used: http://143.205.176.132/ftp/datasets/mmsys12/BigBuckBunny/ 2) for both the streams i have used: http://143.205.176.132/ftp/datasets/mmsys13/
Please open separate bugs for any streams that do not work