[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