[FFmpeg-devel] [RFC] Dropping the rgb2rgb interface (was: [PATCH] make building swscale rgb template conditional)

[Michael Niedermayer]

On Thu, Sep 09, 2010 at 02:05:10AM -0300, Ramiro Polla wrote:
> On Sun, Sep 5, 2010 at 12:07 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Sun, Sep 05, 2010 at 03:22:06AM -0300, Ramiro Polla wrote:
> [...]
> >> Using global function pointers is obviously a bad idea. I'd suggest we
> >> drop exposing rgb2rgb but I faintly remember Michael being against
> >> that because some programs already use that interface and we'd have to
> >> bump major if we want to remove it, and also because it's useful to
> >> have a simpler alternative to get rgb2rgb functions than going through
> >> the swscale interface. Also I'd like to point out that on shared
> >> builds these global function pointers no longer work since only
> >> swscale_*, sws_*, and ff_* stuff is exported.
> >>
> >> If we must keep the rgb2rgb interface (not the global pointers, these
> >> should definitely go away at next major bump), I suggest we either:
> >
> >> 1- create a simpler function like sws_getRGBContext that only takes
> >> arguments relative to rgb2rgb (like src and dst pixel format), and
> >> that can be used with sws_scale()
> >
> > i see no sense in this ...
> 
> The rgb2rgb functions could be obtained with fewer parameters (for
> example there is no need to specify the algorithm for 1:1
> conversions).
> 
> >> 2- create an rgb2rgb context which the user can initialize and then
> >> use rgb->rgb24tobgr32() and such.
> >
> > maybe i dont know
> >
> >
> >>
> >> Michael, how do you suggest we proceed regarding the rgb2rgb interface?
> >
> > yes, what do the users of rgb2rgb think?
> > do they want to keep an alternative way to access it or can the main sws
> > api be used?
> 
> I searched for quite some time to try and find where rgb2rgb is being
> used. I mainly used google codesearch. The search strings tried to
> exclude all internal uses of FFmpeg and MPlayer, and were along the
> lines of:
> "rgb15tobgr15 -file:(mpng|mencoder|vf_.*|vo_.*|utils|vo_vesa|ws|skinload|cs_test|colorspace-test|swscale).c
> -file:lumiere-sections.txt -file:swscale-0.def -file:index.sgml
> -file:rgb2rgb(_template|_1)?.(h|c|sgml)"
> "sws_rgb2rgb_init
> -file:(swscale-example|mga_.*|mpng|mencoder|vf_.*|vo_.*|utils|vo_vesa|ws|skinload|cs_test|colorspace-test|swscale).c
> -file:lumiere-sections.txt -file:swscale-0.def -file:index.sgml
> -file:rgb2rgb(_template|_1)?.(h|c|sgml)"
> 
> There are some programs that at some point used rgb2rgb directly:
> xvidcap has had it commented out forever
> veejay no longer uses it
> avidemux no longer uses it
> xbmc cargo cults initialization of sws_rgb2rgb_init() unnecessarily
> 
> Some other programs are either forks of mplayer (mplayer-ce,
> mplayerxp, syllable, warpvision) or copied some specific code inside
> their libraries (for example crusher has interleaveBytes).
> 
> All other hits for any of those global pointers and sws_rgb2rgb_init()
> are either inside FFmpeg/MPlayer or are in files listing the functions
> from rgb2rgb (like .def or .txt files). rgb2rgb.h is not even an
> installed header. I've sent a patch to mplayer-dev-eng to remove the
> last uses of rgb2rgb. I think it's safe to assume that no one uses
> rgb2rgb anymore.
> 
> Does anyone have anything against dropping the rgb2rgb "external"
> interface (direct access to global function pointers and some other
> functions in rgb2rgb.h). Please speak up now.

lets drop it then
and thanks for the excelent analysis of the situation

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100909/9dad6b93/attachment.pgp>