[FFmpeg-devel] Meaningful return codes (was [PATCH 7/9] Replace AVERROR_NOTSUPP by AVERROR(ENOSYS))

[Michael Niedermayer]

Hi

On Fri, Jul 20, 2007 at 11:05:12AM +0200, Panagiotis Issaris wrote:
> Hi
> 
> Op 19-jul-07, om 22:28 heeft Michael Niedermayer het volgende  
> geschreven:
> 
> > Hi
> >
> > On Thu, Jul 19, 2007 at 07:22:00PM +0200, Panagiotis Issaris wrote:
> >> Hi
> >>
> >> [...]
> >> I see. Thanks for the information!
> >>
> >> Related to this: As I've mentioned before on this list, one of the
> >> things
> >> I'd like to change in libavcodec is the handling of errors. I'd like
> >> returncodes indicating failure to carry information about the cause
> >> of the
> >> failure. So, for example, in the case of mpegvideo_enc.c, I'd  
> >> prefer to
> >> see MPV_encode_init() return different errorcodes for different  
> >> kinds of
> >> failures occurring instead of the currently used "return -1;" for all
> >> errors.
> >>
> >> I'm not sure about the granularity though...
> >>
> >> Would something like this be nice & useful?
> >>
> >> #define AVERROR_PIXFMT_NOTSUP       <somevalue>
> >> #define AVERROR_BFRAMES_NOTSUP      <somevalue+1>
> >> #define AVERROR_RC_PARSE_ERROR      <somevalue+2>
> >> ...
> >
> > whats the sense of this?
> > an application which has code to parse these and do some appropriate
> > action beyond printing an error message can as well just contain code
> > to set the values properly in the first place
> >
> 
> No, I think it's different. For example, it could be that the values  
> are not
> set by the application but by an end-user. Ofcourse the app should parse
> the values for legality in some sense, but lets say your writing a GUI
> for transcoding files. The end-user tries to transcode a DVD to  
> MPEG-4 but
> then tries to use the H.261 codec instead. Then you can either just show
> an error-message explaining that only CIF and QCIF are supported or _for
> some applications_ it could be useful to switch to CIF automatically.  
> I'm
> pretty sure most of us here would prefer that _not_ to happen, but  
> for some
> end-users, this could be the prefered way of doing things.
> 
> When you're returning "-1" in all possible cases something is going  
> wrong,
> an application can not choose how to handle such situations; it can  
> only display
> the error message.

well and if you get a return saying invalid_size like you suggest above
your application does what? try every width x height bruteforce?
now if codecs contain a list of valid sizes, so can the application
just check the user param against that and select a appropriate one
the error code doesnt simplify that


> 
> Ofcourse, the application could also contain all the logic to do these
> checks by itself, but imho that would be unnecessary code duplication  
> as that
> exact logic is already available in the library.

the exact is NOT available in the lib, lavc will generally fail randomly
if some nonsense combinations are used
you are totally going into the wrong direction here, the error codes
would be a true nightmare for an application to do anything with
its the most stupid idea ive seen here since a long time
just look at how many parameters we have, having an error code for each
is total sick, no application will contain code to do anything with that
monster list of codes

what makes sense is a generic solution, that is one which tells you
per indexes into an AVOption array which combination of fields are
causing a problem and then somehow return a list describing alternative
minimal changes needed to avoid the problem
like

if someone tried mpeg4 + 4mv + ilme + simple_profile + 2_b_frames
{flags, 4mv, 0}, {profile, main}
{flags, ilme, 0}, {profile, main}
{flags, ilme, 0}, {b_frames, 0}

now tell me how do you want to describe these 3 alternative solutions
with an error code


> 
> A good example would be the rc_eq parsing code. If libavcodec would  
> return
> meaningful return codes, it would be easy to write an app providing  
> the posibility
> to alter it. If not, it would have to duplicate a lot of the code  
> which is already
> in libavcodec.

it can easily be shown that its impossible to check rc_eq for sanity
and a syntactically wrong rc_eq (which is the less likely case)
should not silently be overridden

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

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070720/272633ae/attachment.pgp>