GNOME Bugzilla – Bug 654294
videodecoder: impossible to flush pending frames in ::set_format
Last modified: 2013-02-11 10:57:52 UTC
When the video format changes and ::set_format() is called it is impossible to push any pending frames downstream. This is the case because the GstVideoState fields have changed already and you can't allocate new srcbuffers anymore. Ideally a copy of the GstVideoState is created and changed, passed to ::set_format() and afterwards, if the format was accepted, copied back over the original GstVideoState.
Indeed. Even worse, the video format should be synchronized with the internal pipeline of the encoder. Some encoders (schroedinger) can change formats without flushing the pipeline. This is important for low latency encoding.
On a similar subject, the newsegment events are applied immediately instead of waiting until the corresponding output buffers are pushed, i.e. the new segment is applied to old buffers and if it's a non-update newsegment all pending buffers are flushed. Do you plan to work on one or both of these issues? (Note: I implemented delaying pushing of serialized events until their buffers are pushed, at least that part is correct now ;) ).
I'd propose to handle ::set_format() different in the encoder and decoder. For the decoder just pass the caps to ::set_format() and only update the GstVideoState if the srcpad caps are set. For the decoder the state would be the output format, for the encoder the current input format. This probably makes more sense from an API point of view too. What do you think David?
The idea was that "format" was supposed to represent the uncompressed format. In the decoder, there's also the concept of "compressed format", which is any information that the decoder needs to pull from upstream caps. Little if any of this information is actually necessary to *decode* the video, except perhaps for a few screwy formats that i'm not remembering at the moment (WMV9, maybe?). However, things like pixel-aspect-ratio and framerate should be copied into the output caps. The copying should be done in the base class, preferably. So I think the correct concept is that GstVideoFormat should be reference counted, and attach it to each frame to represent the uncompressed format. Decoder should have a second compressed format attached. Once the proper copying is done in the base class, there should be very little need for decoders to implement set_format(). However, this leaves out one detail, which is codec_data. IMO, it's reasonable to handle this separately. On a related subject, we might want to have a high-level boolean that represents whether or not the codec can handle format changes on the fly. This is somewhat related to codec_data, since for example, an h264 decode can change format on the fly only when it's in annex b mode.
Is this still the case with GstVideoDecoder in -base ?
No, it's fixed