[Bug 737095] gstqtmux: subtitle muxing doesn't work

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=737095
  GStreamer | gst-plugins-good | git

Thiago Sousa Santos <thiagossantos> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #286762|none                        |reviewed
             status|                            |

--- Comment #5 from Thiago Sousa Santos <thiagossantos at gmail.com> 2014-09-22 20:08:57 UTC ---
Review of attachment 286762:
 --> (https://bugzilla.gnome.org/review?bug=737095&attachment=286762)

I would just leave the 0.15 thing as it seems to be the recommended value from
the spec.

For the other changes I'd prefer having smaller patches where we could put an
specific commit message for each. Makes it easier to review and track changes.

So, I'd propose 1 patch for the lock at collect pads, 1 for the duration and
buffer length fixes and another one for the atoms.c changes. What do you think?

::: gst/isomp4/atoms.c
@@ -2939,3 @@
     tx3g->font_size = 0.05 * height;

-    height = 0.15 * height;

"The subtitle track’s height should be 0.15 * the 'vide' track header height.
This allows room for two lines
of subtitle text. For example, if the 'vide' track header height is 720 pixels,
then the 'sbtl ' track header
height should be 108 (pixels)."

According to qtff spec.

@@ +3428,3 @@
+
+  trak->tkhd.alternate_group = 2;       /* same for all subtitles */
+  trak->tkhd.layer = -1;        /* above video (layer 0) */

Could you put this in a patch separate? It seems unrelated to the other changes
and it would be easier to track regressions or understand the commit better.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.