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

Cyril Russo stage.nexvision
Fri Nov 5 18:36:10 CET 2010



>>> The context is opaque, the options can only be set using the
>>> sws_setColorspaceDetails() or the libavutil/opt.h API.
>>>
>> Yes the context is opaque so you can't change the struct's members.
>> How do you set, for example, the source width, or the destination width,
>> or the destination height with sws_setColorspaceDetails, or even
>> sws_scale ?
>>
>> So you're back to use alternative means (like av_set_ et al), meaning
>> you have to *guess* what are the members of the context, and it's not
>> clean at all.
> You could try to read the source
Yes. Hopefully, else the code would directly go to the trashcan, because 
it'd be unusable.
Since the struct is opaque, you have to dig inside the implementation 
files however, which I criticize here.

> Every AVOption struct (Like swscontext) has a class field that contains an
> AVClass and that has a option field that contains a AVOption array and in that
> are all options with type, help text, name and max/min/default.
> Its a lot more convenient than func6() because a GUI can list all newly added
> options with that array but not by adding a new func91()
It's not something that I'd call easy. It might be convenient for an 
automated interpretor, but, frankly, who is going to :
1) Enumerate all the field of the "opaque" structure, programmatically, 
in order to get a vague description
2) Figure out which "member" to change / set, trial and error here
3) Call sws_scale, and it'll appear to work even if you've forgot to set 
the srcWidth or destHeight member correctly, but will produce dumb 
picture on output.
With the getContext *helper*, well, it might not help FFmpeg devs who 
knows how to use the framework, but it helps the library user who 
doesn't want to bother with such low level details.

What I'm saying here is that this change *moves* the burden of 
SwsContext internals into the user code, and this is subject to bad 
quality implementation in user code they'll copy & paste some old 
example code found by googling up. So when you'll update the API, the 
code will still *appear* to work, but it subtly fail.

Having a funcX() method, at least, prevent such case, since the burden 
is inside FFmpeg's core.
I agree that from your (FFmpeg maintainer's) point of view, it's easier 
to externalize the maintenance, but I don't really think it helps user 
in that particular case.

In that case, what about a variadic function, like:
sws_setup_context_base(const char* name, ...);
And the implementation:
sws_get_context(source width, height, dest width & height, colorspace 
stuff, flags) { sws_setup_context_base("sourceWidth", 234, 
"sourceHeight", 345, NULL); }

That way, if you change the SwsContext format, the user front end's 
"sws_get_context" stay the same.

Best regards,
Cyril




More information about the ffmpeg-devel mailing list