GNOME Bugzilla – Bug 739598
rfbsrc: incorrectly calculates caps
Last modified: 2016-04-05 19:39:44 UTC
Created attachment 289955 [details] [review] patch implementing _get_caps and _set_caps rfbsrc calculates its caps from RfbDecoder after a connection is made to a server and then sets the caps on the pad. However, when caps negotiation happens, caps are calculated from the pad's template caps and the peer's caps. This usually means that RGB is selected, but it does not reflect the format of the data the decoder is producing. That is to say, RGB will probably be negotiated, but the pushed buffers could actually contain BGRx video data. To fix this, I've squirreled away the appropriate caps in the RfbDecoder and use those in overriding _get_caps and _set_caps. I'm unfamiliar with caps negotiation, so please point me in the right direction if this isn't correct.
Right, so basically there's nothing to negotiate, right? rfbsrc just outputs whatever it gets from the other end. I think this can probably be achieved with less code. Don't think we need to implement ::set_caps() at all, that's only so that basesrc can notify the subclass of the outcome of the negotiation it does in ::negotiate(). What we really want is perhaps to implement our own ::negotiate() function that does the gst_pad_set_caps() + negotiate_pool(). We also don't need that ::fixate() function if there's nothing to fixate (ie. to narrow down the range of possible caps to a specific set of caps). The get_caps may also not be needed then (could do gst_pad_use_fixed_caps() even).
Are you planning on updating your patch?
(In reply to comment #2) > Are you planning on updating your patch? Yes, sorry. Your reply got buried under an email mountain. I'm unfamiliar with all the caps mechanisms, so thanks for your guidance. I'll update my patch today.
I've made couple of patches to try and improve the situation with this element. It's still far from perfect, but hopefully it helps. About the pixel formats, it's not totally exact to say that we need to obey to the server. What we currently do, is read the serve "natural" pixel format from the ServerInit message. Though, we could try and set another pixel format, by sending a SetPixelFormat message. This is not a negotiation, the spec pretends server should support everything, while in practice they don't. So ideally, if it's a known format and downstream support it, we should always use the natural format, otherwise we should try and set a pixel format compatible with downstream. Of course, we could also finish fixing the "natural" format first, and then consider adding SetPixelFormat support. In this case, we need to do what Tim said.
Fix in commit d3d34b.