GNOME Bugzilla – Bug 580869
dvdsubdec ARGB support
Last modified: 2009-05-21 13:23:15 UTC
In dvdsubdec only AYUV, support is there. Adding ARGB support will help to skip the colorspace conversion using ffmpegcolorspace, on the whole frame instead only the subtitle part's conversion can be done. Other information:
Created attachment 133646 [details] [review] dvdsubdec element patch to support ARGB output.
Created attachment 134010 [details] [review] Cleaned the earlier patch
In general this seems like an OK idea. A couple of notes on the patch: * I'd prefer to use glib's CLAMP macro rather than the inline clip() function. * Move the use_ARGB check out of the DRAW_RUN inner loop. Instead, I'd redefine the Color_val struct to be guint A, Y_R, U_G, V_B and define separate palette caches: palette_cache_rgb, hl_palette_cache_rgb. That way, you can switch to select the different color palette outside the inner loop, and avoid the extra check. * Are you actually using dvdsubdec successfully somewhere? I stopped using it in favour of gstdvdspu, so I'm not sure how complete its functionality is any more. * In the sinkpad setcaps function, the gst_pad_fixate_caps call looks wrong - you want to check the entire set of allowed caps for ARGB support, not the fixated set. Also, a bit further down - if the peer accepts the proposed caps, you break from the loop without setting the use_ARGB flag. It looks like use_ARGB will only get set if ARGB is in the set of allowed caps, but the peer rejects it. At which point, it will set ARGB as the output caps, but not the flag that actually renders in that mode.
Created attachment 134721 [details] [review] Cleaned as per Jan suggestion Thanks Jan, I had attached the patch as per your suggestions. Please let me know if there is anything missing.
Created attachment 135091 [details] [review] Removed clip from patch Please check, I had tested , it works properly for me.
I think this looks fine now - apart from the common submodule change bit, that should not go in.
Thanks, committed: commit 59bd88e4bdd0a0ef7d5a1b0b3813eac8708f3fe0 Author: Kapil Agrawal <kapil@mediamagictechnologies.com> Date: Thu May 21 14:20:22 2009 +0100 dvdsubdec: Support ARGB output Negotiate to and render into ARGB buffers directly if the peer supports it. Fixes: #580869