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

[Hendrik Leppkes]

On Mon, Jun 6, 2016 at 6:29 PM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Mon, Jun 06, 2016 at 06:14:15PM +0200, Hendrik Leppkes wrote:
>> On Mon, Jun 6, 2016 at 6:09 PM, Michael Niedermayer
>> <michael at niedermayer.cc> wrote:
>> > 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?
>>
>> I do, which is why I think it should go away, keeping a duplicated and
>> deprecated part of API around for ever is not a good thing, and
>> codecpar is a much better API to cleanly export information, instead
>> of AVCodecContext which contains hundreds of fields of questionably
>> usefulness.
>> But this decision is still 2 years in the future either way.
>
> really, I agree
> what keeps annoying the hell out of me is that the new API as it is
> currently has piranha filled holes where there where solid and simple
> bridges one could walk over previously
> and when i tried to put a board over a hole someone hit me with a
> flame thrower
>

Thats because we want the holes to be identified and filled up
properly, and not just boarded over, to keep with the analogy.
So to translate that, needs should be properly identified and decided
what to do about them, or if something needs to be done at all (we're
all aware we have a whole bunch of "hack solutions" that may not be
worth repeating), instead of just blindly copying some field into
codecpar (which is part of why avctx got so big in the first place,
codec-specific things that end up in a global context, hence being
reluctant to repeat such mistakes when we have a clean slate here)

- Hendrik