[Bug 781447] qtmux: Add new prefill recording mode

[GStreamer (GNOME Bugzilla)]

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

Jan Schmidt <thaytan at noraisin.net> changed:

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

--- Comment #2 from Jan Schmidt <thaytan at noraisin.net> ---
Review of attachment 349993:
 --> (https://bugzilla.gnome.org/review?bug=781447&attachment=349993)

Mostly looks good. Some small comments.

::: gst/isomp4/gstqtmux.c
@@ +296,3 @@
 #define DEFAULT_RESERVED_MOOV_UPDATE_PERIOD   GST_CLOCK_TIME_NONE
 #define DEFAULT_RESERVED_BYTES_PER_SEC_PER_TRAK 550
+#define DEFAULT_RESERVED_PREFILL TRUE

Should this be TRUE?

@@ +2214,3 @@
+    default:
+      GST_ERROR_OBJECT (qtmux, "unsupported codec for pre-filling");
+      return -1;

Slightly strange to put the default case handler in the middle of the switch.

@@ +2375,3 @@
+      if (walk == NULL) {
+        qpad->expected_sample_duration_n = 25;
+        qpad->expected_sample_duration_d = 1;

Might want some kind of discoverable log output for this fallback case?

@@ +2439,3 @@
+static gboolean
+gst_qt_mux_prefill_samples (GstQTMux * qtmux)
+{

The result of this function isn't checked anywhere, and doesn't seem to
generate any GST_ELEMENT_ERROR on unsupported codecs

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