[Bug 755614] qtdemux: support for cenc auxiliary info parsing outside of moof box

[GStreamer (GNOME Bugzilla)]

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

Tim-Philipp Müller <t.i.m at zen.co.uk> changed:

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

--- Comment #6 from Tim-Philipp Müller <t.i.m at zen.co.uk> ---
Comment on attachment 313249
  --> https://bugzilla.gnome.org/attachment.cgi?id=313249
qtdemux: support for cenc auxiliary info parsing outside of moof box

Looks fine, but didn't test myself (I saw that others did, of course). Would
appreciate a file or stream for my collection though.

Just a few small nitpicks:

>+  qtdemux->cenc_aux_info_offset = 0;
>+  qtdemux->info_sizes = NULL;
>+  qtdemux->sample_count = 0;

Can we make this cenc_aux_info_sizes and cenc_aux_sample_count or somesuch to
make it clear this is cenc-related? (esp. for the sample_count member)


>+  if (qtdemux->info_sizes) {
>+    g_free (qtdemux->info_sizes);
>+    qtdemux->info_sizes = NULL;
>+  }

g_free() is NULL-safe, don't need the if (same for the dozen other places
below).

>+      if (G_UNLIKELY (qtdemux->info_sizes == NULL)) {
>         GST_ERROR_OBJECT (qtdemux, "failed to parse saiz box");
>         goto fail;
>       }

Let's drop the G_UNLIKELY, this is not a performance-critical code path I
think? (I realise the original code has it, but if we are changing it we might
as well drop it; imho we use this way too often everywhere and the only thing
is does is makes code harder to read; this is a pet peeve of mine, feel free to
ignore it.)

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