GNOME Bugzilla – Bug 698854
[applemedia] Several improvements for the applemedia plugins
Last modified: 2013-05-20 11:31:32 UTC
This set of patches provides the following changes/improvements to the applemedia plugin: 1) Move plugins using private frameworks to applemedia-nonpublic 2) Don't build the dynamic API for public frameworks, but use them directly 3) Only use public API in CoreMedia 4) Use the best colorspace on each platform to avoid colorspace conversions 5) Add support for MPEG-2 decoding. There is an exception in 2) for VideoToolbox, which went public in the 10.8 SDK. To be able to built it against older SDK's and in iOS we keep using the dynamic API for this framework. Branch: https://github.com/ylatuya/gst-plugins-bad/tree/applemedia-1.0
Looks all good to me except for the MPEG2 commit: "video/mpeg" is not enough for the caps, it should be "video/mpeg,mpegversion=2,systemstream=false". And I guess this also supports MPEG1, so mpegversion=[1-2] should work too. Is there no support for MPEG4 and/or h263?
(In reply to comment #1) > Looks all good to me except for the MPEG2 commit: "video/mpeg" is not enough > for the caps, it should be "video/mpeg,mpegversion=2,systemstream=false". And I > guess this also supports MPEG1, so mpegversion=[1-2] should work too. > > Is there no support for MPEG4 and/or h263? mpegversion=2 is set afterwards (see the second chunk https://github.com/ylatuya/gst-plugins-bad/commit/e7cf1a8676e72ccedc590979056047a400c0942a) but systemstream is not set. I should check mpeg1 too. My hardware does not support mpeg4 so I couldn't test it although it should be straightforward adding it.
More commits in https://github.com/ylatuya/gst-plugins-bad/commits/applemedia-1.0: * fix caps for mpeg: mpegversion = [1-2], systemstream = false * fix H264 with b-frames
https://github.com/ylatuya/gst-plugins-bad/commit/95864c5dec23924a269b2910d90c86ebfec33107 You know that you can set multiple fields with a single gst_structure_set() call? ;) https://github.com/ylatuya/gst-plugins-bad/commit/565455fc12a1aaa38e20897aa9f82e4665abfcc4 Is it guaranteed that we always get a callback for each buffer we pass to the decode function? Looks good to me but I'm worried about buffer leaks here. Would be less of a problem if GstVideoDecoder was used, then you could just pass the frame ID there and the buffer is not really lost and can be garbage collected later. Otherwise looks all good to me :)
(In reply to comment #4) > > https://github.com/ylatuya/gst-plugins-bad/commit/565455fc12a1aaa38e20897aa9f82e4665abfcc4 > > Is it guaranteed that we always get a callback for each buffer we pass to the > decode function? Looks good to me but I'm worried about buffer leaks here. > Would be less of a problem if GstVideoDecoder was used, then you could just > pass the frame ID there and the buffer is not really lost and can be garbage > collected later. > I moved back the unref of the input buffer to decode_buffer () :) https://github.com/ylatuya/gst-plugins-bad/blob/908d9fb1d81503a8d25c95684ac0a61ae00dc99a/sys/applemedia/vtdec.c#L484
(In reply to comment #4) > https://github.com/ylatuya/gst-plugins-bad/commit/95864c5dec23924a269b2910d90c86ebfec33107 > > You know that you can set multiple fields with a single gst_structure_set() > call? ;) > Right, will fix that too
Can we push this? It's an obvious improvement and we can keep fixing more things in git. Good job Andoni!
Yes please, there's no point in waiting :)
http://www.youtube.com/watch?v=K8E_zMLCRNg
All pushed now