| Message ID | 1564461924-4772-1-git-send-email-linjie.fu@intel.com |
|---|---|
| State | New |
| Headers |
Return-Path: <ffmpeg-devel-bounces@ffmpeg.org> X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id E8D42449C4F for <patchwork@ffaux-bg.ffmpeg.org>; Tue, 30 Jul 2019 07:46:09 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C017268017D; Tue, 30 Jul 2019 07:46:09 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 37096680D20 for <ffmpeg-devel@ffmpeg.org>; Tue, 30 Jul 2019 07:46:01 +0300 (EEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 29 Jul 2019 21:45:59 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,325,1559545200"; d="scan'208";a="179617212" Received: from icl-dev.sh.intel.com ([10.239.158.73]) by FMSMGA003.fm.intel.com with ESMTP; 29 Jul 2019 21:45:58 -0700 From: Linjie Fu <linjie.fu@intel.com> To: ffmpeg-devel@ffmpeg.org Date: Tue, 30 Jul 2019 12:45:24 +0800 Message-Id: <1564461924-4772-1-git-send-email-linjie.fu@intel.com> X-Mailer: git-send-email 2.7.4 Subject: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add AV_CODEC_CAP_VARIABLE_DIMENSIONS flag X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches <ffmpeg-devel.ffmpeg.org> List-Unsubscribe: <http://ffmpeg.org/mailman/options/ffmpeg-devel>, <mailto:ffmpeg-devel-request@ffmpeg.org?subject=unsubscribe> List-Archive: <http://ffmpeg.org/pipermail/ffmpeg-devel/> List-Post: <mailto:ffmpeg-devel@ffmpeg.org> List-Help: <mailto:ffmpeg-devel-request@ffmpeg.org?subject=help> List-Subscribe: <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>, <mailto:ffmpeg-devel-request@ffmpeg.org?subject=subscribe> Reply-To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> Cc: Linjie Fu <linjie.fu@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org> |
Commit Message
Fu, Linjie
July 30, 2019, 4:45 a.m. UTC
Add AV_CODEC_CAP_VARIABLE_DIMENSIONS flag to indicate whether encoder
supports variable dimension encoding.
Signed-off-by: Linjie Fu <linjie.fu@intel.com>
---
[v2]: update API changes.
doc/APIchanges | 3 +++
fftools/cmdutils.c | 2 ++
libavcodec/avcodec.h | 5 +++++
libavcodec/rawenc.c | 1 +
libavcodec/version.h | 2 +-
5 files changed, 12 insertions(+), 1 deletion(-)
Comments
Am Di., 30. Juli 2019 um 06:46 Uhr schrieb Linjie Fu <linjie.fu@intel.com>: > > Add AV_CODEC_CAP_VARIABLE_DIMENSIONS flag to indicate > whether encoder supports variable dimension encoding. Isn't this about variable (or "changing") "resolutions" instead of dimensions? Carl Eugen
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Carl Eugen Hoyos > Sent: Tuesday, July 30, 2019 16:06 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > Am Di., 30. Juli 2019 um 06:46 Uhr schrieb Linjie Fu <linjie.fu@intel.com>: > > > > Add AV_CODEC_CAP_VARIABLE_DIMENSIONS flag to indicate > > whether encoder supports variable dimension encoding. > > Isn't this about variable (or "changing") "resolutions" instead of dimensions? > Comment is welcome and "variable resolutions" is good. But is "dimension" not quite suitable for the meaning of "variable size"? - linjie
Am Di., 30. Juli 2019 um 11:25 Uhr schrieb Fu, Linjie <linjie.fu@intel.com>: > > > -----Original Message----- > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > > Of Carl Eugen Hoyos > > Sent: Tuesday, July 30, 2019 16:06 > > To: FFmpeg development discussions and patches <ffmpeg- > > devel@ffmpeg.org> > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > Am Di., 30. Juli 2019 um 06:46 Uhr schrieb Linjie Fu <linjie.fu@intel.com>: > > > > > > Add AV_CODEC_CAP_VARIABLE_DIMENSIONS flag to indicate > > > whether encoder supports variable dimension encoding. > > > > Isn't this about variable (or "changing") "resolutions" instead of dimensions? > > > > Comment is welcome and "variable resolutions" is good. > But is "dimension" not quite suitable for the meaning of "variable size"? It took me some time to understand that "dimension" implies resolution, especially since the FFmpeg documentation mentions resolution iirc. Carl Eugen
On 7/30/2019 6:33 AM, Carl Eugen Hoyos wrote: > Am Di., 30. Juli 2019 um 11:25 Uhr schrieb Fu, Linjie <linjie.fu@intel.com>: >> >>> -----Original Message----- >>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf >>> Of Carl Eugen Hoyos >>> Sent: Tuesday, July 30, 2019 16:06 >>> To: FFmpeg development discussions and patches <ffmpeg- >>> devel@ffmpeg.org> >>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add >>> AV_CODEC_CAP_VARIABLE_DIMENSIONS flag >>> >>> Am Di., 30. Juli 2019 um 06:46 Uhr schrieb Linjie Fu <linjie.fu@intel.com>: >>>> >>>> Add AV_CODEC_CAP_VARIABLE_DIMENSIONS flag to indicate >>>> whether encoder supports variable dimension encoding. >>> >>> Isn't this about variable (or "changing") "resolutions" instead of dimensions? >>> >> >> Comment is welcome and "variable resolutions" is good. > >> But is "dimension" not quite suitable for the meaning of "variable size"? > > It took me some time to understand that "dimension" implies resolution, > especially since the FFmpeg documentation mentions resolution iirc. > > Carl Eugen We have a AV_SIDE_DATA_PARAM_CHANGE_DIMENSIONS flag to signal resolution changes, so both words seem to be used interchangeably. This also makes me think that the existing AV_CODEC_CAP_PARAM_CHANGE codec cap can be used for this already, without the need for a new one. It should a matter of using AV_PKT_DATA_PARAM_CHANGE packet side data afterwards in some form.
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of James Almer > Sent: Tuesday, July 30, 2019 21:27 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > On 7/30/2019 6:33 AM, Carl Eugen Hoyos wrote: > > Am Di., 30. Juli 2019 um 11:25 Uhr schrieb Fu, Linjie <linjie.fu@intel.com>: > >> > >>> -----Original Message----- > >>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > Behalf > >>> Of Carl Eugen Hoyos > >>> Sent: Tuesday, July 30, 2019 16:06 > >>> To: FFmpeg development discussions and patches <ffmpeg- > >>> devel@ffmpeg.org> > >>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > >>> AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > >>> > >>> Am Di., 30. Juli 2019 um 06:46 Uhr schrieb Linjie Fu <linjie.fu@intel.com>: > >>>> > >>>> Add AV_CODEC_CAP_VARIABLE_DIMENSIONS flag to indicate > >>>> whether encoder supports variable dimension encoding. > >>> > >>> Isn't this about variable (or "changing") "resolutions" instead of > dimensions? > >>> > >> > >> Comment is welcome and "variable resolutions" is good. > > > >> But is "dimension" not quite suitable for the meaning of "variable size"? > > > > It took me some time to understand that "dimension" implies resolution, > > especially since the FFmpeg documentation mentions resolution iirc. > > > > Carl Eugen > > We have a AV_SIDE_DATA_PARAM_CHANGE_DIMENSIONS flag to signal > resolution > changes, so both words seem to be used interchangeably. > > This also makes me think that the existing > AV_CODEC_CAP_PARAM_CHANGE > codec cap can be used for this already, without the need for a new one. > It should a matter of using AV_PKT_DATA_PARAM_CHANGE packet side > data > afterwards in some form. Thanks. Saw the modification in fe75dc8583b65612f3a94144ee090e741dc926d5, and it seems it was designed for decoder at first, like "rawdec/nellymoserdec/interplayvideo". "Also define a codec capability for codecs that can handle parameters changed externally between decoded packets. " Since currently no encoder is engaged with this flag, it's good for me to reuse this if there is no other concern. - linjie
On Tue, Jul 30, 2019 at 10:27:10AM -0300, James Almer wrote: > On 7/30/2019 6:33 AM, Carl Eugen Hoyos wrote: > > Am Di., 30. Juli 2019 um 11:25 Uhr schrieb Fu, Linjie <linjie.fu@intel.com>: > >> > >>> -----Original Message----- > >>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > >>> Of Carl Eugen Hoyos > >>> Sent: Tuesday, July 30, 2019 16:06 > >>> To: FFmpeg development discussions and patches <ffmpeg- > >>> devel@ffmpeg.org> > >>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > >>> AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > >>> > >>> Am Di., 30. Juli 2019 um 06:46 Uhr schrieb Linjie Fu <linjie.fu@intel.com>: > >>>> > >>>> Add AV_CODEC_CAP_VARIABLE_DIMENSIONS flag to indicate > >>>> whether encoder supports variable dimension encoding. > >>> > >>> Isn't this about variable (or "changing") "resolutions" instead of dimensions? > >>> > >> > >> Comment is welcome and "variable resolutions" is good. > > > >> But is "dimension" not quite suitable for the meaning of "variable size"? > > > > It took me some time to understand that "dimension" implies resolution, > > especially since the FFmpeg documentation mentions resolution iirc. > > > > Carl Eugen > > We have a AV_SIDE_DATA_PARAM_CHANGE_DIMENSIONS flag to signal resolution > changes, so both words seem to be used interchangeably. > > This also makes me think that the existing AV_CODEC_CAP_PARAM_CHANGE > codec cap can be used for this already, without the need for a new one. > It should a matter of using AV_PKT_DATA_PARAM_CHANGE packet side data > afterwards in some form. if AV_PKT_DATA_PARAM_CHANGE is used then other parameters would need to be handled too i think. For example pixel format changes alongside width and height. And it leaves some areas of uncertanity which may require more flags like does a aspect ratio change or a change of one of the colorspace parameters need a encoder "flush/restart". (the flush is done from outside the encoder in the patch so the code would need to know) Also for symmetry, if AV_PKT_DATA_PARAM_CHANGE expects sidedata on the input side of the decoder, something should produce matching side data on the encoder side. encoder -> decoder should continue working. So only a demuxer generating the side data could be a problem. [...]
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Michael Niedermayer > Sent: Wednesday, July 31, 2019 14:04 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > On Tue, Jul 30, 2019 at 10:27:10AM -0300, James Almer wrote: > > On 7/30/2019 6:33 AM, Carl Eugen Hoyos wrote: > > > Am Di., 30. Juli 2019 um 11:25 Uhr schrieb Fu, Linjie <linjie.fu@intel.com>: > > >> > > >>> -----Original Message----- > > >>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > Behalf > > >>> Of Carl Eugen Hoyos > > >>> Sent: Tuesday, July 30, 2019 16:06 > > >>> To: FFmpeg development discussions and patches <ffmpeg- > > >>> devel@ffmpeg.org> > > >>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > >>> AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > >>> > > >>> Am Di., 30. Juli 2019 um 06:46 Uhr schrieb Linjie Fu > <linjie.fu@intel.com>: > > >>>> > > >>>> Add AV_CODEC_CAP_VARIABLE_DIMENSIONS flag to indicate > > >>>> whether encoder supports variable dimension encoding. > > >>> > > >>> Isn't this about variable (or "changing") "resolutions" instead of > dimensions? > > >>> > > >> > > >> Comment is welcome and "variable resolutions" is good. > > > > > >> But is "dimension" not quite suitable for the meaning of "variable size"? > > > > > > It took me some time to understand that "dimension" implies resolution, > > > especially since the FFmpeg documentation mentions resolution iirc. > > > > > > Carl Eugen > > > > We have a AV_SIDE_DATA_PARAM_CHANGE_DIMENSIONS flag to signal > resolution > > changes, so both words seem to be used interchangeably. > > > > > This also makes me think that the existing > AV_CODEC_CAP_PARAM_CHANGE > > codec cap can be used for this already, without the need for a new one. > > It should a matter of using AV_PKT_DATA_PARAM_CHANGE packet side > data > > afterwards in some form. > > if AV_PKT_DATA_PARAM_CHANGE is used then other parameters would > need to > be handled too i think. > For example pixel format changes alongside width and height. > And it leaves some areas of uncertanity which may require more flags > like does a aspect ratio change or a change of one of the colorspace > parameters need a encoder "flush/restart". (the flush is done from > outside the encoder in the patch so the code would need to know) > > Also for symmetry, if AV_PKT_DATA_PARAM_CHANGE expects sidedata on > the input side of the decoder, something should produce matching > side data on the encoder side. > > encoder -> decoder should continue working. So only a demuxer > generating the side data could be a problem. Sounds like a new flag will be more robust?
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Fu, Linjie > Sent: Thursday, August 1, 2019 22:51 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > -----Original Message----- > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > Behalf > > Of Michael Niedermayer > > Sent: Wednesday, July 31, 2019 14:04 > > To: FFmpeg development discussions and patches <ffmpeg- > > devel@ffmpeg.org> > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > On Tue, Jul 30, 2019 at 10:27:10AM -0300, James Almer wrote: > > > On 7/30/2019 6:33 AM, Carl Eugen Hoyos wrote: > > > > Am Di., 30. Juli 2019 um 11:25 Uhr schrieb Fu, Linjie > <linjie.fu@intel.com>: > > > >> > > > >>> -----Original Message----- > > > >>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] > On > > Behalf > > > >>> Of Carl Eugen Hoyos > > > >>> Sent: Tuesday, July 30, 2019 16:06 > > > >>> To: FFmpeg development discussions and patches <ffmpeg- > > > >>> devel@ffmpeg.org> > > > >>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > > >>> AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > >>> > > > >>> Am Di., 30. Juli 2019 um 06:46 Uhr schrieb Linjie Fu > > <linjie.fu@intel.com>: > > > >>>> > > > >>>> Add AV_CODEC_CAP_VARIABLE_DIMENSIONS flag to indicate > > > >>>> whether encoder supports variable dimension encoding. > > > >>> > > > >>> Isn't this about variable (or "changing") "resolutions" instead of > > dimensions? > > > >>> > > > >> > > > >> Comment is welcome and "variable resolutions" is good. > > > > > > > >> But is "dimension" not quite suitable for the meaning of "variable size"? > > > > > > > > It took me some time to understand that "dimension" implies > resolution, > > > > especially since the FFmpeg documentation mentions resolution iirc. > > > > > > > > Carl Eugen > > > > > > We have a AV_SIDE_DATA_PARAM_CHANGE_DIMENSIONS flag to signal > > resolution > > > changes, so both words seem to be used interchangeably. > > > > > > > > This also makes me think that the existing > > AV_CODEC_CAP_PARAM_CHANGE > > > codec cap can be used for this already, without the need for a new one. > > > It should a matter of using AV_PKT_DATA_PARAM_CHANGE packet side > > data > > > afterwards in some form. > > > > if AV_PKT_DATA_PARAM_CHANGE is used then other parameters would > > need to > > be handled too i think. > > For example pixel format changes alongside width and height. > > And it leaves some areas of uncertanity which may require more flags > > like does a aspect ratio change or a change of one of the colorspace > > parameters need a encoder "flush/restart". (the flush is done from > > outside the encoder in the patch so the code would need to know) > > > > Also for symmetry, if AV_PKT_DATA_PARAM_CHANGE expects sidedata > on > > the input side of the decoder, something should produce matching > > side data on the encoder side. > > > > encoder -> decoder should continue working. So only a demuxer > > generating the side data could be a problem. > > Sounds like a new flag will be more robust? Ping for this patch set? https://patchwork.ffmpeg.org/patch/14122/ https://patchwork.ffmpeg.org/patch/14139/ https://patchwork.ffmpeg.org/patch/14140/
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Fu, Linjie > Sent: Saturday, August 31, 2019 12:40 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > -----Original Message----- > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > Behalf > > Of Fu, Linjie > > Sent: Thursday, August 1, 2019 22:51 > > To: FFmpeg development discussions and patches <ffmpeg- > > devel@ffmpeg.org> > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > > -----Original Message----- > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > > Behalf > > > Of Michael Niedermayer > > > Sent: Wednesday, July 31, 2019 14:04 > > > To: FFmpeg development discussions and patches <ffmpeg- > > > devel@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > > > On Tue, Jul 30, 2019 at 10:27:10AM -0300, James Almer wrote: > > > > On 7/30/2019 6:33 AM, Carl Eugen Hoyos wrote: > > > > > Am Di., 30. Juli 2019 um 11:25 Uhr schrieb Fu, Linjie > > <linjie.fu@intel.com>: > > > > >> > > > > >>> -----Original Message----- > > > > >>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] > > On > > > Behalf > > > > >>> Of Carl Eugen Hoyos > > > > >>> Sent: Tuesday, July 30, 2019 16:06 > > > > >>> To: FFmpeg development discussions and patches <ffmpeg- > > > > >>> devel@ffmpeg.org> > > > > >>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > > > >>> AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > >>> > > > > >>> Am Di., 30. Juli 2019 um 06:46 Uhr schrieb Linjie Fu > > > <linjie.fu@intel.com>: > > > > >>>> > > > > >>>> Add AV_CODEC_CAP_VARIABLE_DIMENSIONS flag to indicate > > > > >>>> whether encoder supports variable dimension encoding. > > > > >>> > > > > >>> Isn't this about variable (or "changing") "resolutions" instead of > > > dimensions? > > > > >>> > > > > >> > > > > >> Comment is welcome and "variable resolutions" is good. > > > > > > > > > >> But is "dimension" not quite suitable for the meaning of "variable > size"? > > > > > > > > > > It took me some time to understand that "dimension" implies > > resolution, > > > > > especially since the FFmpeg documentation mentions resolution iirc. > > > > > > > > > > Carl Eugen > > > > > > > > We have a AV_SIDE_DATA_PARAM_CHANGE_DIMENSIONS flag to > signal > > > resolution > > > > changes, so both words seem to be used interchangeably. > > > > > > > > > > > This also makes me think that the existing > > > AV_CODEC_CAP_PARAM_CHANGE > > > > codec cap can be used for this already, without the need for a new one. > > > > It should a matter of using AV_PKT_DATA_PARAM_CHANGE packet > side > > > data > > > > afterwards in some form. > > > > > > if AV_PKT_DATA_PARAM_CHANGE is used then other parameters would > > > need to > > > be handled too i think. > > > For example pixel format changes alongside width and height. > > > And it leaves some areas of uncertanity which may require more flags > > > like does a aspect ratio change or a change of one of the colorspace > > > parameters need a encoder "flush/restart". (the flush is done from > > > outside the encoder in the patch so the code would need to know) > > > > > > Also for symmetry, if AV_PKT_DATA_PARAM_CHANGE expects sidedata > > on > > > the input side of the decoder, something should produce matching > > > side data on the encoder side. > > > > > > encoder -> decoder should continue working. So only a demuxer > > > generating the side data could be a problem. > > > > Sounds like a new flag will be more robust? > > Ping for this patch set? > https://patchwork.ffmpeg.org/patch/14122/ > https://patchwork.ffmpeg.org/patch/14139/ > https://patchwork.ffmpeg.org/patch/14140/ > Anything I could do for this patch set?
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Fu, > Linjie > Sent: Wednesday, September 11, 2019 15:12 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > -----Original Message----- > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > Behalf > > Of Fu, Linjie > > Sent: Saturday, August 31, 2019 12:40 > > To: FFmpeg development discussions and patches <ffmpeg- > > devel@ffmpeg.org> > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > > -----Original Message----- > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > > Behalf > > > Of Fu, Linjie > > > Sent: Thursday, August 1, 2019 22:51 > > > To: FFmpeg development discussions and patches <ffmpeg- > > > devel@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > > > > -----Original Message----- > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > > > Behalf > > > > Of Michael Niedermayer > > > > Sent: Wednesday, July 31, 2019 14:04 > > > > To: FFmpeg development discussions and patches <ffmpeg- > > > > devel@ffmpeg.org> > > > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > > > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > > > > > On Tue, Jul 30, 2019 at 10:27:10AM -0300, James Almer wrote: > > > > > On 7/30/2019 6:33 AM, Carl Eugen Hoyos wrote: > > > > > > Am Di., 30. Juli 2019 um 11:25 Uhr schrieb Fu, Linjie > > > <linjie.fu@intel.com>: > > > > > >> > > > > > >>> -----Original Message----- > > > > > >>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] > > > On > > > > Behalf > > > > > >>> Of Carl Eugen Hoyos > > > > > >>> Sent: Tuesday, July 30, 2019 16:06 > > > > > >>> To: FFmpeg development discussions and patches <ffmpeg- > > > > > >>> devel@ffmpeg.org> > > > > > >>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > > > > >>> AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > > >>> > > > > > >>> Am Di., 30. Juli 2019 um 06:46 Uhr schrieb Linjie Fu > > > > <linjie.fu@intel.com>: > > > > > >>>> > > > > > >>>> Add AV_CODEC_CAP_VARIABLE_DIMENSIONS flag to indicate > > > > > >>>> whether encoder supports variable dimension encoding. > > > > > >>> > > > > > >>> Isn't this about variable (or "changing") "resolutions" instead of > > > > dimensions? > > > > > >>> > > > > > >> > > > > > >> Comment is welcome and "variable resolutions" is good. > > > > > > > > > > > >> But is "dimension" not quite suitable for the meaning of "variable > > size"? > > > > > > > > > > > > It took me some time to understand that "dimension" implies > > > resolution, > > > > > > especially since the FFmpeg documentation mentions resolution iirc. > > > > > > > > > > > > Carl Eugen > > > > > > > > > > We have a AV_SIDE_DATA_PARAM_CHANGE_DIMENSIONS flag to > > signal > > > > resolution > > > > > changes, so both words seem to be used interchangeably. > > > > > > > > > > > > > > This also makes me think that the existing > > > > AV_CODEC_CAP_PARAM_CHANGE > > > > > codec cap can be used for this already, without the need for a new > one. > > > > > It should a matter of using AV_PKT_DATA_PARAM_CHANGE packet > > side > > > > data > > > > > afterwards in some form. > > > > > > > > if AV_PKT_DATA_PARAM_CHANGE is used then other parameters > would > > > > need to > > > > be handled too i think. > > > > For example pixel format changes alongside width and height. > > > > And it leaves some areas of uncertanity which may require more flags > > > > like does a aspect ratio change or a change of one of the colorspace > > > > parameters need a encoder "flush/restart". (the flush is done from > > > > outside the encoder in the patch so the code would need to know) > > > > > > > > Also for symmetry, if AV_PKT_DATA_PARAM_CHANGE expects > sidedata > > > on > > > > the input side of the decoder, something should produce matching > > > > side data on the encoder side. > > > > > > > > encoder -> decoder should continue working. So only a demuxer > > > > generating the side data could be a problem. > > > > > > Sounds like a new flag will be more robust? > > > > Ping for this patch set? > > https://patchwork.ffmpeg.org/patch/14122/ > > https://patchwork.ffmpeg.org/patch/14139/ > > https://patchwork.ffmpeg.org/patch/14140/ > > > Anything I could do for this patch set? Another kindly ping. -linjie
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Fu, > Linjie > Sent: Thursday, October 10, 2019 14:46 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > -----Original Message----- > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Fu, > > Linjie > > Sent: Wednesday, September 11, 2019 15:12 > > To: FFmpeg development discussions and patches <ffmpeg- > > devel@ffmpeg.org> > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > > -----Original Message----- > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > > Behalf > > > Of Fu, Linjie > > > Sent: Saturday, August 31, 2019 12:40 > > > To: FFmpeg development discussions and patches <ffmpeg- > > > devel@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > > > > -----Original Message----- > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > > > Behalf > > > > Of Fu, Linjie > > > > Sent: Thursday, August 1, 2019 22:51 > > > > To: FFmpeg development discussions and patches <ffmpeg- > > > > devel@ffmpeg.org> > > > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > > > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > > > > > > -----Original Message----- > > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > > > > Behalf > > > > > Of Michael Niedermayer > > > > > Sent: Wednesday, July 31, 2019 14:04 > > > > > To: FFmpeg development discussions and patches <ffmpeg- > > > > > devel@ffmpeg.org> > > > > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > > > > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > > > > > > > On Tue, Jul 30, 2019 at 10:27:10AM -0300, James Almer wrote: > > > > > > On 7/30/2019 6:33 AM, Carl Eugen Hoyos wrote: > > > > > > > Am Di., 30. Juli 2019 um 11:25 Uhr schrieb Fu, Linjie > > > > <linjie.fu@intel.com>: > > > > > > >> > > > > > > >>> -----Original Message----- > > > > > > >>> From: ffmpeg-devel [mailto:ffmpeg-devel- > bounces@ffmpeg.org] > > > > On > > > > > Behalf > > > > > > >>> Of Carl Eugen Hoyos > > > > > > >>> Sent: Tuesday, July 30, 2019 16:06 > > > > > > >>> To: FFmpeg development discussions and patches <ffmpeg- > > > > > > >>> devel@ffmpeg.org> > > > > > > >>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > > > > > >>> AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > > > >>> > > > > > > >>> Am Di., 30. Juli 2019 um 06:46 Uhr schrieb Linjie Fu > > > > > <linjie.fu@intel.com>: > > > > > > >>>> > > > > > > >>>> Add AV_CODEC_CAP_VARIABLE_DIMENSIONS flag to indicate > > > > > > >>>> whether encoder supports variable dimension encoding. > > > > > > >>> > > > > > > >>> Isn't this about variable (or "changing") "resolutions" instead of > > > > > dimensions? > > > > > > >>> > > > > > > >> > > > > > > >> Comment is welcome and "variable resolutions" is good. > > > > > > > > > > > > > >> But is "dimension" not quite suitable for the meaning of "variable > > > size"? > > > > > > > > > > > > > > It took me some time to understand that "dimension" implies > > > > resolution, > > > > > > > especially since the FFmpeg documentation mentions resolution > iirc. > > > > > > > > > > > > > > Carl Eugen > > > > > > > > > > > > We have a AV_SIDE_DATA_PARAM_CHANGE_DIMENSIONS flag to > > > signal > > > > > resolution > > > > > > changes, so both words seem to be used interchangeably. > > > > > > > > > > > > > > > > > This also makes me think that the existing > > > > > AV_CODEC_CAP_PARAM_CHANGE > > > > > > codec cap can be used for this already, without the need for a new > > one. > > > > > > It should a matter of using AV_PKT_DATA_PARAM_CHANGE packet > > > side > > > > > data > > > > > > afterwards in some form. > > > > > > > > > > if AV_PKT_DATA_PARAM_CHANGE is used then other parameters > > would > > > > > need to > > > > > be handled too i think. > > > > > For example pixel format changes alongside width and height. > > > > > And it leaves some areas of uncertanity which may require more flags > > > > > like does a aspect ratio change or a change of one of the colorspace > > > > > parameters need a encoder "flush/restart". (the flush is done from > > > > > outside the encoder in the patch so the code would need to know) > > > > > > > > > > Also for symmetry, if AV_PKT_DATA_PARAM_CHANGE expects > > sidedata > > > > on > > > > > the input side of the decoder, something should produce matching > > > > > side data on the encoder side. > > > > > > > > > > encoder -> decoder should continue working. So only a demuxer > > > > > generating the side data could be a problem. > > > > > > > > Sounds like a new flag will be more robust? > > > > > > Ping for this patch set? > > > https://patchwork.ffmpeg.org/patch/14122/ > > > https://patchwork.ffmpeg.org/patch/14139/ > > > https://patchwork.ffmpeg.org/patch/14140/ > > > > > Anything I could do for this patch set? > > Another kindly ping. > Hi, Please help to review.
On Mon, Feb 17, 2020 at 06:28:23PM +0000, Fu, Linjie wrote: > > -----Original Message----- > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Fu, > > Linjie > > Sent: Thursday, October 10, 2019 14:46 > > To: FFmpeg development discussions and patches <ffmpeg- > > devel@ffmpeg.org> > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > > -----Original Message----- > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > Fu, > > > Linjie > > > Sent: Wednesday, September 11, 2019 15:12 > > > To: FFmpeg development discussions and patches <ffmpeg- > > > devel@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > > > > -----Original Message----- > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > > > Behalf > > > > Of Fu, Linjie > > > > Sent: Saturday, August 31, 2019 12:40 > > > > To: FFmpeg development discussions and patches <ffmpeg- > > > > devel@ffmpeg.org> > > > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > > > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > > > > > > -----Original Message----- > > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > > > > Behalf > > > > > Of Fu, Linjie > > > > > Sent: Thursday, August 1, 2019 22:51 > > > > > To: FFmpeg development discussions and patches <ffmpeg- > > > > > devel@ffmpeg.org> > > > > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > > > > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > > > > > > > > -----Original Message----- > > > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > > > > > Behalf > > > > > > Of Michael Niedermayer > > > > > > Sent: Wednesday, July 31, 2019 14:04 > > > > > > To: FFmpeg development discussions and patches <ffmpeg- > > > > > > devel@ffmpeg.org> > > > > > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > > > > > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > > > > > > > > > On Tue, Jul 30, 2019 at 10:27:10AM -0300, James Almer wrote: > > > > > > > On 7/30/2019 6:33 AM, Carl Eugen Hoyos wrote: > > > > > > > > Am Di., 30. Juli 2019 um 11:25 Uhr schrieb Fu, Linjie > > > > > <linjie.fu@intel.com>: > > > > > > > >> > > > > > > > >>> -----Original Message----- > > > > > > > >>> From: ffmpeg-devel [mailto:ffmpeg-devel- > > bounces@ffmpeg.org] > > > > > On > > > > > > Behalf > > > > > > > >>> Of Carl Eugen Hoyos > > > > > > > >>> Sent: Tuesday, July 30, 2019 16:06 > > > > > > > >>> To: FFmpeg development discussions and patches <ffmpeg- > > > > > > > >>> devel@ffmpeg.org> > > > > > > > >>> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > > > > > > >>> AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > > > > >>> > > > > > > > >>> Am Di., 30. Juli 2019 um 06:46 Uhr schrieb Linjie Fu > > > > > > <linjie.fu@intel.com>: > > > > > > > >>>> > > > > > > > >>>> Add AV_CODEC_CAP_VARIABLE_DIMENSIONS flag to indicate > > > > > > > >>>> whether encoder supports variable dimension encoding. > > > > > > > >>> > > > > > > > >>> Isn't this about variable (or "changing") "resolutions" instead of > > > > > > dimensions? > > > > > > > >>> > > > > > > > >> > > > > > > > >> Comment is welcome and "variable resolutions" is good. > > > > > > > > > > > > > > > >> But is "dimension" not quite suitable for the meaning of "variable > > > > size"? > > > > > > > > > > > > > > > > It took me some time to understand that "dimension" implies > > > > > resolution, > > > > > > > > especially since the FFmpeg documentation mentions resolution > > iirc. > > > > > > > > > > > > > > > > Carl Eugen > > > > > > > > > > > > > > We have a AV_SIDE_DATA_PARAM_CHANGE_DIMENSIONS flag to > > > > signal > > > > > > resolution > > > > > > > changes, so both words seem to be used interchangeably. > > > > > > > > > > > > > > > > > > > > This also makes me think that the existing > > > > > > AV_CODEC_CAP_PARAM_CHANGE > > > > > > > codec cap can be used for this already, without the need for a new > > > one. > > > > > > > It should a matter of using AV_PKT_DATA_PARAM_CHANGE packet > > > > side > > > > > > data > > > > > > > afterwards in some form. > > > > > > > > > > > > if AV_PKT_DATA_PARAM_CHANGE is used then other parameters > > > would > > > > > > need to > > > > > > be handled too i think. > > > > > > For example pixel format changes alongside width and height. > > > > > > And it leaves some areas of uncertanity which may require more flags > > > > > > like does a aspect ratio change or a change of one of the colorspace > > > > > > parameters need a encoder "flush/restart". (the flush is done from > > > > > > outside the encoder in the patch so the code would need to know) > > > > > > > > > > > > Also for symmetry, if AV_PKT_DATA_PARAM_CHANGE expects > > > sidedata > > > > > on > > > > > > the input side of the decoder, something should produce matching > > > > > > side data on the encoder side. > > > > > > > > > > > > encoder -> decoder should continue working. So only a demuxer > > > > > > generating the side data could be a problem. > > > > > > > > > > Sounds like a new flag will be more robust? > > > > > > > > Ping for this patch set? > > > > https://patchwork.ffmpeg.org/patch/14122/ iam hesitating because it feeds encoders with changing resolution resulting in potentially undefined behavior ... > > > > https://patchwork.ffmpeg.org/patch/14139/ long discussion here its not immedeatly clear if anyone was against it Also there is the question about the API, is there anything in the API documentation that restricts the user app from changing the encoder input frame dimensions? This should be documented somewhere if its not ... If a flag is added that affects this, it would have to be documented so someone writing a user app using the encoders would know if they are allowed to change the resolution. With just the flag and its documentation a developer could miss the flag entirely > > > > https://patchwork.ffmpeg.org/patch/14140/ > > > > > > > Anything I could do for this patch set? > > > > Another kindly ping. > > > Hi, > > Please help to review. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Quoting Michael Niedermayer (2020-02-17 23:38:31) > On Mon, Feb 17, 2020 at 06:28:23PM +0000, Fu, Linjie wrote: > > > > > > https://patchwork.ffmpeg.org/patch/14122/ > > iam hesitating because it feeds encoders with changing resolution resulting > in potentially undefined behavior ... > > > > > > > https://patchwork.ffmpeg.org/patch/14139/ > > long discussion here its not immedeatly clear if anyone was against it > > Also there is the question about the API, is there anything in the API > documentation that restricts the user app from changing the encoder > input frame dimensions? > This should be documented somewhere if its not ... > > If a flag is added that affects this, it would have to > be documented so someone writing a user app using the encoders > would know if they are allowed to change the resolution. > With just the flag and its documentation a developer could miss > the flag entirely Is there even a sufficiently strong use case for this? Why not just create a new encoder instance?
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Anton Khirnov > Sent: Tuesday, February 18, 2020 21:32 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > Quoting Michael Niedermayer (2020-02-17 23:38:31) > > On Mon, Feb 17, 2020 at 06:28:23PM +0000, Fu, Linjie wrote: > > > > > > > > https://patchwork.ffmpeg.org/patch/14122/ > > > > iam hesitating because it feeds encoders with changing resolution resulting > > in potentially undefined behavior ... > > Any suggestions? > > > > > > https://patchwork.ffmpeg.org/patch/14139/ > > > > long discussion here its not immedeatly clear if anyone was against it Please help to give some inputs if this would leads to some unexpected results from your point of view. > > Also there is the question about the API, is there anything in the API > > documentation that restricts the user app from changing the encoder > > input frame dimensions? > > This should be documented somewhere if its not ... > > > > If a flag is added that affects this, it would have to > > be documented so someone writing a user app using the encoders > > would know if they are allowed to change the resolution. > > With just the flag and its documentation a developer could miss > > the flag entirely Since I didn't find the descriptions in doxygen: https://ffmpeg.org/doxygen/trunk/group__lavc__encoding.html#ga4fde50e2cad4cf3d66d882a7078aeab4 The things should be done seem to be: [1]. Documentation in API, to declare that encoders require input frame to be in constant dimensions, unless the encoders have the capability of dynamic resolution encoding [2]. [2]. Documentation in API, to declare this codec flag would affect the restrict in [1]. If it's ok, I'll update the patch with the documentations added. > Is there even a sufficiently strong use case for this? Why not just Like transcoding reinit-large_420_8-to-small_420_8.h264 (from 352x288 to 240x196) and prefer to keep the resolution changing in the output stream. IMHO not modifying the resolution unless it's required by user is kind of natural. > create a new encoder instance? Would you please help to elaborate more about "create a new encoder instance?" What I intended to do is to flush and close the encoder when resolution changing, and reinit/recreate a new one.
Quoting Fu, Linjie (2020-02-18 16:06:10) > > Is there even a sufficiently strong use case for this? Why not just > > Like transcoding reinit-large_420_8-to-small_420_8.h264 (from 352x288 to 240x196) > and prefer to keep the resolution changing in the output stream. > > IMHO not modifying the resolution unless it's required by user is kind of natural. > > > create a new encoder instance? > > Would you please help to elaborate more about "create a new encoder instance?" > > What I intended to do is to flush and close the encoder when resolution changing, > and reinit/recreate a new one. Yes, that's what I mean. Flush and destroy an AVCodecContext and open a new one. But then why is there a need for this flag?
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Anton Khirnov > Sent: Wednesday, February 19, 2020 00:42 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > Quoting Fu, Linjie (2020-02-18 16:06:10) > > > Is there even a sufficiently strong use case for this? Why not just > > > > Like transcoding reinit-large_420_8-to-small_420_8.h264 (from 352x288 to > 240x196) > > and prefer to keep the resolution changing in the output stream. > > > > IMHO not modifying the resolution unless it's required by user is kind of > natural. > > > > > create a new encoder instance? > > > > Would you please help to elaborate more about "create a new encoder > instance?" > > > > What I intended to do is to flush and close the encoder when resolution > changing, > > and reinit/recreate a new one. > > Yes, that's what I mean. Flush and destroy an AVCodecContext and open a > new one. But then why is there a need for this flag? > Previously I intended to add close and re-init inside specific encoder like [1], without destroy AVCodecContext, and some codec [2] may support dynamic resolution encoding without reinitialization. (which allows resolution changing on P/inter frame, not only I/key frame). So it's more secure to add a flag to declare the capability, and add the support for encoders step by step. [1] https://patchwork.ffmpeg.org/project/ffmpeg/patch/1564307683-22194-1-git-send-email-linjie.fu@intel.com/ [2] https://patchwork.ffmpeg.org/project/ffmpeg/patch/1565688544-9043-1-git-send-email-linjie.fu@intel.com/
On Tue, Feb 18, 2020 at 05:42:19PM +0100, Anton Khirnov wrote: > Quoting Fu, Linjie (2020-02-18 16:06:10) > > > Is there even a sufficiently strong use case for this? Why not just > > > > Like transcoding reinit-large_420_8-to-small_420_8.h264 (from 352x288 to 240x196) > > and prefer to keep the resolution changing in the output stream. > > > > IMHO not modifying the resolution unless it's required by user is kind of natural. > > > > > create a new encoder instance? > > > > Would you please help to elaborate more about "create a new encoder instance?" > > > > What I intended to do is to flush and close the encoder when resolution changing, > > and reinit/recreate a new one. > > Yes, that's what I mean. Flush and destroy an AVCodecContext and open a > new one. But then why is there a need for this flag? some sort of capability check would be needed for flush destroy open too because there are a range of cases where this would not produce a decodeable file. for example some codecs like msmpeg4 do not store the resolution in their header and instead its stored in the container only once per stream. thx [...]
Quoting Michael Niedermayer (2020-02-19 19:07:28) > On Tue, Feb 18, 2020 at 05:42:19PM +0100, Anton Khirnov wrote: > > Quoting Fu, Linjie (2020-02-18 16:06:10) > > > > Is there even a sufficiently strong use case for this? Why not just > > > > > > Like transcoding reinit-large_420_8-to-small_420_8.h264 (from 352x288 to 240x196) > > > and prefer to keep the resolution changing in the output stream. > > > > > > IMHO not modifying the resolution unless it's required by user is kind of natural. > > > > > > > create a new encoder instance? > > > > > > Would you please help to elaborate more about "create a new encoder instance?" > > > > > > What I intended to do is to flush and close the encoder when resolution changing, > > > and reinit/recreate a new one. > > > > Yes, that's what I mean. Flush and destroy an AVCodecContext and open a > > new one. But then why is there a need for this flag? > > some sort of capability check would be needed for flush destroy open too > because there are a range of cases where this would not produce a decodeable > file. > for example some codecs like msmpeg4 do not store the resolution in their > header and instead its stored in the container only once per stream. Right, but that is a property of a given codec ID and should be a flag in AVCodecDescriptor, not in a specific encoder implementation. Actually I had some plans to add such a flag and then write a bitstream filter for concatenating streams where some extra processing is required.
Quoting Fu, Linjie (2020-02-19 17:25:30) > > Yes, that's what I mean. Flush and destroy an AVCodecContext and open a > > new one. But then why is there a need for this flag? > > > Previously I intended to add close and re-init inside specific encoder like [1], > without destroy AVCodecContext, and some codec [2] may support > dynamic resolution encoding without reinitialization. (which allows resolution > changing on P/inter frame, not only I/key frame). > > So it's more secure to add a flag to declare the capability, and add the support > for encoders step by step. The concern here is that libavcodec encoders have always assumed that the stream parameters are set once and never change afterwards. Violating that constraint might lead to undefined behaviour in code that relies on this assumption, and it would be quite hard to identify all such code. So the question is whether this danger and the effort required to avoid it are justified. Do we get sufficient benefits from having parameter change capability inside encoders over just creating a new encoder instance when needed? Do people really need to change resolution mid-GOP?
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Fu, > Linjie > Sent: Tuesday, February 18, 2020 23:06 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > -----Original Message----- > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > Anton Khirnov > > Sent: Tuesday, February 18, 2020 21:32 > > To: FFmpeg development discussions and patches <ffmpeg- > > devel@ffmpeg.org> > > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add > > AV_CODEC_CAP_VARIABLE_DIMENSIONS flag > > > > Quoting Michael Niedermayer (2020-02-17 23:38:31) > > > On Mon, Feb 17, 2020 at 06:28:23PM +0000, Fu, Linjie wrote: > > > > > > > > > > https://patchwork.ffmpeg.org/patch/14122/ > > > > > > iam hesitating because it feeds encoders with changing resolution > resulting > > > in potentially undefined behavior ... > > > > > Any suggestions? > > > > > > > > https://patchwork.ffmpeg.org/patch/14139/ > > > > > > long discussion here its not immedeatly clear if anyone was against it > > Please help to give some inputs if this would leads to some unexpected > results from your point of view. > > > > Also there is the question about the API, is there anything in the API > > > documentation that restricts the user app from changing the encoder > > > input frame dimensions? > > > This should be documented somewhere if its not ... > > > > > > If a flag is added that affects this, it would have to > > > be documented so someone writing a user app using the encoders > > > would know if they are allowed to change the resolution. > > > With just the flag and its documentation a developer could miss > > > the flag entirely > > Since I didn't find the descriptions in doxygen: > https://ffmpeg.org/doxygen/trunk/group__lavc__encoding.html#ga4fde50 > e2cad4cf3d66d882a7078aeab4 > > The things should be done seem to be: > > [1]. Documentation in API, to declare that encoders require input frame to > be in constant dimensions, unless the encoders have the capability of > dynamic resolution encoding [2]. > > [2]. Documentation in API, to declare this codec flag would affect the restrict > in [1]. > > If it's ok, I'll update the patch with the documentations added. > Update the patch with API documentations added. And add checks to make sure it won't result into undefined behavior. As to the support for more encoders, flush/destroy/reinit the encoder would work, will investigate more to do this inside specific encoder or in more general ways. Please help to review, thanks. - Linjie
diff --git a/doc/APIchanges b/doc/APIchanges index 07331b1..2e855f2 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,9 @@ libavutil: 2017-10-21 API changes, most recent first: +2019-07-xx - xxxxxxxxxx - lavc 58.55.101 - avcodec.h + Add AV_CODEC_CAP_VARIABLE_DIMENSIONS + -------- 8< --------- FFmpeg 4.2 was cut here -------- 8< --------- 2019-06-21 - a30e44098a - lavu 56.30.100 - frame.h diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c index 9cfbc45..25ea1c6 100644 --- a/fftools/cmdutils.c +++ b/fftools/cmdutils.c @@ -1424,6 +1424,8 @@ static void print_codec(const AVCodec *c) printf("hardware "); if (c->capabilities & AV_CODEC_CAP_HYBRID) printf("hybrid "); + if (c->capabilities & AV_CODEC_CAP_VARIABLE_DIMENSIONS) + printf("multidimension "); if (!c->capabilities) printf("none"); printf("\n"); diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index d234271..a7704ea 100644 --- a/libavcodec/avcodec.h +++ b/libavcodec/avcodec.h @@ -1092,6 +1092,11 @@ typedef struct RcOverride{ #define AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE (1 << 20) /** + * Codec supports variable dimensions in encoding. + */ +#define AV_CODEC_CAP_VARIABLE_DIMENSIONS (1 << 21) + +/** * Pan Scan area. * This specifies the area which should be displayed. * Note there may be multiple such areas for one frame. diff --git a/libavcodec/rawenc.c b/libavcodec/rawenc.c index d181b74..486c0d7 100644 --- a/libavcodec/rawenc.c +++ b/libavcodec/rawenc.c @@ -92,4 +92,5 @@ AVCodec ff_rawvideo_encoder = { .id = AV_CODEC_ID_RAWVIDEO, .init = raw_encode_init, .encode2 = raw_encode, + .capabilities = AV_CODEC_CAP_VARIABLE_DIMENSIONS, }; diff --git a/libavcodec/version.h b/libavcodec/version.h index 43c8cdb..e70ebc0 100644 --- a/libavcodec/version.h +++ b/libavcodec/version.h @@ -29,7 +29,7 @@ #define LIBAVCODEC_VERSION_MAJOR 58 #define LIBAVCODEC_VERSION_MINOR 55 -#define LIBAVCODEC_VERSION_MICRO 100 +#define LIBAVCODEC_VERSION_MICRO 101 #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ LIBAVCODEC_VERSION_MINOR, \