Created attachment 361232 [details] [review]
update fragment info with 'sidx'

* update fragment info after 'sidx' parsed.
* fill correct fragment range end
* if 'sidx' not specified in mpd, set seek flag true.

please kindly review the patch.

Review of attachment 361232 [details] [review]:

::: ext/dash/gstdashdemux.c
@@ +1354,3 @@
         dashstream->actual_position += entry->duration;
       } else {
+        stream->fragment.range_end = stream->fragment.range_start + entry->size - 1;

This is wrong. The range request should only be until the end of the current SIDX entry, not until the end of the fragment.

@@ +2961,3 @@
         if (dash_stream->sidx_parser.status == GST_ISOFF_SIDX_PARSER_FINISHED &&
+            (SIDX (dash_stream)->entry_index != 0) || (!stream->downloading_index) &&
+            ref_type1_found == FALSE) {

The parser status should be not FINISHED already if a ref_type of 1 was found. Is it not?

::: gst-libs/gst/adaptivedemux/gstadaptivedemux.c
@@ +3297,3 @@
           stream->fragment.index_range_end, NULL);
+      
+      gst_adaptive_demux_stream_update_fragment_info(stream->demux, stream);

Why is this needed exactly, what are the events that have to happen for this to be required?

Created attachment 361651 [details]
an example mpd file

Let me explain "not working as expected" in more details.

take the attached mpd file for example.

I expect the download sequence will be 
* http://xxx/video.mp4 range 0-819
* http://xxx/video.mp4 range 820-1511
* http://xxx/video.mp4 range 1512-12345
* http://xxx/video.mp4 range 12346-23456
...
...

however, current git master's download sequence
* http://xxx/video.mp4 range 0-819
* http://xxx/video.mp4 range 820-1511
* http://xxx/video.mp4 (NO RANGE HERE!!!!)


so I reported "a whole file is downloaded without using range download, and also bitrate switch is not available"

Review of attachment 361232 [details] [review]:

please review my explaination, thanks

::: ext/dash/gstdashdemux.c
@@ +1357,1 @@
       }

yes, I agree "The range request should only be until the end of the current SIDX entry", that is why here, range end shall be range.start + “current SIDX entry”‘'s size 

fragment.range_end is not updated

@@ +2964,1 @@
           /* Need to jump to the requested SIDX entry. Push everything up to

(!stream->downloading_index)  this is added when sidx range is not explicitly specified in the mpd file.
so that *sidx_seek_needed set to true, early terminate currently no-range downloading whole file, then jump to the requested SIDX entry by using sidx info.


'ref type 1' is not supported, so 'ref type 1' will downloaded as a whole file without using 'sidx' info.    ref_type1_found added here to guarantee this pattern not broken, otherwise 'ref type 1' will not be playback at all

::: gst-libs/gst/adaptivedemux/gstadaptivedemux.c
@@ +3297,3 @@
           stream->fragment.index_range_end, NULL);
+      
+      gst_adaptive_demux_stream_update_fragment_info(stream->demux, stream);

in 'gst_adaptive_demux_stream_download_loop',  'gst_adaptive_demux_stream_update_fragment_info' in called.
at this time, sidx is not retrieved and parsed yet.
fragment range will not be set.
after finish 'gst_adaptive_demux_stream_download_header_fragment', sidx is retrieved and parsed, however fragment range is not set yet.

'gst_adaptive_demux_stream_update_fragment_info’ is added here to update fragment range.
otherwise, the whole file will be downloaded without range request

Please explain that in more detail in the commit message, and also split it up into the separate functional changes you have there.

Created attachment 362953 [details] [review]
fix segmentBase type with 'sidx' not using range

add more detailed commit message

-- 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/619.