Are you going to work on a patch? :)

Created attachment 343375 [details] [review]
qtmux: add basic HEVC/H.265 muxing support

Since I am interested in HEVC muxing in qtmux, here is a patch to add this feature. It is basically the same code as h264. Resulting file, encoding with this pipeline:

gst-launch-1.0 videotestsrc num-buffers=60 ! x265enc ! h265parse ! qtmux ! filesink location=/tmp/out.mp4

is successfully read by vlc and mplayer, and also using GStreamer plugins.

Review of attachment 343375 [details] [review]:

Some things that are weird, but they're probably copy&paste from the h264 code. Maybe omit them here and provide another patch for h264 :)

::: gst/isomp4/gstqtmux.c
@@ +4240,3 @@
+
+    if ((strcmp (format, "hvc1") != 0 && strcmp (format, "hev1") != 0) ||
+        alignment == NULL || strcmp (alignment, "au") != 0) {

alignment can't possibly be NULL here (and stream-format can't be missing either). They're both in the template caps.

@@ +4261,3 @@
+      gint avg_bitrate = 0;
+
+      gst_structure_get_int (structure, "bitrate", &avg_bitrate);

This should come via tags if anything, the caps generally don't have the bitrate

(In reply to Sebastian Dröge (slomo) from comment #3)
> Review of attachment 343375 [details] [review] [review]:
> 
> Some things that are weird, but they're probably copy&paste from the h264
> code. Maybe omit them here and provide another patch for h264 :)
> 
> ::: gst/isomp4/gstqtmux.c
> @@ +4240,3 @@
> +
> +    if ((strcmp (format, "hvc1") != 0 && strcmp (format, "hev1") != 0) ||
> +        alignment == NULL || strcmp (alignment, "au") != 0) {
> 
> alignment can't possibly be NULL here (and stream-format can't be missing
> either). They're both in the template caps.

Indeed, I will remove all this tests as all are already done when intersecting with pad template.

> 
> @@ +4261,3 @@
> +      gint avg_bitrate = 0;
> +
> +      gst_structure_get_int (structure, "bitrate", &avg_bitrate);
> 
> This should come via tags if anything, the caps generally don't have the
> bitrate

Copy&paste from h264, I assumed that weird code use it, but it doesn't make sense and associated qtmux fields are set on tags event. I will provide a patch for h264.

and thanks for review.

Created attachment 343380 [details] [review]
qtmux: add basic HEVC/H.265 muxing support

Comment on attachment 343380 [details] [review]
qtmux: add basic HEVC/H.265 muxing support

Looks good, now just h264 before we forget again. thanks!

(In reply to Sebastian Dröge (slomo) from comment #6)
> Comment on attachment 343380 [details] [review] [review]
> qtmux: add basic HEVC/H.265 muxing support
> 
> Looks good, now just h264 before we forget again. thanks!

I opened a new bug for this:
https://bugzilla.gnome.org/show_bug.cgi?id=777181

Attachment 343380 [details] pushed as 4c6a69f - qtmux: add basic HEVC/H.265 muxing support