On Mon, Aug 17, 2015 at 5:30 PM, Ronald S. Bultje <rsbul...@gmail.com> wrote:
> Hi,
>
> On Mon, Aug 17, 2015 at 9:20 AM, Michael Niedermayer <mich...@niedermayer.cc
>> wrote:
>
>> On Mon, Aug 17, 2015 at 07:48:07AM -0400, Ronald S. Bultje wrote:
>> > Hi,
>> >
>> > On Mon, Aug 17, 2015 at 6:40 AM, Michael Niedermayer
>> <mich...@niedermayer.cc
>> > > wrote:
>> >
>> > > On Sun, Aug 16, 2015 at 05:45:55PM -0400, Ronald S. Bultje wrote:
>> > > > Hi,
>> > > >
>> > > > On Sun, Aug 16, 2015 at 5:24 PM, Andreas Cadhalpun <
>> > > > andreas.cadhal...@googlemail.com> wrote:
>> > > >
>> > > > > On 16.08.2015 22:15, Ronald S. Bultje wrote:
>> > > > > > Convert last users to av_opt_get_*() counterparts.
>> > > > > > ---
>> > > > > > libavfilter/af_aresample.c | 17 +++++++++--------
>> > > > > > libavutil/opt.h | 3 +++
>> > > > > > 2 files changed, 12 insertions(+), 8 deletions(-)
>> > > > >
>> > > > > I'm fine with this, but the patch is incomplete:
>> > > > > libavfilter/x86/vf_spp.c also uses av_get_int.
>> > > > > Furthermore the FF_OPT_TYPE_* defines are used in several places.
>> > > >
>> > > >
>> > > > Yes I noticed the last one in vf_spp.c, I missed it (don't know how),
>> > > have
>> > > > it locally amended. I'm working on FF_OPT_TYPE_* separately (I'm
>> doing
>> > > one
>> > > > deprecation macro at a time).
>> > > >
>> > >
>> > > > Some really are a mess, Michael can you comment on how on earth you
>> could
>> > > > consider adding av_stream_get_end_pts as a replacement for AVFrac
>> without
>> > > > deeply frowning at yourself? Can you take some time and fix that
>> > > correctly?
>> > >
>> > > AVFrac is used to exactly compute timestamps
>> >
>> >
>> > I know what it does.
>> >
>> > I'm talking about a developer's decision to take a deprecated field,
>> leave
>> > it actively deprecated and about to be removed and then add a public that
>> > uses exactly this API. How could you do that? How does that in any way
>> help
>> > us deprecate the struct or field? AVStream.pts is still marked as
>> > deprecated and isn't even hidden from the public API in your supposedly
>> > "fixed" implementation. Its API is still exposed in the place where you
>> put
>> > the data. It still doesn't compile after the version bump. In other
>> words,
>> > your patch didn't solve anything.
>> >
>>
>> > Can you please fix it properly? Properly means that after bump, it
>> compiles
>> > and passes fate.
>>
>> > I'd almost suggest we set up a fate station that tests
>> > that for the next version bump.
>>
>> no objection but i think first the code should pass fate after a bump
>> before such a fate machine is added
>>
>>
>> > If we intend to hide AVFrac from the user,
>> > this probably means renaming AVFrac to just Frac, and moving it into some
>> > internal-only data structure not exposed to the user, where it's
>> accessible
>> > for av_stream_get_end_pts to use.
>>
>> the AVFrac pts field is in AVStream
>> internal AVStream fields are simply after the comment:
>> /*****************************************************************
>> * All fields below this line are not part of the public API. They
>> * may not be used outside of libavformat and can be changed and
>> * removed at will.
>> * New public fields should be added right above.
>> *****************************************************************
>> */
>>
>> I can move the field after that with appropriate #if, if that
>> resolves the issue
>
>
> How do we enforce this btw? Isn't it much nicer (since we allocate AVStream
> inside lavf anyway) if we hide all the items and make them
> internally-visible only in a separate struct (typedef struct AVStreamReal {
> AVStream parent; [ all other members ] } AVStreamReal;)?
>
> (I bet that mplayer or some other half-assed project uses one of these
> members and will complain.)
>
We moved internal things into an opaque sub-struct in other contexts,
so doing it for AVStream is probably a logical next step.
However the usualy way to do this is using AVStreamInternal* at the
end of the AVStream, not using a different internal struct definition
like that.
PS:
Complaints about things explicitly marked "internal" going away should
be flat out ignored.
- Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
