[FFmpeg-devel] [PATCH 1/2] avformat/utils: Export coded dimensions unconditionally

[Michael Niedermayer]

On Mon, Jun 06, 2016 at 09:01:44AM +0200, Hendrik Leppkes wrote:
> On Sun, Jun 5, 2016 at 5:13 AM, Michael Niedermayer
> <michael at niedermayer.cc> wrote:
> > This fixes a API regression
> > Probably fixes Ticket5451
> >
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavformat/utils.c   |    4 ++--
> >  libavformat/version.h |    2 +-
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > index ac056c4..7ca0a12 100644
> > --- a/libavformat/utils.c
> > +++ b/libavformat/utils.c
> > @@ -3789,8 +3789,6 @@ FF_DISABLE_DEPRECATION_WARNINGS
> >              av_codec_set_lowres(st->codec, av_codec_get_lowres(st->internal->avctx));
> >              st->codec->width = st->internal->avctx->width;
> >              st->codec->height = st->internal->avctx->height;
> > -            st->codec->coded_width = st->internal->avctx->coded_width;
> > -            st->codec->coded_height = st->internal->avctx->coded_height;
> >          }
> >
> >          if (st->codec->codec_tag != MKTAG('t','m','c','d'))
> > @@ -3807,6 +3805,8 @@ FF_DISABLE_DEPRECATION_WARNINGS
> >          }
> >
> >          // Fields unavailable in AVCodecParameters
> > +        st->codec->coded_width = st->internal->avctx->coded_width;
> > +        st->codec->coded_height = st->internal->avctx->coded_height;
> >          st->codec->properties = st->internal->avctx->properties;
> >  FF_ENABLE_DEPRECATION_WARNINGS
> >  #endif
> > diff --git a/libavformat/version.h b/libavformat/version.h
> > index c92a23f..2f79b7f 100644
> > --- a/libavformat/version.h
> > +++ b/libavformat/version.h
> > @@ -29,7 +29,7 @@
> >
> >  #include "libavutil/version.h"
> >
> > -// When bumping major check Ticket5467, 5421 for regressing
> > +// When bumping major check Ticket5467, 5421, 5451 for regressing
> >  // Also please add any ticket numbers that you belive might regress here
> >  #define LIBAVFORMAT_VERSION_MAJOR  57
> >  #define LIBAVFORMAT_VERSION_MINOR  37
> > --
> > 1.7.9.5
> 
> The ticket in question accesses st->codec to get this field. st->codec
> is going away, so there is no point in testing this "regression",
> since they need to update their API usage first for it to even
> compile.

IMO its neccessary to first discuss the removial of st->codec on
ffmpeg-devel before it is removed, thats a major change and should
not just happen without discussion and some sort of more or less
consensus. Or dont you want to be in "control" over the project you
work on and maintain?

Also the ticket list is meant along the lines of "This could break,
we must be conciously aware of the consequences of our actions so
we can make informed decissions"

by the time we want to bump API, Chromium might have been changed
and no longer require the coded_height/width fields
or it might still need them and a trivial update in Chromium might
fix that or it might be that it depends on these fields and an
update to the codecpar API if its lacking the field might cause a
major problem for Chromium.
We must be aware of what removing the st->codec API does to our users
so we can make a wise and informed decission.


> Heck the same thing applies to the other tickets you listed here, all
> of those access information from st->codec which is going away
> entirely, so you can't really "test" these cases. Someone would have
> to update the code to use codecpar first, and while doing this
> migration they would need to preserve whatever behavior exists today.

the first listed ticket #5467 is about ffmpeg displaying information
about the input streams losslessness
That ticket has nothing to do with codecpar as such
Either ffmpeg does display this information or it does not, it doesnt
need it from codecpar, it could obtain the information by other means

The 2nd listed ticket (#5421) of the 3 is similar about encoder and qp
range display.

By the time we want to bump, ffmpeg might not use st->codec anymore to
display these and the tickets then wont regress or it might still use
it and regress. This is important information when we want to bump the
version ...

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160606/b599faf2/attachment.sig>