[FFmpeg-devel] [PATCH] avcodec: Add AVClass to AVCodecParameters

[Ronald S. Bultje]

Hi,

On Tue, May 17, 2016 at 2:29 PM, Hendrik Leppkes <h.leppkes at gmail.com>
wrote:

> On Tue, May 17, 2016 at 7:46 PM, Ronald S. Bultje <rsbultje at gmail.com>
> wrote:
> > Hi,
> >
> > On Tue, May 17, 2016 at 1:24 PM, Hendrik Leppkes <h.leppkes at gmail.com>
> > wrote:
> >
> >> On Tue, May 17, 2016 at 7:22 PM, Ronald S. Bultje <rsbultje at gmail.com>
> >> wrote:
> >> > Hi,
> >> >
> >> > On Tue, May 17, 2016 at 12:34 PM, Hendrik Leppkes <
> h.leppkes at gmail.com>
> >> > wrote:
> >> >
> >> >> On Tue, May 17, 2016 at 6:26 PM, Ronald S. Bultje <
> rsbultje at gmail.com>
> >> >> wrote:
> >> >> > Hi,
> >> >> >
> >> >> > On Tue, May 17, 2016 at 11:18 AM, Michael Niedermayer <
> >> >> > michael at niedermayer.cc> wrote:
> >> >> >
> >> >> >> AVCodecContext contains all the fields that AVCodecParameters has.
> >> >> >> Examples are width, height, bitrate, profile, sample_rate,
> channels
> >> >> >>
> >> >> >> In AVCodecContext these fields are accessible through AVOptions
> >> >> >> in AVCodecParameters these fields are not accessible through
> >> AVOptions
> >> >> >> and your code will crash mysteriously if you try to access them
> that
> >> >> >> way
> >> >> >>
> >> >> >> This is a inconsistency that was not there before
> >> >> >
> >> >> >
> >> >> > So, at the risk of going semi-offtopic, let me take a different
> angle
> >> at
> >> >> > this problem.
> >> >> >
> >> >> > AVCodecParameter is supposed to be an intermediate layer that
> removes
> >> the
> >> >> > need for lavf to access lavc structs like AVCodecContext, right?
> >> >> >
> >> >> > So, I worked (a long long time ago) on this software called
> GStreamer,
> >> >> and
> >> >> > specifically, one of the plugins I worked on early on was
> gst-ffmpeg.
> >> I
> >> >> > loved working on it because it practically added so many codecs to
> the
> >> >> > player which were previously unsupported. Yay! Now, the API back
> then
> >> was
> >> >> > horrible. You had an AVCodecContext (no AVOptions) and there were
> >> >> > approximately 100 million fields that could be set. Most were
> >> >> undocumented,
> >> >> > and it was totally unclear which needed to be set. For example, to
> >> play a
> >> >> > WMA file, you might have to set block_align, but for other audio
> >> codecs,
> >> >> > you do not [1].
> >> >> >
> >> >> > For video, sometimes you would have to set extradata (e.g. for
> SVQ3),
> >> but
> >> >> > other times, you would not. Huffyuv required bits_per_sample, but
> h264
> >> >> did
> >> >> > not.
> >> >> >
> >> >> > So how do you write generic user-facing code where the demuxer and
> >> >> decoder
> >> >> > actually are separate entities and merely interact over a
> serialized
> >> >> > communication channel? You can't serialize AVCodecContext. I mean,
> you
> >> >> > could, but that's a terrible idea. And nothing documented which
> fields
> >> >> were
> >> >> > required to be set and which weren't.
> >> >> >
> >> >> > I was hoping that AVCodecParameters would solve this. Ideally, it
> >> would
> >> >> be
> >> >> > a dictionary of fields that are to be shared between demuxer and
> >> decoder
> >> >> > (or codec and muxer). But I think it has sort of strayed off of
> that
> >> >> intent
> >> >> > and is just a flat minimized AVCodecContext as it existed in 2003.
> >> >> >
> >> >> > Can we fix this? I think AVOptions would be ideally suited to fix
> >> this.
> >> >> > AVClass maybe helps also. But if introspection suggests I can set
> >> >> > block_align on the h264 decoder, or width on an audio decoder, or
> who
> >> >> knows
> >> >> > what. Then I don't think AVOptions or AVClass in AVCodecParameters
> >> helps
> >> >> at
> >> >> > all. It just creates a minimized AVCodecContext from 2003 that will
> >> >> slowly
> >> >> > become the current AVCodecContext, which is not all that useful for
> >> >> people
> >> >> > that don't use ffmpeg.exe...
> >> >> >
> >> >> >
> >> >>
> >> >> This is C, I don't want to have to run introspection on an object to
> >> >> find out what I can set.
> >> >> A clean and doumented struct is a far more effective way to get this
> >> >> information across, which is also why I don't want AVOptions for such
> >> >> a struct, if its not needed to set private options, it would just
> >> >> double the need for docs (both in doxy and in the AVOption
> >> >> definition), and to figure out which fields actually exist, you still
> >> >> have to read the struct or a comment in the header file - so why not
> >> >> use the struct in the first place?
> >> >>
> >> >> Why use something "generic" with a dict or some weirdness, which
> loses
> >> >> the strong typing and self-documentation of such a struct? Sounds
> like
> >> >> you want to be working in another language, honestly. :)
> >> >> AVOption will never "know" which fields a particular codec will need,
> >> >> so that point is not going to get solved either way.
> >> >
> >> >
> >> > It's not for us (ffmpeg.c developers), it's for people using the
> ffmpeg
> >> > libraries on other language platforms (e.g. python).
> >>
> >> Maybe the ports to those other languages should devise an interface
> >> appropriate for that language, and we should keep a clean C interface?
> >
> >
> > I suppose. Do we document which fields need to be set without trial and
> > error or manually inspecting the decoder source code?
> >
>
> The fields are documented as video and audio appropriately, but
> otherwise since this information depends on every single decoder,
> there is no way to really document this properly.
> Its a much shorter list than AVCodecContext used to have, so its much
> easier to figure out and fill.
>
> Afterall, if you want to go down on a level this low and fill this
> struct manually, we do expect you do know a tiny bit about codecs, so
> filling any of these fields should not be a huge problem.
> If you just use avformat->avcodec, you do not need to concern yourself
> with the contents of this struct.
>
> We should of course always strife to improve the documentation of our
> public interface to outline which fields are unlikely to be required
> to be set (only a small subset of codecs, like raw codecs often
> needing more than compressed ones), or never required to be set, and
> just used to export a bit of extra detail from avformat.


But isn't this knowledge critical if we want to call avformat and avcodec
"separated"?

We currently just replace AVCodecContext with AVCodecParameters, which is
clearly an improvement, but is really still suffering from the same
fundamental problem, just at a smaller scale.

Ronald