GNOME Bugzilla – Bug 750835
Adding stereoscopic/multiview API support
Last modified: 2015-09-10 15:46:35 UTC
Hi, Bug 611157 contains the last unlanded patches to add stereoscopic video support to GStreamer, and I expect them to land soon. This bug is a heads up that I'll be working on adding support to gstreamer-vaapi, and a place to land patches.
OK, thanks. Do you have actual stereo-high and multiview-high contents from real world? :)
I have ITU streams, and a couple of blurays (which have MVC split across MPEG-TS sub-streams and we don't support that yet), but if you have any you can share let me know :)
Created attachment 305159 [details] [review] multiview: Initial attempt at stereo/multiview support Add support for marking caps and buffers for multiview or stereoscopic output. Preliminary patch only, for discussion.
With this patch, and the patches in bug 611157 I can run pipelines like this: gst-launch-1.0 playbin uri=file://`pwd`/MVCICT-1.264 video-sink="glimagesink" yields downmixed anaglyph mono output by default or gst-launch-1.0 playbin uri=file://`pwd`/MVCICT-1.264 video-sink="glimagesink output-multiview-mode=side-by-side" to draw both views side by side gst-launch-1.0 playbin uri=file://`pwd`/MVCICT-1.264 video-sink="glimagesink output-multiview-mode=left" for the left eye view or gst-launch-1.0 playbin uri=file://`pwd`/MVCICT-1.264 video-sink="glimagesink output-multiview-mode=row-interleaved" for output suitable for display on a passive line-by-line 3dTV I put a couple of FIXMEs in the patch, where I'm not sure: * The profile in gstvaapidecoder_h264.c:ensure_context is simply HIGH, not HIGH_STEREO or HIGH_MULTIVIEW, so that can't be used as a key for setting output properties properly. I'm not sure if there's a way to distinguish 2 unassociated views from left/right eye stereo views. * In gstvaapidecode.c:gst_vaapi_decode_input_state_replace, a new input state with caps that are more specific than received from the parser were ignored completely, so the multiview mode, flags and views fields never got passed up to the gstvaapidecode level from the H.264 decoder. I'm not sure how else to ensure they get put on the output caps. It's not correct to say that parser should set the multiview mode - deciding to decode frame-by-frame is a decision of the decoder, not an inherent property of the h264 IMHO. Anyway, here's something to look at :)
Created attachment 307755 [details] [review] multiview: Initial attempt at stereo/multiview support Add support for marking caps and buffers for multiview or stereoscopic output. Preliminary patch only, for discussion.
Updated patch also has some changes to the encoder path if it has multiview-mode info in the sink caps, so it's possible to do MVC transcode without setting properties on the encoder explicitly. I'd like to also have a way to set the 'view-ids' - but I'm not sure which mechanism to use for passing them. Options: * Add optional view-ids field to the video caps MVC decoders and encoder can add a view-ids array when in multiview-mode=multiview-frame-by-frame, same as the view-ids property on the vaapi encoder * tag event More available to applications, but not negotiable, but can be sent along with caps * Buffer meta. Not great - means elements won't know the view-id mappings until the first buffer arrives, but does provide more flexibility than a plain guint array of 'view ids', which seem a bit H.264 specific (maybe that's fine - H.264 is the only extant MVC format I know) I still wouldn't mind input from Victor about the gst_vaapi_decode_input_state_replace() logic that calls gst_caps_is_always_compatible(). I'm not sure what the code is/was trying to achieve. I put a FIXME next to it.
(In reply to Jan Schmidt from comment #6) > I still wouldn't mind input from Victor about the > gst_vaapi_decode_input_state_replace() logic that calls > gst_caps_is_always_compatible(). I'm not sure what the code is/was trying to > achieve. I put a FIXME next to it. It is an optimization: if the new input state is "compatible" with the current set, then do nothing. Otherwise, the output caps are re-negotiated. Perhaps it's not the best to do and I would gladly accept any advice or improvement :)
(In reply to Jan Schmidt from comment #6) > I still wouldn't mind input from Victor about the > gst_vaapi_decode_input_state_replace() logic that calls > gst_caps_is_always_compatible(). I'm not sure what the code is/was trying to > achieve. I put a FIXME next to it. "Well, actually", bug 752558 might be caused because of gst_caps_is_always_compatible()
(In reply to Víctor Manuel Jáquez Leal from comment #7) > (In reply to Jan Schmidt from comment #6) > > I still wouldn't mind input from Victor about the > > gst_vaapi_decode_input_state_replace() logic that calls > > gst_caps_is_always_compatible(). I'm not sure what the code is/was trying to > > achieve. I put a FIXME next to it. > > It is an optimization: if the new input state is "compatible" with the > current set, then do nothing. Otherwise, the output caps are re-negotiated. > > Perhaps it's not the best to do and I would gladly accept any advice or > improvement :) May we can use gst_caps_is_equal() and if not ,do the renegotiation...
Review of attachment 307755 [details] [review]: ::: gst-libs/gst/vaapi/gstvaapidecoder_h264.c @@ +1455,3 @@ + if (reset_context) { + /* FIXME: Is it valid to assume 2 views is stereo, or should we + * check constraints? */ Check the value of sps->profile_idc 118 for multiview and 128 for stereo....
(In reply to Víctor Manuel Jáquez Leal from comment #7) > (In reply to Jan Schmidt from comment #6) > > I still wouldn't mind input from Victor about the > > gst_vaapi_decode_input_state_replace() logic that calls > > gst_caps_is_always_compatible(). I'm not sure what the code is/was trying to > > achieve. I put a FIXME next to it. > > It is an optimization: if the new input state is "compatible" with the > current set, then do nothing. Otherwise, the output caps are re-negotiated. > > Perhaps it's not the best to do and I would gladly accept any advice or > improvement :) I think the problem I saw it cause was related to trying to add extra fields to the caps that weren't present before, and they just got ignored... but I can't reproduce it now, and things seem to work without changing gst_vaapi_decode_input_state_replace(). I'm tempted to drop that part of the patch and see if there's problems.
Created attachment 308000 [details] [review] multiview: Initial attempt at stereo/multiview support Add support for marking caps and buffers for multiview or stereoscopic output. Preliminary patch only, for discussion.
New patch, with the 2 FIXMEs now gone.
(In reply to Víctor Manuel Jáquez Leal from comment #8) > "Well, actually", bug 752558 might be caused because of > gst_caps_is_always_compatible() Oops, that's not related at all. Sorry the noise.
(In reply to Jan Schmidt from comment #4) > * The profile in gstvaapidecoder_h264.c:ensure_context is simply HIGH, not > HIGH_STEREO or HIGH_MULTIVIEW, so that can't be used as a key for setting > output properties properly. I'm not sure if there's a way to distinguish 2 > unassociated views from left/right eye stereo views. The base-view might have only Sps headers, and which will advertise the profile as HIGH. The non-base view have subset-sps which advertise the profile as STEREO/MULTI.. This is the reason even for h264 parse to set output caps twice, first "profile=high" and then "profile=stereo"... In gstvaapidecoder_h264.c: we won't reset the context for profile change in stereo/mutliview. We only reset the context for a profile change, if the stream is non-setereo/non-multivew... So basically the value of priv->profile should be always high :).. The logic is correct, but for clarity I would prefer to set priv->profile = new_profile even though there is no context_reset. But doesn't matter for now.. > * In gstvaapidecode.c:gst_vaapi_decode_input_state_replace, a new input > state with caps that are more specific than received from the parser were > ignored completely, so the multiview mode, flags and views fields never got > passed up to the gstvaapidecode level from the H.264 decoder. I'm not sure > how else to ensure they get put on the output caps. It's not correct to say > that parser should set the multiview mode - deciding to decode > frame-by-frame is a decision of the decoder, not an inherent property of the > h264 IMHO. > > Anyway, here's something to look at :)
Regarding the usage of gst_caps_is_always_compatible(): You were right, the usage won't be correct in all use cases. For eg: If the parser didn't set any mulitvew-mode , and decoder provides a new caps with multiview-mode field, it doesn't get updated because of the caps_subset() checking... I would rather prefer to use gst_caps_is_equal() or do the caps replace (renegotiation) if any of width, height, fps, pixel-aspect-ratio, interlaced, multiview-mode multiview-flags has been changed in the new caps from decoder..
Also with the new patch, gst-launch-1.0 -v filesrc location=MVCICT-1.264 ! vaapiparse_h264 ! vaapidecode ! glimagesink output-multiview-mode=side-by-side is not working for me !
(In reply to sreerenj from comment #16) > Regarding the usage of gst_caps_is_always_compatible(): > > You were right, the usage won't be correct in all use cases. For eg: If the > parser didn't set any mulitvew-mode , and decoder provides a new caps with > multiview-mode field, it doesn't get updated because of the caps_subset() > checking... > > I would rather prefer to use gst_caps_is_equal() or do the caps replace > (renegotiation) if any of width, height, fps, pixel-aspect-ratio, > interlaced, multiview-mode multiview-flags has been changed in the new caps > from decoder.. That feels sensible, even though I still can't recreate the failure I had before. Maybe that's because of the internal parser update so that it now sets the MVC profile
(In reply to sreerenj from comment #17) > Also with the new patch, > > gst-launch-1.0 -v filesrc location=MVCICT-1.264 ! vaapiparse_h264 ! > vaapidecode ! glimagesink output-multiview-mode=side-by-side > > is not working for me ! That's working for me. How does it fail for you?
(In reply to Jan Schmidt from comment #19) > (In reply to sreerenj from comment #17) > > Also with the new patch, > > > > gst-launch-1.0 -v filesrc location=MVCICT-1.264 ! vaapiparse_h264 ! > > vaapidecode ! glimagesink output-multiview-mode=side-by-side > > > > is not working for me ! > > That's working for me. How does it fail for you? Hm, I am getting some weird results after updating gstreamer/base/good/bad/ to git master... .... vaapidecode ! glimagesink is not even running !! Are you using the gitmaster ? gstreamer : 815b5f69e805c032fc11c77c36e5b8ab5929856d plugins-base: 815b5f69e805c032fc11c77c36e5b8ab5929856d plugins-bad: 7db723831ddcc07f506e64e027b504ac16a067f7
> Are you using the gitmaster ? > gstreamer : 815b5f69e805c032fc11c77c36e5b8ab5929856d > plugins-base: 815b5f69e805c032fc11c77c36e5b8ab5929856d > plugins-bad: 7db723831ddcc07f506e64e027b504ac16a067f7 You're correct - after updating to git master, I can't play to glimagesink any more. Looks like a7d1b7fcadda78e9a5ad9071634a70606f937d26 (GstVideoOverlayCompositionMeta bits) in plugins-bad broke it.
(In reply to sreerenj from comment #20) > (In reply to Jan Schmidt from comment #19) > > (In reply to sreerenj from comment #17) > > > Also with the new patch, > > > > > > gst-launch-1.0 -v filesrc location=MVCICT-1.264 ! vaapiparse_h264 ! > > > vaapidecode ! glimagesink output-multiview-mode=side-by-side > > > > > > is not working for me ! > > > > That's working for me. How does it fail for you? > > Hm, I am getting some weird results after updating gstreamer/base/good/bad/ > to git master... > .... vaapidecode ! glimagesink is not even running !! See bug 752912
I am okay with the patch,, BTB why there is no frame-by-frame mode option in the input-mode-override property of glviewconvert ???
(In reply to sreerenj from comment #23) > BTB why there is no frame-by-frame mode option in the input-mode-override > property of glviewconvert ??? There's no sensible way to know the cadence of input frames to mark the views, or to handle seeks. Frame-by-frame and multiview really require that the input source knows already, in which case the frames are already marked as frame-by-frame.
Again, not getting the side-by-side view with the following pipeline: gst-launch-1.0 playbin uri=file://`pwd`/MVCICT-1.264 video-sink="glimagesink output-multiview-mode=side-by-side" plugins-bad: commit fd665514ba9f998d55508cd681f2a9c02f0f96a1
(In reply to Jan Schmidt from comment #24) > (In reply to sreerenj from comment #23) > > > BTB why there is no frame-by-frame mode option in the input-mode-override > > property of glviewconvert ??? > > There's no sensible way to know the cadence of input frames to mark the > views, or to handle seeks. Frame-by-frame and multiview really require that > the input source knows already, in which case the frames are already marked > as frame-by-frame. I was thinking to duplicate the frame rate with videorate, and then a glviewconvert having input-mode-override=frame-by-frame :)
(In reply to sreerenj from comment #25) > Again, not getting the side-by-side view with the following pipeline: > > gst-launch-1.0 playbin uri=file://`pwd`/MVCICT-1.264 video-sink="glimagesink > output-multiview-mode=side-by-side" > > plugins-bad: commit fd665514ba9f998d55508cd681f2a9c02f0f96a1 I'll check it out. Was working for me a few days ago. (In reply to sreerenj from comment #26) > (In reply to Jan Schmidt from comment #24) > > (In reply to sreerenj from comment #23) > > > > > BTB why there is no frame-by-frame mode option in the input-mode-override > > > property of glviewconvert ??? > > > > There's no sensible way to know the cadence of input frames to mark the > > views, or to handle seeks. Frame-by-frame and multiview really require that > > the input source knows already, in which case the frames are already marked > > as frame-by-frame. > > I was thinking to duplicate the frame rate with videorate, and then a > glviewconvert having input-mode-override=frame-by-frame :) I'm not sure what that does. In general, input-mode-override=frame-by-frame would work, if it was changed to not rely on the first-in-bundle flag - until you do a seek, then it might break depending on whether you landed on a odd frame or even frame. If you land on the wrong one, the views get swapped. In general it seemed better that we do frame-by-frame input override in the demuxers, where they can do the seeks and buffer markers right.
I got it working with current master, but I needed to comment this out: diff --git a/gst/vaapi/gstvaapidecode.c b/gst/vaapi/gstvaapidecode.c index b00eed1..6c4b6a5 100644 --- a/gst/vaapi/gstvaapidecode.c +++ b/gst/vaapi/gstvaapidecode.c @@ -150,7 +150,7 @@ gst_vaapi_decode_input_state_replace (GstVaapiDecode * decode, if (gst_caps_is_always_compatible (curcaps, new_state->caps)) { GST_DEBUG ("Ignoring new caps %" GST_PTR_FORMAT " is a subset of %" GST_PTR_FORMAT, new_state->caps, curcaps); - return FALSE; + /* return FALSE; */ } } gst_video_codec_state_unref (decode->input_state); I will change the gst_caps_is_always_compatible to gst_caps_equal, as Sree recommended. Using the vaapidecodebin (vaapidecoder + vaapipostproc) the playback is quite slow. Demoting vaapidecodebin, the pipeline doesn't work out of the box, flags=0x17 needs to be set, and then the playback is good.
Created attachment 310101 [details] [review] vaapidecode: renegotiate if caps are not equal When the new input state comes from the state_changed() callback, the address of the input_state variable is the same of the parameter. If it is the case, something has changed and it is required to re-negotiated. If the new state comes from set_format() virtual method, the address of the variable is different, so we have to compare the values. The use of gst_caps_is_always_compatible() for this optimization may lead to false positives. It is better to stick to gst_caps_is_strictly_equal() to know if it is required a re-negotiation. Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>
Pushed, a slightly different one though.. commit 32d1c5adff4c478a2b7aab0dc9a87de4839cd03b Author: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com> Date: Sat Aug 29 00:27:05 2015 +0300 vaapidecode: renegotiate if caps are not equal
Review of attachment 310101 [details] [review]: pushed..
The patch seems to work OK, with current master (either gstreamer and gstreamer-vaapi), with MVCICT-1.264 thaytan, sree, do we push it as is, or shall we wait more?
I vote for push :)
(In reply to Víctor Manuel Jáquez Leal from comment #32) > The patch seems to work OK, with current master (either gstreamer and > gstreamer-vaapi), with MVCICT-1.264 > > thaytan, sree, do we push it as is, or shall we wait more? I have no objection, If it works with the master... We still have to think about the view-id array option, but that can be done later ...