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

[James Almer]

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.

The alternative is to remove the two print_int() lines right now, of
course, but i don't think it's the best approach when the above patch is
simple, clean, and local to ffprobe.c (unlike the original suggestion).