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

Michael Niedermayer michaelni at gmx.at
Thu Nov 13 01:42:05 CET 2014


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 dont see any need for a callback but its quite possible i
> >misundertood something
> 
> I hope I explained.

yes, it seems we are thinking of somewhat different cases


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

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141113/0c888864/attachment.asc>


More information about the ffmpeg-devel mailing list