After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 755411 - mpdparser: Only check stream->segments for a repeated last segment if we have a static list of segments
mpdparser: Only check stream->segments for a repeated last segment if we have...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
unspecified
Other All
: Normal normal
: 1.6.0
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-09-22 12:44 UTC by Sebastian Dröge (slomo)
Modified: 2015-09-22 13:04 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
mpdparser: Only check stream->segments for a repeated last segment if we have a static list of segments (1.25 KB, patch)
2015-09-22 12:44 UTC, Sebastian Dröge (slomo) 		committed Details | Review

Description Sebastian Dröge (slomo) 2015-09-22 12:44:41 UTC 
See commit message
Comment 1 Sebastian Dröge (slomo) 2015-09-22 12:44:45 UTC 
Created attachment 311853 [details] [review]
mpdparser: Only check stream->segments for a repeated last segment if we have a static list of segments

Otherwise we'll crash, trying to derefence NULL. And if we have no static list
of segments, we can't have repeated segments anyway.

Regression introduced by cfe2871a5e3c5b1db20470927642b776e055a87a
Comment 2 Thiago Sousa Santos 2015-09-22 12:49:37 UTC 
Review of attachment 311853 [details] [review]:

::: ext/dash/gstmpdparser.c
@@ +4755,3 @@
 
+    if (segments_count > 0 && stream->segments
+        && stream->segment_index + 1 == segments_count) {

Shouldn't segments_count be 0 when stream->segments == NULL?

Fixing the root cause here seems safer.
Comment 3 Sebastian Dröge (slomo) 2015-09-22 12:56:32 UTC 
(In reply to Thiago Sousa Santos from comment #2)
> Review of attachment 311853 [details] [review] [review]:
> 
> ::: ext/dash/gstmpdparser.c
> @@ +4755,3 @@
>  
> +    if (segments_count > 0 && stream->segments
> +        && stream->segment_index + 1 == segments_count) {
> 
> Shouldn't segments_count be 0 when stream->segments == NULL?

No, you can still have a finite number of segments... based on a segment template, the segment durations and the duration of the period.
Comment 4 Thiago Sousa Santos 2015-09-22 13:00:30 UTC 
Review of attachment 311853 [details] [review]:

::: ext/dash/gstmpdparser.c
@@ +4755,3 @@
 
+    if (segments_count > 0 && stream->segments
+        && stream->segment_index + 1 == segments_count) {

Seems I was wrong, it can be NULL, indeed.

Thanks for the fix.
Comment 5 Sebastian Dröge (slomo) 2015-09-22 13:04:13 UTC 
commit a25253130b573c842805439e424eb048c1ef9bbc
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Tue Sep 22 14:43:44 2015 +0200

    mpdparser: Only check stream->segments for a repeated last segment if we have a static list of segments
    
    Otherwise we'll crash, trying to derefence NULL. And if we have no static list
    of segments, we can't have repeated segments anyway.
    
    Regression introduced by cfe2871a5e3c5b1db20470927642b776e055a87a
    
    https://bugzilla.gnome.org/show_bug.cgi?id=755411