[FFmpeg-devel] [PATCH 1/2] lavc: Add coded_w/h to AVCodecParameters

[Li, Zhong]

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of Li, Zhong
> Sent: Tuesday, January 30, 2018 10:06 AM
> To: FFmpeg development discussions and patches
> <ffmpeg-devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] lavc: Add coded_w/h to
> AVCodecParameters
> 
> > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On
> Behalf
> > Of Michael Niedermayer
> > Sent: Friday, January 26, 2018 11:56 PM
> > To: FFmpeg development discussions and patches
> > <ffmpeg-devel at ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH 1/2] lavc: Add coded_w/h to
> > AVCodecParameters
> >
> > On Fri, Jan 19, 2018 at 01:15:13PM -0300, James Almer wrote:
> > > On 1/19/2018 7:12 AM, Hendrik Leppkes wrote:
> > > > On Fri, Jan 19, 2018 at 4:19 AM, Li, Zhong <zhong.li at intel.com> wrote:
> > > >>> -----Original Message-----
> > > >>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On
> > > >>> Behalf Of James Almer
> > > >>> Sent: Thursday, January 18, 2018 1:15 PM
> > > >>> To: ffmpeg-devel at ffmpeg.org
> > > >>> Subject: Re: [FFmpeg-devel] [PATCH 1/2] lavc: Add coded_w/h to
> > > >>> AVCodecParameters
> > > >>>
> > > >>> On 1/18/2018 2:03 AM, Zhong Li wrote:
> > > >>>> coded_width/height may be different from width/height sometimes
> > > >>>
> > > >>>> (e.g, crop or lowres cases).
> > > >>>
> > > >>> Which is why it's not a field that belongs to AVCodecParameters.
> > > >>>
> > > >>> Codec level cropping has nothing to do with containers. Same
> > > >>> with lowres, which is an internal feature, and scheduled for removal.
> > > >>
> > > >> Got it. How about fixing ticket #6958 as below?
> > > >>
> > > >> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c index
> > > >> 0e7a771..233760d 100644
> > > >> --- a/fftools/ffprobe.c
> > > >> +++ b/fftools/ffprobe.c
> > > >> @@ -2512,10 +2512,12 @@ static int show_stream(WriterContext
> *w,
> > AVFormatContext *fmt_ctx, int stream_id
> > > >>      case AVMEDIA_TYPE_VIDEO:
> > > >>          print_int("width",        par->width);
> > > >>          print_int("height",       par->height);
> > > >> +#if FF_API_LAVF_AVCTX
> > > >>          if (dec_ctx) {
> > > >>              print_int("coded_width",  dec_ctx->coded_width);
> > > >>              print_int("coded_height", dec_ctx->coded_height);
> > > >>          }
> > > >> +#endif
> > > >>          print_int("has_b_frames", par->video_delay);
> > > >>          sar = av_guess_sample_aspect_ratio(fmt_ctx, stream,
> > NULL);
> > > >>          if (sar.den) {
> > > >> @@ -2912,6 +2914,10 @@ static int open_input_file(InputFile
> > > >> *ifile, const char *filename)
> > > >>
> > > >>              ist->dec_ctx->pkt_timebase = stream->time_base;
> > > >>              ist->dec_ctx->framerate = stream->avg_frame_rate;
> > > >> +#if FF_API_LAVF_AVCTX
> > > >> +            ist->dec_ctx->coded_width =
> > stream->codec->coded_width;
> > > >> +            ist->dec_ctx->coded_height =
> > > >> +stream->codec->coded_height; #endif
> > > >>
> > > >
> > > > As mentioned in the thread for that patch already, writing new
> > > > code using deprecated API should really be avoided.
> > > >
> > > > The way I see it, if someone really needs to know coded w/h (which
> > > > is typically an internal technical detail of no relevance to
> > > > users), they should decode a frame and get it from the decoder.
> > > >
> > > > - Hendrik
> > >
> > > This specific approach is IMO acceptable. It basically recovers the
> > > pre-codecpar behavior until AVStream->codec is removed, and
> > > effectively fixes the "regression".
> > > Once that's gone, ffprobe will stop reporting coded_width/height
> > > altogether instead of doing so with a bogus value, as everything is
> > > being wrapped in the proper checks.
> >
> > +1
> >
> > [...]
> > --
> > Michael     GnuPG fingerprint:
> > 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Thanks for Hendrik/James/ Michael's review.
> Will this patch be applied? (Another thread maybe easier to apply:
> https://patchwork.ffmpeg.org/patch/7344/ )

Ping.
I see the priority of ticket #6958 has been set as "important".