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 763968 - qtdemux: Add check condition for fail case in get_duration function
qtdemux: Add check condition for fail case in get_duration function
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal enhancement
: 1.9.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-03-21 02:39 UTC by Jimmy Ohn
Modified: 2016-03-24 12:38 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
qtdemux: Add check condition for fail case in get_duration function (3.65 KB, patch)
2016-03-21 02:40 UTC, Jimmy Ohn 		none Details | Review
qtdemux: Add check condition for fail case in get_duration function (2.12 KB, patch)
2016-03-22 04:22 UTC, Jimmy Ohn 		committed Details | Review

Description Jimmy Ohn 2016-03-21 02:39:22 UTC 
Currently, get_duration function always return the TRUE value even though it can't be set the duration correctly. So, we need to add the else condition about the fail case.
So I have modify it and related caller function.
Comment 1 Jimmy Ohn 2016-03-21 02:40:21 UTC 
Created attachment 324407 [details] [review]
qtdemux: Add check condition for fail case in get_duration function
Comment 2 Sebastian Dröge (slomo) 2016-03-21 08:43:37 UTC 
Review of attachment 324407 [details] [review]:

::: gst/isomp4/qtdemux.c
@@ +834,3 @@
+
+          gst_qtdemux_get_duration (qtdemux, &duration);
+          if (duration > 0) {

Why do you remove the check for the return value here?

@@ +4422,3 @@
 
+  if (G_UNLIKELY (QTSEGMENT_IS_EMPTY (&stream->
+              segments[stream->segment_index]))) {

No useless indentation changes please :)

@@ +5187,3 @@
   /* If we're doing a keyframe-only trickmode, only push keyframes on video streams */
+  if (G_UNLIKELY (qtdemux->segment.
+          flags & GST_SEGMENT_FLAG_TRICKMODE_KEY_UNITS)) {

No useless indentation changes please :)
Comment 3 Jimmy Ohn 2016-03-21 08:46:17 UTC 
(In reply to Sebastian Dröge (slomo) from comment #2)
> Review of attachment 324407 [details] [review] [review]:
> 
> ::: gst/isomp4/qtdemux.c
> @@ +834,3 @@
> +
> +          gst_qtdemux_get_duration (qtdemux, &duration);
> +          if (duration > 0) {
> 
> Why do you remove the check for the return value here?
> 
> @@ +4422,3 @@
>  
> +  if (G_UNLIKELY (QTSEGMENT_IS_EMPTY (&stream->
> +              segments[stream->segment_index]))) {
> 
> No useless indentation changes please :)
> 
> @@ +5187,3 @@
>    /* If we're doing a keyframe-only trickmode, only push keyframes on video
> streams */
> +  if (G_UNLIKELY (qtdemux->segment.
> +          flags & GST_SEGMENT_FLAG_TRICKMODE_KEY_UNITS)) {
> 
> No useless indentation changes please :)

Actually, I use the gst-indent tools about the qtdemux.c file. But If you don't agree it, I'll modify it.:)
Comment 4 Jimmy Ohn 2016-03-22 04:22:21 UTC 
Created attachment 324518 [details] [review]
qtdemux: Add check condition for fail case in get_duration function
Comment 5 Sebastian Dröge (slomo) 2016-03-24 12:38:03 UTC 
Attachment 324518 [details] pushed as c633f2a - qtdemux: Add check condition for fail case in get_duration function