[FFmpeg-devel] [PATCH] metadata compatibility layer (part2)

Aurelien Jacobs aurel
Fri Jan 9 00:25:03 CET 2009


Michael Niedermayer wrote:

> On Thu, Jan 08, 2009 at 11:11:12PM +0100, Aurelien Jacobs wrote:
> > Michael Niedermayer wrote:
> > 
> > > On Wed, Jan 07, 2009 at 11:25:50PM +0100, Aurelien Jacobs wrote:
> > > > Michael Niedermayer wrote:
> > > > 
> > > > > On Wed, Jan 07, 2009 at 01:21:07AM +0100, Aurelien Jacobs wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > Hi attached patch is the second (and final) part of the metadata
> > > > > > compatibility layer. It adds compatibility in the other way around.
> > > > > > When a demuxer read metadata and stores it with av_metadata_set(),
> > > > > > it automatically fill the old API metadata fields, so that a
> > > > > > software using the old API is still able to get metadata from
> > > > > > demuxers using the new API.
> > > > > > 
> > > > > > It is slightly ugly, especially the AVFormatContext pointer stored
> > > > > > in the AVMetadata struct, but that's the cleanest way I found.
> > > > > > As the struct is not exposed in public API and all of this is
> > > > > > just a temporary transition code, I think it is acceptable.
> > > > > 
> > > > > I think this code is too ugly, i was already very unhappy about the
> > > > > last of these compatibility patches but this one is just too much.
> > > > > I do not want this or anything similar in a file i have to maintain.
> > > > 
> > > > Well, if that's the only problem, I can maintain that file ;-)
> > > > Anyway, this code shouldn't require much maintenance (if any).
> > > > 
> > > > > Id much rather bump the major version several times.
> > > > 
> > > > >From our side, it's obviously much easier to bump major version
> > > > and not care about old API at all.
> > > > But from the side of the libav* users (and distro maintainers,
> > > > etc...) it's much more painful to support such harsh API change.
> > > > For software using lavf it means either:
> > > >  - stay with the old API and be unusable with ffmpeg's svn
> > > >  - upgrade to new API and be unusable with disto's packaged
> > > >    ffmpeg (ie. the majority of users)
> > > >  - add an equivalent compatibility layer in the software itself
> > > >    (or clutter the code with #ifdef), which leads to code
> > > >    duplication in every software using lavf
> > > > And obviously different software will choose different solutions,
> > > > which will lead many people compiling software against FFmpeg
> > > > to problems, and not the same problems depending on the
> > > > software they build.
> > > > This IMO leads to a big mess, and this is the kind of thing
> > > > which tarnish ffmpeg's reputation.
> > > > So IMO, avoiding this is worth a few dozen line of temporary
> > > > code properly under #if.
> > > 
> > > could all this "code" not be put in a seperate file instead of
> > > metadata.c/h ?
> > 
> > Sure ! I moved this code in metadata_compat.c.
> > I also used Ronald's idea (thanks!) to avoid putting
> > AVFormatContext* in AVMetadata. So now the patch contains just
> > very minimal changes to actual code, and the compatibility
> > layer is self contained in metadata_compat.c.
> > After this one is applied I will also move the first compat
> > function to metadata_compat.c so that metadata.c will stay
> > small/simple/beautiful...
> 
> patch ok

Applied.

Aurel




More information about the ffmpeg-devel mailing list