On Tue, 10 May 2016, Michael Niedermayer wrote:

On Tue, May 10, 2016 at 03:53:31PM +0200, wm4 wrote:
On Tue, 10 May 2016 14:35:02 +0200
Michael Niedermayer <mich...@niedermayer.cc> wrote:

On Tue, May 10, 2016 at 02:02:52PM +0200, Hendrik Leppkes wrote:
On Tue, May 10, 2016 at 1:43 PM, Michael Niedermayer
<mich...@niedermayer.cc> wrote:
On Tue, May 10, 2016 at 11:17:12AM +0100, Derek Buitenhuis wrote:
On 5/10/2016 7:24 AM, Hendrik Leppkes wrote:
I don't like this, the struct is pretty cleanly defined and unlikely
to be extended much over time.
Most other structs have AVOptions so the CLI can interact with it, but
this struct is not meant to be modified by users, its just a direct
line of communication between demuxer->decoder or encoder->muxer.

To me this mostly still feels like trying to make a demuxing library
something it isn't. It demuxes and muxes.

You can argue that it should be easier than decoding a frame to get this
info (which I don't personally think is much trouble at all). However,
jamming it into libavformat for reasons like "we've always done this" and
"there's no better place" is not good design IMO.

It sounds like what you really want is a higher level "easy" API. Don't
jam it in to existing decoupled APIs because it is convenient, IMO. That's
how horrible things like having ffmpeg.c functionality in libavformat came
to exist, and a decade of misery followed.

This patch adds an AVClass field to AVCodecParameters like we use in
other public structures.
i fail to see how this patch is related to your reply

What the patch does, is it makes the API more consistent and easy to
use. Users call the AVOption functions to set fields in the main
public structures, if they do that for AVCodecParameters it would
crash.
Even an empty AVClass that doesnt list options would reduce that
misery by at least not crashing but giving a clear error message.


We should not design our API around people not caring to read our API,
because thats not a fight we can ever win.

APIs should be designed with the "Principle of least astonishment"

its quite surprising that AVOption APIs work with most public API
structures but crash with some
(AVPacket is another and i am of the oppinon and stated that previously
 IIRC that it too should have a AVClass as its first element, this is
 just hard to add as it requires ABI bumping everything which is why
 i didnt push much for that being done, it needs to be done at a
 time we bump all stuff for more important reasons)

also let me steal the text from wikipedia about the
Principle_of_least_astonishment, as its IMO well written

Indeed, av_opt calls on structs which don't support it compile without
warning, but mysteriously crash at runtime. That's a violation of this
principle. And no, it doesn't mean that every struct should have an

AVClass, because that would be insane.

iam not seriously suggesting to add it to every public struct but
playing devils advocate here why is that insane ?
it would actually solve the problem and be consistent

I also don't see why adding an AVClass for most public structs is so evil. Apart from the crash issue above, having an av_opt support for struct members is definitely good, only way to have that conveniently at the moment is to use AVClass.

Specifying the class separately from the object for each av_opt call would also be a bit ugly, so even I agree it is not perfect, I don't see a better way, and we already have AVClasses for some structs...

Does libav have any plans for AVClass?

Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to