[FFmpeg-devel] [PATCH, v2 2/4] avc/avcodec: add AV_CODEC_CAP_VARIABLE_DIMENSIONS flag

Tue Feb 18 00:38:31 EET 2020

On Mon, Feb 17, 2020 at 06:28:23PM +0000, Fu, Linjie wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Fu,
> > Linjie
> > Sent: Thursday, October 10, 2019 14:46
> > To: FFmpeg development discussions and patches <ffmpeg-
> > devel at 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 at ffmpeg.org> On Behalf Of
> > Fu,
> > > Linjie
> > > Sent: Wednesday, September 11, 2019 15:12
> > > To: FFmpeg development discussions and patches <ffmpeg-
> > > devel at 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 at ffmpeg.org] On
> > > Behalf
> > > > Of Fu, Linjie
> > > > Sent: Saturday, August 31, 2019 12:40
> > > > To: FFmpeg development discussions and patches <ffmpeg-
> > > > devel at 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 at ffmpeg.org] On
> > > > Behalf
> > > > > Of Fu, Linjie
> > > > > Sent: Thursday, August 1, 2019 22:51
> > > > > To: FFmpeg development discussions and patches <ffmpeg-
> > > > > devel at 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 at ffmpeg.org] On
> > > > > Behalf
> > > > > > Of Michael Niedermayer
> > > > > > Sent: Wednesday, July 31, 2019 14:04
> > > > > > To: FFmpeg development discussions and patches <ffmpeg-
> > > > > > devel at 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 at intel.com>:
> > > > > > > >>
> > > > > > > >>> -----Original Message-----
> > > > > > > >>> From: ffmpeg-devel [mailto:ffmpeg-devel-
> > bounces at ffmpeg.org]
> > > > > On
> > > > > > Behalf
> > > > > > > >>> Of Carl Eugen Hoyos
> > > > > > > >>> Sent: Tuesday, July 30, 2019 16:06
> > > > > > > >>> To: FFmpeg development discussions and patches <ffmpeg-
> > > > > > > >>> devel at 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 at 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 at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200217/4dce7603/attachment.sig>