GNOME Bugzilla – Bug 758258
matroska: Missing support for prores video
Last modified: 2015-12-18 16:51:49 UTC
Created attachment 315802 [details] mkv with prores video which fails to play Play the attached file with gst-play, notice it fails: $ gst-play-1.0 00813_short.mkv Press 'k' to see a list of keyboard shortcuts. Now playing 00813_short.mkv WARNING No decoder available for type 'video/x-unknown, codec-id=(string)V_PRORES'. WARNING debug information: gsturidecodebin.c(939): unknown_type_cb (): /GstPlayBin:playbin/GstURIDecodeBin:uridecodebin0 0:00:00.0 / 0:00:00.0 Reached end of play list. The file has been obtained like this: ffmpeg -i 00813.mp4 -t 0.05 -c:a opus -b:a 256000 -c:v prores -profile:v 1 -sn 00813_short.mkv
The problem here is that the matroska demuxer doesn't know about prores.
Created attachment 315804 [details] [review] matroska-demux: Recognise ProRes video streams Generate video/x-prores caps for ProRes video streams. Doesn't make files actually play, because it seems every frame needs an 8 byte header prepended, as described in http://wiki.multimedia.cx/index.php?title=Apple_ProRes#Frame_layout
Created attachment 315805 [details] [review] matroska-demux: Play ProRes video streams Generate video/x-prores caps for ProRes video streams. Every frame needs an 8 byte header prepended, as described in http://wiki.multimedia.cx/index.php?title=Apple_ProRes#Frame_layout so do that in a post-processing callback.
Review of attachment 315805 [details] [review]: ::: gst/matroska/matroska-demux.c @@ +2995,3 @@ + gst_buffer_unmap (newbuf, &map); + gst_buffer_append (newbuf, *buf); + *buf = newbuf; *buf = gst_buffer_append (newbuf, *buf); append() returns a new buffer and takes ownership of both its arguments @@ +5268,3 @@ + + GST_LOG ("Prores video, codec fourcc %" GST_FOURCC_FORMAT, + GST_FOURCC_ARGS (fourcc)); Should we put the fourcc into the caps? @@ +5271,3 @@ + + caps = gst_caps_new_empty_simple ("video/x-prores"); + *codec_name = g_strdup_printf ("Apple ProRes"); This stuff should everywhere use pbutils :) But at a later time
Created attachment 315810 [details] [review] matroska-demux: Play ProRes video streams Generate video/x-prores caps for ProRes video streams. Every frame needs an 8 byte header prepended, as described in http://wiki.multimedia.cx/index.php?title=Apple_ProRes#Frame_layout so do that in a post-processing callback.
(In reply to Sebastian Dröge (slomo) from comment #4) > Review of attachment 315805 [details] [review] [review]: > > ::: gst/matroska/matroska-demux.c > @@ +2995,3 @@ > + gst_buffer_unmap (newbuf, &map); > + gst_buffer_append (newbuf, *buf); > + *buf = newbuf; > > *buf = gst_buffer_append (newbuf, *buf); > > append() returns a new buffer and takes ownership of both its arguments Thanks! Fixed. > @@ +5268,3 @@ > + > + GST_LOG ("Prores video, codec fourcc %" GST_FOURCC_FORMAT, > + GST_FOURCC_ARGS (fourcc)); > > Should we put the fourcc into the caps? Looking at qtmux, it seems the fourcc should be mapped to one of 4 'variant=(string)..." values. That should allow remuxing, and gives matroska-mux the info it would need to implement handling too. > @@ +5271,3 @@ > + > + caps = gst_caps_new_empty_simple ("video/x-prores"); > + *codec_name = g_strdup_printf ("Apple ProRes"); > > This stuff should everywhere use pbutils :) But at a later time Is that just gst_pb_utils_get_codec_description() ?
(In reply to Jan Schmidt from comment #6) > > @@ +5271,3 @@ > > + > > + caps = gst_caps_new_empty_simple ("video/x-prores"); > > + *codec_name = g_strdup_printf ("Apple ProRes"); > > > > This stuff should everywhere use pbutils :) But at a later time > > Is that just gst_pb_utils_get_codec_description() ? There's also a function that directly creates the codec tag from caps, but yes
Created attachment 315812 [details] [review] matroska-mux: Implement prores support
Comment on attachment 315812 [details] [review] matroska-mux: Implement prores support >+ } else if (strcmp (mimetype, "video/x-prores") == 0) { >+ guint32 fourcc; >+ const gchar *variant; >+ >+ variant = gst_structure_get_string (structure, "format"); >+ if (!variant || !g_strcmp0 (variant, "standard")) >+ fourcc = GST_MAKE_FOURCC ('a', 'p', 'c', 'n'); >+ else if (!g_strcmp0 (variant, "lt")) >+ fourcc = GST_MAKE_FOURCC ('a', 'p', 'c', 's'); >+ else if (!g_strcmp0 (variant, "hq")) >+ fourcc = GST_MAKE_FOURCC ('a', 'p', 'c', 'h'); >+ else if (!g_strcmp0 (variant, "proxy")) >+ fourcc = GST_MAKE_FOURCC ('a', 'p', '4', 'h'); >+ >+ gst_matroska_mux_free_codec_priv (context); >+ context->codec_priv = g_malloc (sizeof (guint32)); >+ *((guint32 *) context->codec_priv) = fourcc; >+ context->codec_priv_size = sizeof (guint32); This could be simplified, with the added advantage that one doesn't have to wonder whether it's endian-safe or not, e.g. why not just do: if (if (!g_strcmp0 (variant, "foo")) context->codec_priv = g_strdup ("abcd"); ... context->codec_priv_size = 4; ?
Review of attachment 315812 [details] [review]: fourcc not being set must be handled.
Created attachment 315817 [details] [review] matroska-mux: Implement prores support
(In reply to Thibault Saunier from comment #11) > Created attachment 315817 [details] [review] [review] > matroska-mux: Implement prores support I just used the proposed simplification (I did not think about it before, nice idea indeed)
(In reply to Thibault Saunier from comment #11) > Created attachment 315817 [details] [review] [review] > matroska-mux: Implement prores support Would buf = gst_buffer_copy(); gst_buffer_resize(buf); be easier? My impression is that the timestamps are kept in the case, but I may be wrong.
Created attachment 315870 [details] [review] matroska-mux: Implement prores support
(In reply to Olivier Crête from comment #13) > (In reply to Thibault Saunier from comment #11) > > Created attachment 315817 [details] [review] [review] [review] > > matroska-mux: Implement prores support > > Would buf = gst_buffer_copy(); gst_buffer_resize(buf); be easier? My > impression is that the timestamps are kept in the case, but I may be wrong. buf = gst_buffer_make_writable(); gst_buffer_resize(buf); indeed.
Review of attachment 315817 [details] [review]: ::: gst/matroska/matroska-mux.c @@ +1267,3 @@ + context->codec_priv = g_strdup ("apcs"); + else if (!g_strcmp0 (variant, "proxy")) + context->codec_priv = g_strdup ("apco"); This doesn't handle variant='4444' for fourcc 'ap4h' as in http://wiki.multimedia.cx/index.php?title=Apple_ProRes
Created attachment 316387 [details] [review] matroska-demux: Play ProRes video streams Generate video/x-prores caps for ProRes video streams. Every frame needs an 8 byte header prepended, as described in http://wiki.multimedia.cx/index.php?title=Apple_ProRes#Frame_layout so do that in a post-processing callback.
Updated the demuxer patch to include the variant info in the caps and tag.
Created attachment 316418 [details] [review] matroska-mux: Implement prores support
Updated withvariant=4444.
These seem sensible - pushing.