[Bug 758515] dashdemux : suggestion for setting the framerate info at dashdemux.

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=758515

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #316686|none                        |needs-work
             status|                            |

--- Comment #9 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 316686:
 --> (https://bugzilla.gnome.org/review?bug=758515&attachment=316686)

::: ext/dash/gstdashdemux.c
@@ +868,3 @@
   guint width = 0, height = 0;
+  gint fps_num = 0, fps_den = 1;
+  gboolean have_ret = FALSE;

have_fps maybe? have_ret is very generic

::: ext/dash/gstmpdparser.c
@@ +5642,3 @@
+  if (stream == NULL || stream->cur_adapt_set == NULL
+      || stream->cur_representation == NULL)
+    return 0;

FALSE instead of 0

And maybe add the checks below, like ...

@@ +5644,3 @@
+    return 0;
+
+  if (stream->cur_adapt_set->maxFrameRate != NULL) {

if (stream->cur_adapt_set && stream->cur_adapt_set->maxFrameRate != NULL) { ...
}

@@ +5646,3 @@
+  if (stream->cur_adapt_set->maxFrameRate != NULL) {
+    *fps_num = stream->cur_adapt_set->maxFrameRate->num;
+    *fps_den = stream->cur_adapt_set->maxFrameRate->den;

We should probably prefer the framerate attribute and only fall back to
maxFramerate if framerate does not exist. According to the spec, both
attributes can be in AdaptationSet, Representation and SubRepresentation

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.