[FFmpeg-devel] [PATCH 3/9] lavf/ffm: store/restore private codec context

[Lukasz Marek]

On 13 November 2014 01:42, Michael Niedermayer <michaelni at gmx.at> wrote:

> On Thu, Nov 13, 2014 at 01:04:28AM +0100, Lukasz Marek wrote:
> > On 12.11.2014 20:32, Michael Niedermayer wrote:
> > >On Wed, Nov 12, 2014 at 06:31:52PM +0100, Lukasz Marek wrote:
> > >>On 12 November 2014 17:53, Michael Niedermayer <michaelni at gmx.at>
> wrote:
> > >>
> > >>>On Wed, Nov 12, 2014 at 05:47:01PM +0100, Stefano Sabatini wrote:
> > >>>>On date Wednesday 2014-11-12 09:09:43 +0100, Nicolas George encoded:
> > >>>>>Le primidi 21 brumaire, an CCXXIII, Lukasz Marek a écrit :
> > >>>>>>+static int ffm_write_header_codec_private_ctx(AVIOContext *pb,
> void
> > >>>*priv_data, int type)
> > >>>>>>+{
> > >>>>>>+    AVIOContext *tmp;
> > >>>>>>+    char *buf = NULL;
> > >>>>>>+
> > >>>>>>+    if (priv_data) {
> > >>>>>>+        if (avio_open_dyn_buf(&tmp) < 0)
> > >>>>>>+            return AVERROR(ENOMEM);
> > >>>>>
> > >>>>>>+        av_opt_serialize(priv_data, AV_OPT_FLAG_ENCODING_PARAM |
> > >>>type, 1, &buf, '=', ',');
> > >>>>>
> > >>>    ^
> > >>>>
> > >>>>>Unless I am mistaken, this is skip_default. What happens if the
> > >>>instance
> > >>>>>that reads the file does not have the same defaults as the instance
> > >>>that has
> > >>>>>written it? For example, what happens if ffmpeg feeding ffserver is
> > >>>not the
> > >>>>>same version as ffserver?
> > >>>>
> > >>>>We should specify an FFM version, and abort in case it's not
> > >>>>compatible.
> > >>>
> > >>
> > >>I'm not sure how it should help. Stupid example:
> > >>codec has on option to encode picture upside down
> > >>In early version  0 - disable 1 - enable feature
> > >>In later version it has been changed 1 - disable, 0 - enable.
> > >>
> > >>version of ffm has nothing to do with it.
> > >>
> > >
> > >>The problem is that the codec context is used to transport the
> > >>>values into the muxer instead of some form of key/value list, string
> > >>>or AVDictionary.
> > >>>
> > >>
> > >>Yes. A solution would be to provide a callback implemented by codec.
> > >>So codec gets a dictionary of properties and version of libav libraries
> > >>that produced it and codec applies them itself.
> > >>If not implemented then current approach is a fallback without worrying
> > >>about compatibility.
> > >>This would allow just to fix some incompatibilities in case they occur.
> > >>Of course to make it working all options should be sent, not only
> > >>non-defaults.
> > >
> > >iam not sure i understand what you have in mind or if i misunderstand
> > >but
> > >What i was thinking of is that the options from a configuration file
> > >or command line would be provided to the ffm muxer in the form of a
> > >string or AVDictionary by the user application (ffserver for example)
> > >and the connecting ffmpeg would receive this string or AVDictionary
> > >from the ffm demuxer and use it to configure the encoder by
> > >passing it into avcodec_open(), defaults would be from the side
> > >that actually runs the encoder, the other side would have no
> > >impact on the defaults it would just transfer the user parameters
> >
> > I think we are talking about different aspects.
> >
> > Regarding your thoughts, isn't it working (more or less) this way
> > already? I know that options are not passed to avcodec_open, but
> > doesn't it have the same (or at least similar) flow and effect
> > eventually?
> >
> > My point (and probably Nicolas') was that options may differ
> > betweens versions of the encoder.
> >
> > for example you have ffserver with codecs, formats etc from version 2.2.
> > Lets say ffserver config file has something like that (not real
> > params in example):
> > VideoCodec somecodec
> > AVOptionVideo brightness 50
> >
> > So, in this example, "somecodec" have option brightness with range
> > 0-100, you set 50 in this example. I skip defaults here, but lets
> > say it is 30.
> >
> > Now, some one connects to this server from the end of the world with
> > ffmpeg version 2.4. codec has changed and brightness has now range
> > 0-200 and default 60.
> > So ffmpeg downloads config from server (no matter in what form)
> > and applies this 50 value that is downloaded. And now the meaning of
> > this 50 is different in newer version.
> >
> > So my idea was to provide a optional callback, codec specific, that
> > could adjust that value to 100. codec would know verion of libraries
> > that provided that value.
>
> if a field with the same name has a different meaning thats a break
> of the interface. It would not just break the config file but also
> ffmpegs command line syntax vs. old scripts.
> I dont think theres any point in trying to support that
> change of default or extending the range of a value might happen but
> changing the meaning of something should not happen or be very very
> rare
>
>
> >
> > You can imagine similar example for defaults probably.
>
> yes, but the config file used simply must be compatible to the
> encoder used. It should not depend on the defaults of intermediate
> components. That is if the config file specifies 50 for brigtness
> thats what the encoder should be set to independant of an intermediate
> lib or app having 50 as default.
> I dont think this is important to be fixed before applying the
> patchset, but i think it should be fixed at some point in the future
> and using a string or key/value list seems a simple way to avoid
> the problem of "mystruct.field == 50 .. hmm but 50 is default, did
> the user set that or not"
>

I understand now.
It should not be hard to do it on top of this patchset.
During the weeked I will make first proposal probably.