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

Michael Niedermayer michael at niedermayer.cc
Fri Jan 26 17:55:45 EET 2018


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

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180126/7786ec9d/attachment.sig>


More information about the ffmpeg-devel mailing list