GNOME Bugzilla – Bug 763205
Fix the bug when live streaming a MPD consists of multiple periods
Last modified: 2018-11-03 13:47:22 UTC
Created attachment 323233 [details] [review] patch to fix claimed issue Dear all: I found that currently if a "live" DASH streaming with a MPD consists of "multiple periods", it cannot advance to next period once the we have completed the first period. For example, in attached MPD there are 2 periods. Once we have done the first period (have played out 5 minutes content), we can never arrive the 2nd period. It is because we never call gst_adaptive_demux_advance_period()。 [Solution]: Once all streams have reached the end of current period & there is a next period, we should advance to next period instead of merely waiting for manifest's update. Patch is as attached, please kindly refer to it. Thank you~
Created attachment 323234 [details] [review] Fix the bug when live streaming a MPD consists of multiple periods
Created attachment 323235 [details] example : a MPD which will lead to claimed issue. example : a MPD which will lead to claimed issue.
Review of attachment 323234 [details] [review]: Thanks for the patch, I guess this is the first time we see a live DASH stream that has multiple periods. Care to say why is this used in the real world? Some live streaming changing from one event transmission to another? Anyway, some small changed needed as mentioned below. Check if you agree, please. ::: gst-libs/gst/adaptivedemux/gstadaptivedemux.c @@ +2869,3 @@ /* we push the EOS after releasing the object lock */ if (gst_adaptive_demux_is_live (demux)) { + if (gst_adaptive_demux_combine_flows (demux) == GST_FLOW_EOS && gst_adaptive_demux_has_next_period (demux)) { I think if it has_next_period() then it makes no sense to wait for a manifest update. So the && should be simplified here. If there is already a next period the current one should already have all its segment listed. In this case, the task should be stopped anyway. If the flow combiner returns EOS then it can already advance to the next period, otherwise it should just exit and the other pad that is still downloading segments for this period will move to the next period. It ends up being pretty similar to what happens when non-live streams have to change periods.
Dear Thiago: Thanks a lot for your suggestion. I think one application may happen for live TV. In this case the content provider will insert advertisements into some predefined time slots among the main program. Since the advertisements obviously differ from main program (maybe the main program has several audio tracks (adaption sets) in different languages but the advertisement is quite simple to have a single one), it is convenient to make the advertisements be a series of periods which are independent from the main program. About your comment, from my previous trace, the download loop does download for each period. Once a specific stream has reached the end of a period (completed download for this period), in gst_adaptive_demux_stream_download_loop the return value of gst_adaptive_demux_stream_update_fragment_info() will be GST_FLOW_EOS. So we enter the switch logic and case GST_FLOW_EOS. The check for the EOS of gst_adaptive_demux_combine_flows will insure that we will go to the next period only when all active streams have done the download for current period. At this moment, if it is a live streaming and we have a next period, we advance to enter it. Otherwise, we do wait for the manifest’s update (expect a new period will appear later) or return EOS if the manifest finally out of date. I think the flow within this patch seems to correspond to your suggestion. Am I right? Or if there is something I missed which could make the patch better, please kindly leave your variable suggestions here; thank you~
Suppose you have an mpd with 2 streams (audio and video) and it has a new period on the MPD. Audio has finished and enters the EOS switch case, when it does the flow combination it will fail the check because the video stream hasn't gone EOS yet for that period. In this case, with your patch, the audio stream thread will go into 'waiting for manifest update'. My question is whether this is useful in this scenario? We already know that this stream has no more fragments on the current period. It would make more sense IMHO to simply stop this thread that has finished downloading and, once the other stream finishes the current period, it will advance and restart all threads again.
Created attachment 325613 [details] [review] Something like this patch. Please check if this works for you and provide an updated patch. Make sure that you use an author and email that we can use to refer to you. Also take a look at other commit messages in our repositories to follow the same pattern before the fix for this bug can be merged. Thanks
Created attachment 328686 [details] [review] Fix the issue when live streaming a MPD consists of multiple periods As request to refine previous patch.
Dear Thiago: You's comment is right. According to your suggestion I duplicated a patch from yours (they are the same) since I am not sure if I need to create one again. Thanks a lot for your suggestion.
Review of attachment 328686 [details] [review]: Code looks fine, but it still lacks a proper commit message and title. Check other commits in our log and you will understand how to do it. Briefly it is: " elementname: short title long description of your change over multiple lines bug-link " Thanks!
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/356.