GNOME Bugzilla – Bug 770921
vaapidecoder loses input caps info
Last modified: 2016-09-30 07:04:41 UTC
When first created, the vaapidecoder takes the input state caps and generates codec_state, which is then managed / updated only by the decoder and changes are notified to vaapidecode via the gst_vaapi_decoder_state_changed() callback. Any changes the base decoder makes to the state will completely override the input_state - even if it's been subsequently updated by new sink caps. This is a problem when the initial caps are incomplete and updated later. For example: video/x-h264, stream-format=(string)byte-stream, alignment=(string)nal, parsed=(boolean)true then later video/x-h264, width=(int)1280, height=(int)1080, framerate=(fraction)25/1, pixel-aspect-ratio=(fraction)3/2, profile=(string)high, stream-format=(string)byte-stream, alignment=(string)nal, parsed=(boolean)true, level=(string)4 Later again, when the H.264 decoder realises the stream is interlaced, it will update the internal codec_state and then destroy the new info that arrived in the later caps, and the result will be output caps with: video/x-raw(memory:VASurface), format=(string)NV12, width=(int)1280, height=(int)1080, interlace-mode=(string)interleaved, pixel-aspect-ratio=(fraction)3/2, chroma-site=(string)mpeg2, colorimetry=(string)bt709, framerate=(fraction)0/1 (the framerate has been lost) That then means that vaapipostproc outputs both fields with identical timestamps after deinterlacing, because it's lost the frame duration.
mmmmhh... My gut tell me that this might be the cause of bug 766978... thanks Jan!
Created attachment 335420 [details] Test sample I can reproduce with this mpeg ts media. Note that this sample is made myself to test this issue. It has only video (h264,interlaced)
Created attachment 335425 [details] [review] vaapidecode: set codec state to decoder during set_format Specified decoder's codec state instance and vaapidecode plugin's one is different. It needs to synchronize them, so that it could avoid losing some video information during codec state change in decoder. I'm not sure yet if this does make sense. Please review this.
(In reply to Hyunjun Ko from comment #3) > Created attachment 335425 [details] [review] [review] > vaapidecode: set codec state to decoder during set_format > > Specified decoder's codec state instance and vaapidecode plugin's one is > different. > It needs to synchronize them, so that it could avoid losing some video > information during codec state change in decoder. > > I'm not sure yet if this does make sense. > Please review this. The problem with this approach is that it might destroy information that has been collected from the stream itself into the codec data. gst-libav and friends solve this problem by keeping the format info from the sinkpad caps separate from anything extracted from the video stream itself, and use information from both only when generating new output source caps - selectively overriding fields (such as framerate, multiview info or pixel-aspect-ratio) using info from the sinkpad caps if it's been provided.
I can reproduce attachment 335420 [details] without any issue with the current gstreamer-vaapi master :/
(In reply to Víctor Manuel Jáquez Leal from comment #5) > I can reproduce attachment 335420 [details] without any issue with the > current gstreamer-vaapi master > > :/ Also the mpd file that @slomo gave a couple days ago. What did change?
Comment on attachment 335425 [details] [review] vaapidecode: set codec state to decoder during set_format codec_state is an internal structure of the vaapidecoder, and keeps its own state of its own parsing.
I'm not sure what you mean by 'without any issue'. The problem is here (using git master): /GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0/GstDecodeBin:decodebin0/GstVaapiDecodeBin:vaapidecodebin0/GstVaapiPostproc:vaapipostproc.GstPad:sink: caps = video/x-raw(memory:VASurface), format=(string)NV12, width=(int)1920, height=(int)1080, interlace-mode=(string)interleaved, pixel-aspect-ratio=(fraction)1/1, chroma-site=(string)mpeg2, colorimetry=(string)bt709, framerate=(fraction)0/1 By the time the caps get to vaapipostproc, the framerate is lost from the info that vaapidecode received (it just didn't get it in the *first* caps it received): vaapidecodebin0/GstVaapiDecode:vaapidecode.GstPad:sink: caps = video/x-h264, stream-format=(string)byte-stream, alignment=(string)nal, pixel-aspect-ratio=(fraction)1/1, width=(int)1920, height=(int)1080, framerate=(fraction)25/1, parsed=(boolean)true, profile=(string)high, level=(string)4 and in this file, the result is that the video sink receives 2 buffers with timestamps at the start, and then every buffer after that has pts/dts none. The 2 buffers that do have timestamps, have the *same* timestamp, because vaapipostproc didn't know a framerate / field duration to move the 2nd timestamp by half a buffer duration. /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (3133440 bytes, dts: none, pts: 0:00:00.125000000, duration: 0:00:00.000000000, offset: -1, offset_end: -1, flags: 00000000 ) 0x7fcb9c0a1a40 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (3133440 bytes, dts: none, pts: 0:00:00.125000000, duration: 0:00:00.000000000, offset: -1, offset_end: -1, flags: 00000040 discont ) 0x7fcbac020e60 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (3133440 bytes, dts: none, pts: none, duration: 0:00:00.000000000, offset: -1, offset_end: -1, flags: 00000000 ) 0x7fcb9c0a13e0 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (3133440 bytes, dts: none, pts: none, duration: 0:00:00.000000000, offset: -1, offset_end: -1, flags: 00000000 ) 0x7fcba0044d00 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (3133440 bytes, dts: none, pts: none, duration: 0:00:00.000000000, offset: -1, offset_end: -1, flags: 00000000 ) 0x7fcb9c0a1820 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (3133440 bytes, dts: none, pts: none, duration: 0:00:00.000000000, offset: -1, offset_end: -1, flags: 00000000 ) 0x7fcb9c0a10b0 /GstPipeline:pipeline0/GstFakeSink:fakesink0: last-message = chain ******* (fakesink0:sink) (3133440 bytes, dts: none, pts: none, duration: 0:00:00.000000000, offset: -1, offset_end: -1, flags: 00000000 ) 0x7fcba0044d00
Created attachment 335518 [details] [review] vaapidecode: reset decoder hard when set_format() set_format() is called by upstream when the stream capabilites has changed. Before, if the new stream is compatible with the old one the VA decoder was not destroyed. Nonetheless, with this behavoir, the VA decoder ignores when the upstreamer parsers gets more details of the stream, such as the framerate. Hence, when the src caps are negotiates, the further sink caps updates are ignored. This patch forces the VA decoder destroying and recreation when set_format() is called.
Well, I guess that's one way to fix it.
(In reply to Jan Schmidt from comment #10) > Well, I guess that's one way to fix it. It can be improved, merging the previous caps with the new one if they are compatible. Or having a way to set the new codec state to the VA decoder.
(In reply to Víctor Manuel Jáquez Leal from comment #11) > (In reply to Jan Schmidt from comment #10) > > Well, I guess that's one way to fix it. > > It can be improved, merging the previous caps with the new one if they are > compatible. Or having a way to set the new codec state to the VA decoder. I think so, but for that it would need to preserve both the original state from the sink pad, and the decoder state separately, so as to selectively merge fields from each. This way works fine for this test case - and probably for most cases anyway. The reason the full caps are late in arriving is because the SPS/PPS aren't available for a while - so nothing is getting output before anyway and decoder reset produces the same decoded frames as previously. It might be slightly more overhead recreating a decoder that wasn't actually useful yet, but I suspect it's not enough to care about.
commit 99d404f1df588e7eb0fd3dced4cf19cec93a2120 Author: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com> Date: Wed Sep 14 16:29:01 2016 +0200 vaapidecode: reset decoder hard when set_format() set_format() is called by upstream when the stream capabilites has changed. Before, if the new stream is compatible with the old one the VA decoder was not destroyed. Nonetheless, with this behavoir, the VA decoder ignores when the upstreamer parsers gets more details of the stream, such as the framerate. Hence, when the src caps are negotiates, the further sink caps updates are ignored. This patch forces the VA decoder destroying and recreation when set_format() is called. https://bugzilla.gnome.org/show_bug.cgi?id=770921
This should go into 1.8 too
(In reply to Jan Schmidt from comment #14) > This should go into 1.8 too Done.