GNOME Bugzilla – Bug 753021
dashdemux: add a check for seek type in parse_seek function
Last modified: 2015-12-16 15:19:33 UTC
Add a check for start-type and stop-type in parse_seek function.
Created attachment 308402 [details] [review] Add a check condition for start-type and stop-type in parse_seek function.
Dear edward and thiagoss could you review this patch?
Review of attachment 308402 [details] [review]: ::: ext/dash/gstdashdemux.c @@ +1132,3 @@ + /* check if start-type/stop-type is SET */ + if (start_type == GST_SEEK_TYPE_NONE || stop_type == GST_SEEK_TYPE_NONE) { Most seeks have stop_type==NONE. You're breaking many things here :) What are you trying to solve here? It should work fine for NONE seek types already, just GST_SEEK_TYPE_END might not work (has to be checked)
Are you planning on updating this patch?
(In reply to Sebastian Dröge (slomo) from comment #4) > Are you planning on updating this patch? I'll do modify and update that patch for your comment ASAP.
Created attachment 316372 [details] [review] Add a check condition for seek type in parse_seek function
Also, I have tested using gst-validate like below command. it's working for me. gst-validate-1.0 playbin uri="http://www-itec.uni-klu.ac.at/ftp/datasets/mmsys12/RedBullPlayStreets/redbull_4s/RedBullPlayStreets_4s_isoffmain_DIS_23009_1_v_2_1c2_2011_08_30.mpd" --set-scenario=simple_seeks
@slomo I have upload new patch for your comment. please check my patch:)
Review of attachment 316372 [details] [review]: ::: ext/dash/gstdashdemux.c @@ +1268,3 @@ + /* check if start-type/stop-type is SET */ + if (start_type != GST_SEEK_TYPE_SET || stop_type == GST_SEEK_TYPE_END) { That doesn't seem right either. We should support GST_SEEK_TYPE_NONE for both, and most of the handling of the other parts is already in gst_segment_do_seek(). Are there any specific seeks that are failing for you without this patch?
(In reply to Sebastian Dröge (slomo) from comment #9) > Review of attachment 316372 [details] [review] [review]: > > ::: ext/dash/gstdashdemux.c > @@ +1268,3 @@ > > + /* check if start-type/stop-type is SET */ > + if (start_type != GST_SEEK_TYPE_SET || stop_type == GST_SEEK_TYPE_END) { > > That doesn't seem right either. > > We should support GST_SEEK_TYPE_NONE for both, and most of the handling of > the other parts is already in gst_segment_do_seek(). Are there any specific > seeks that are failing for you without this patch? Actually, seeks are working well without this patch. I just try to patch about the "TODO" list in dashdemux. I think that If start_type isn't set to GST_SEEK_TYPE_SET, we should return the failed result. So, could you recommend how to check start_type and stop_type in this code? As you know, I'm newbie of gstreamer:)
Why would it be an error if start_type is not SET? NONE would work very well too :) You would use NONE for example if you just want to change the playback rate but want to continue playing from the current position.
(In reply to Sebastian Dröge (slomo) from comment #11) > Why would it be an error if start_type is not SET? NONE would work very well > too :) You would use NONE for example if you just want to change the > playback rate but want to continue playing from the current position. Oh I understood! Thanks for your great opinion!:)