[FFmpeg-devel] [RFC] Reintroduce sws_getContext() as sws_create_context()

Cyril Russo stage.nexvision
Thu Nov 4 17:07:16 CET 2010


Hi,

   I'm a FFmpeg library user. Recently, you've deprecated sws_getContext 
telling to use sws_alloc_context & sws_init_context instead.
This looks like a bad choice to me, as they are not equivalent.

First, sws_getContext had these avantages:
1) Library's user didn't need to know what the SwsContext content was.
2) All in one function, with no possible error in handling the return code.

And these cons: (so far as I can understand)
1) not the same naming convention as the other libraries in FFmpeg project
2) API is fixed.

In my opinion, new code have these defects:
1) sws_init_context meaning isn't clear. What is it used for ? Why is 
there no equivalent to this init function in other libraries ?
2) SwsContext isn't described in swscale.h file, but it's required to 
set its member if you want to actually use the context.
3) Since SwsContext structure can change, the user code (mine) could 
break if you rename/remove a member of the struct.
4) sws_getCachedContext is still here but not deprecated, and the 
documentation refer to sws_getContext.
5) I finally have to duplicate the getContext code in my code, and 
handle all the possible errors myself.

For me, I would prefer the "blackbox" idea, and having a function that 
does the dirty work of filling the opaque structure member correctly, 
and not having to deal with it.
So, for symmetry with the other libavX lib, maybe having a new function 
called:
sws_create_context(getContext_args_here) would bring back all the 
required features.
Usage would then be:
SwsContext * context = sws_alloc_context();
if (sws_create_context(context, 234, 243, ...etc...) != 0) { 
perror("error creating the context"); return -1; }

Instead of:
SwsContext * c = sws_alloc_context();
if (c == NULL) { perror("error allocating the context"); return -1; }
     c->flags= flags;
     c->srcW= srcW;
     c->srcH= srcH;
     c->dstW= dstW;
     c->dstH= dstH;
     c->srcRange = handle_jpeg(&srcFormat);
     c->dstRange = handle_jpeg(&dstFormat);
     c->srcFormat= srcFormat;
     c->dstFormat= dstFormat;

     if (param) {
         c->param[0] = param[0];
         c->param[1] = param[1];
     }
     sws_setColorspaceDetails(c, ff_yuv2rgb_coeffs[SWS_CS_DEFAULT], 
c->srcRange, ff_yuv2rgb_coeffs[SWS_CS_DEFAULT] /* FIXME*/, c->dstRange, 
0, 1<<16, 1<<16);
     if(sws_init_context(c, srcFilter, dstFilter) < 0) {
         sws_freeContext(c);
         perror("error initializing the context");
         return -1;
     }

Let me know if you think it's a good idea, before I try to write a patch.
Best regards,
Cyril




More information about the ffmpeg-devel mailing list