[FFmpeg-cvslog] Merge commit '85770f2a2651497861ed938efcd0df3696ff5e45'

[Michael Niedermayer]

On Mon, May 02, 2011 at 12:19:31AM +0200, Stefano Sabatini wrote:
> On date Sunday 2011-05-01 22:08:29 +0200, Michael Niedermayer wrote:
> > On Sun, May 01, 2011 at 01:01:30AM +0200, Stefano Sabatini wrote:
> > > On date Sunday 2011-05-01 00:27:06 +0200, Michael Niedermayer wrote:
> > > > ffmpeg | branch: master | Michael Niedermayer <michaelni at gmx.at> | Sun May  1 00:21:56 2011 +0200| [ffb5a0d533498102c31aa131bc91a4cce868b0a8] | committer: Michael Niedermayer
> > > > 
> > > > Merge commit '85770f2a2651497861ed938efcd0df3696ff5e45'
> > > > 
> > > > * commit '85770f2a2651497861ed938efcd0df3696ff5e45':
> > > >   AVOptions: make default_val a union, as proposed in AVOption2.
> > > >   Move ff_dynarray_add to lavu and make it public.
> > > >   lavf: remove duplicate assignment in avformat_alloc_context.
> > > >   lavf: use designated initializers for AVClasses.
> > > >   options: simplify av_find_opt by using av_next_option.
> > > > 
> > > > Merged-by: Michael Niedermayer <michaelni at gmx.at>
> > > > 
> > > > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=ffb5a0d533498102c31aa131bc91a4cce868b0a8
> > > > ---
> > > 
> > > Why did you remove .i64 and .q from the default_val union?
> > 
> > short awnser: because it didnt work, it broke fate.
> > 
> > more elaborately, i think its a bad idea to have 3 different ways
> > to store a real-scalar value. It leads to problems/confusion and
> > complex code. The problem is quite real (fate failing) and fixing
> > that would lead to complex code with IMHO little solid advantage
> > Its also more error prone as its easy to set the wrong one in the
> > table and theres nothing that would point at that mistake.
> 
> Can you say where the fate problem exactly was?

I dont know exactly, i saw uses of i think dbl where it was not
certain the type actually was double. also the tables might have
contained wrong types. It seemed best to throw the other types out
instead of debuging this further


> 
> > > For the rational it is more safe/logical not assume sizeof(double) >=
> > > sizeof(int64_t), for the second it may be convenient to express a
> > > default by using a rational rather than its double representation, for
> > > documentation/convenience purposes.
> > 
> > for documentation/convenience one can write dbl=5.0/3.0 in the table
> 
> yes, although there is an approximation error but maybe this can be
> safely ignored

yes


> 
> > > Also I suppose we agreed that public API changes should be posted for
> > > review before being pushed.
> > 
> > yes, but this did not seem controversal to me.
> 
> But there was an RFC and someone else was working on that (Anton), in
> this case letting the original contributor do the work (in case he
> wants) is IMO a good norm, in doubt better to choose the safest option
> and publish the patch anyway (eventually telling: i'll push soon if I
> see no objections...).

I dont remember a RFC, But as you mention it now i found it on
libav-devel, well if people want me to read something please post it to
ffmpeg-devel or CC me directly.

I did know anton was planing to work on it the day before from IRC
and i wanted
to implement it before him because he said it needs deep changes in
avoptions and i knew that i could implement it without deep changes.
It was unfortunate that i couldnt finish it before anton started.
And thus some work was duplicated.


> 
> > Adding a string value in addition to double is something that was
> > discussed and something everyone seems to have agreed to be a good idea. 
> 
> > But let me ask differently, what should i change on the code?
> > if nothing then IMHO skiping the patch sending wasnt that wrong
> > and if theres something, please tell me, ill fix it.
> 
> I have a bad feeling about the int64/double issue, it could affect
> portability (but I need to check better the code), also I'd like to
> fix default string values setting.
> 
> WIP patches attached (just as a reference, the second one can be
> possibly simplified, and not tested), and I'm not sure I like the
> av_null_string approach.

I think simply storing NULL when NULL is meant is simpler

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

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-cvslog/attachments/20110502/19c78a32/attachment.asc>