Created attachment 367079 [details] [review]
qtmux: Make sure timecode uses the same timescale as video

Don't blindly derive it from the frame rate, but try to get the per-pad
configured timescale first (if it exists)

Review of attachment 367079 [details] [review]:

::: gst/isomp4/gstqtmux.c
@@ +2591,3 @@
 
+        atom_trak_set_timecode_type (qpad->tc_trak, qtmux->context,
+            gst_qt_mux_pad_get_timescale (qtpad), tc);

You should take the actually configured timescale here from qpad->trak_ste->mdia.mdhd.time_info.timescale or so.

It might be configured via the pad property, via the muxer property, or taken from the framerate. That decision was done already and stored in the qpad.

@@ +4092,3 @@
 
+    atom_trak_set_timecode_type (pad->tc_trak, qtmux->context,
+        gst_qt_mux_pad_get_timescale (qtpad), pad->first_tc);

Same here

Created attachment 367310 [details] [review]
qtmux: Make sure timecode uses the same timescale as video

Don't blindly derive it from the frame rate, but try to get the per-pad
configured timescale first (if it exists)

Thanks, fixed. There was also another issue: frame duration seemed to be hardcoded to 100 for the timecode track. Fixed that too.

Review of attachment 367310 [details] [review]:

Looks good except for one thing

::: gst/isomp4/atoms.c
@@ +3825,3 @@
   trak->mdia.mdhd.time_info.timescale =
+      trak_timescale !=
+      0 ? trak_timescale : atom_framerate_to_timescale (tc->config.fps_n,

This can never be 0, can it?

@@ +3837,3 @@
   tmcd->timescale =
+      trak_timescale !=
+      0 ? trak_timescale : atom_framerate_to_timescale (tc->config.fps_n,

This can never be 0, can it?

@@ +3839,3 @@
+      0 ? trak_timescale : atom_framerate_to_timescale (tc->config.fps_n,
+      tc->config.fps_d);
+  tmcd->frame_duration = tmcd->timescale * tc->config.fps_d / tc->config.fps_n;

You probably want to use gst_util_uint64_scale_int() here, just in case timescale and fps_d are both rather big.

Created attachment 367315 [details] [review]
qtmux: Make sure timecode uses the same timescale as video

Don't blindly derive it from the frame rate, but try to get the per-pad
configured timescale first (if it exists)

Attachment 367315 [details] pushed as 379059b - qtmux: Make sure timecode uses the same timescale as video